On Fri, 1 Dec 2023 16:54:34 GMT, Anton Bobrov <d...@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