RFR: 8320655: awt screencast robot spin and sync issues with native libpipewire api [v3]

Alexander Zvegintsev azvegint at openjdk.org
Fri Dec 1 18:33:08 UTC 2023


On Fri, 1 Dec 2023 16:54:34 GMT, Anton Bobrov <duke at openjdk.org> wrote:

>> I think it's better to do it in the native, to avoid extra native-java hops.
>> 
>> As for the number of extra attempts, I think one-two is enough.
>> 
>> Something like this (I haven't tested it thoroughly yet):
>> 
>> 
>> diff --git a/src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c b/src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c
>> index 6a7c4124935..a7c5aadb1ae 100644
>> --- a/src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c
>> +++ b/src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c
>> @@ -855,6 +855,32 @@ static void arrayToRectangles(JNIEnv *env,
>>      (*env)->ReleaseIntArrayElements(env, boundsArray, body, 0);
>>  }
>>  
>> +static int makeAnAttempt(
>> +        const gchar *token,
>> +        GdkRectangle *requestedArea,
>> +        GdkRectangle *affectedScreenBounds,
>> +        gint affectedBoundsLength) {
>> +    if (!initScreencast(token, affectedScreenBounds, affectedBoundsLength)) {
>> +        return pw.pwFd;
>> +    }
>> +
>> +    if (!doLoop(*requestedArea)) {
>> +        return RESULT_ERROR;
>> +    }
>> +
>> +    while (!isAllDataReady()) {
>> +        fp_pw_thread_loop_lock(pw.loop);
>> +        fp_pw_thread_loop_wait(pw.loop);
>> +        if (hasPipewireFailed) {
>> +            fp_pw_thread_loop_unlock(pw.loop);
>> +            doCleanup();
>> +            return RESULT_ERROR;
>> +        }
>> +        fp_pw_thread_loop_unlock(pw.loop);
>> +    }
>> +    return RESULT_OK;
>> +}
>> +
>>  /*
>>   * Class:     sun_awt_screencast_ScreencastHelper
>>   * Method:    closeSession
>> @@ -911,25 +937,17 @@ JNIEXPORT jint JNICALL Java_sun_awt_screencast_ScreencastHelper_getRGBPixelsImpl
>>              jx, jy, jwidth, jheight, token
>>      );
>>  
>> -    if (!initScreencast(token, affectedScreenBounds, affectedBoundsLength)) {
>> -        releaseToken(env, jtoken, token);
>> -        return pw.pwFd;
>> -    }
>> -
>> -    if (!doLoop(requestedArea)) {
>> -        releaseToken(env, jtoken, token);
>> -        return RESULT_ERROR;
>> -    }
>> +    int attemptResult = makeAnAttempt(token, &requestedArea,
>> +                                      affectedScreenBounds, affectedBoundsLength);
>>  
>> -    while (!isAllDataReady()) {
>> -        fp_pw_thread_loop_lock(pw.loop);
>> -        fp_pw_thread_loop_wait(pw.loop);
>> -        if (hasPipewireFailed) {
>> -            fp_pw_thread_loop_unlock(pw.loop);
>> +    if (attemptResult) {
>> +        DEBUG_SCREENCAST("First attempt failed with %i, re-trying...\n", attemptResult);
>> +     ...
>
> @azvegint let me have a look next week and i'll try to come up with another PR for this. the pipewire is a bit tricky to test in my experience as there are quite a few moving parts/layers there and on some error conditions it even hangs bc they chose to use no timeouts in their implementation so it just gets stuck there forever sometimes, so i would need to come up with some way/s to test this.

Sure, let's move the retries to the [8321176](https://bugs.openjdk.org/browse/JDK-8321176)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16794#discussion_r1412447547


More information about the client-libs-dev mailing list