From f3fc4fb9735a4b67a3740b3796495017eb5c978c Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 2 Dec 2022 16:24:37 +0000 Subject: [PATCH] Fix threading of onFallbackApplied callback The callback is currently triggered on the ExoPlayer playback thread instead of the app thread that added the listener. PiperOrigin-RevId: 492474405 --- .../transformer/FallbackListener.java | 28 +++++++++++------ .../exoplayer2/transformer/Transformer.java | 6 +++- .../transformer/FallbackListenerTest.java | 31 ++++++++++++++++--- .../transformer/VideoEncoderWrapperTest.java | 1 + 4 files changed, 51 insertions(+), 15 deletions(-) diff --git a/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/FallbackListener.java b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/FallbackListener.java index 8f54f4cd8c..36ccf8c73d 100644 --- a/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/FallbackListener.java +++ b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/FallbackListener.java @@ -20,6 +20,7 @@ import static com.google.android.exoplayer2.util.Assertions.checkState; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.MediaItem; +import com.google.android.exoplayer2.util.HandlerWrapper; import com.google.android.exoplayer2.util.ListenerSet; import com.google.android.exoplayer2.util.Util; @@ -32,6 +33,7 @@ import com.google.android.exoplayer2.util.Util; private final MediaItem mediaItem; private final TransformationRequest originalTransformationRequest; private final ListenerSet transformerListeners; + private final HandlerWrapper transformerListenerHandler; private TransformationRequest fallbackTransformationRequest; private int trackCount; @@ -40,16 +42,20 @@ import com.google.android.exoplayer2.util.Util; * Creates a new instance. * * @param mediaItem The {@link MediaItem} to transform. - * @param transformerListeners The {@linkplain Transformer.Listener listeners} to forward events - * to. + * @param transformerListeners The {@linkplain Transformer.Listener listeners} to call {@link + * Transformer.Listener#onFallbackApplied} on. + * @param transformerListenerHandler The {@link HandlerWrapper} to call {@link + * Transformer.Listener#onFallbackApplied} events on. * @param originalTransformationRequest The original {@link TransformationRequest}. */ public FallbackListener( MediaItem mediaItem, ListenerSet transformerListeners, + HandlerWrapper transformerListenerHandler, TransformationRequest originalTransformationRequest) { this.mediaItem = mediaItem; this.transformerListeners = transformerListeners; + this.transformerListenerHandler = transformerListenerHandler; this.originalTransformationRequest = originalTransformationRequest; this.fallbackTransformationRequest = originalTransformationRequest; } @@ -98,15 +104,19 @@ import com.google.android.exoplayer2.util.Util; if (transformationRequest.hdrMode != originalTransformationRequest.hdrMode) { fallbackRequestBuilder.setHdrMode(transformationRequest.hdrMode); } - fallbackTransformationRequest = fallbackRequestBuilder.build(); + TransformationRequest newFallbackTransformationRequest = fallbackRequestBuilder.build(); + fallbackTransformationRequest = newFallbackTransformationRequest; if (trackCount == 0 && !originalTransformationRequest.equals(fallbackTransformationRequest)) { - transformerListeners.queueEvent( - /* eventFlag= */ C.INDEX_UNSET, - listener -> - listener.onFallbackApplied( - mediaItem, originalTransformationRequest, fallbackTransformationRequest)); - transformerListeners.flushEvents(); + transformerListenerHandler.post( + () -> + transformerListeners.sendEvent( + /* eventFlag= */ C.INDEX_UNSET, + listener -> + listener.onFallbackApplied( + mediaItem, + originalTransformationRequest, + newFallbackTransformationRequest))); } } } diff --git a/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/Transformer.java b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/Transformer.java index 340d4b4254..6b21dc6707 100644 --- a/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/Transformer.java +++ b/library/transformer/src/main/java/com/google/android/exoplayer2/transformer/Transformer.java @@ -752,7 +752,11 @@ public final class Transformer { TransformerInternalListener transformerInternalListener = new TransformerInternalListener(mediaItem); FallbackListener fallbackListener = - new FallbackListener(mediaItem, listeners, transformationRequest); + new FallbackListener( + mediaItem, + listeners, + clock.createHandler(looper, /* callback= */ null), + transformationRequest); transformerInternal = new TransformerInternal( context, diff --git a/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/FallbackListenerTest.java b/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/FallbackListenerTest.java index 60a26756b3..80aa79b069 100644 --- a/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/FallbackListenerTest.java +++ b/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/FallbackListenerTest.java @@ -26,10 +26,12 @@ import android.os.Looper; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.MediaItem; import com.google.android.exoplayer2.util.Clock; +import com.google.android.exoplayer2.util.HandlerWrapper; import com.google.android.exoplayer2.util.ListenerSet; import com.google.android.exoplayer2.util.MimeTypes; import org.junit.Test; import org.junit.runner.RunWith; +import org.robolectric.shadows.ShadowLooper; /** Unit tests for {@link FallbackListener}. */ @RunWith(AndroidJUnit4.class) @@ -41,7 +43,8 @@ public class FallbackListenerTest { public void onTransformationRequestFinalized_withoutTrackRegistration_throwsException() { TransformationRequest transformationRequest = new TransformationRequest.Builder().build(); FallbackListener fallbackListener = - new FallbackListener(PLACEHOLDER_MEDIA_ITEM, createListenerSet(), transformationRequest); + new FallbackListener( + PLACEHOLDER_MEDIA_ITEM, createListenerSet(), createHandler(), transformationRequest); assertThrows( IllegalStateException.class, @@ -52,10 +55,12 @@ public class FallbackListenerTest { public void onTransformationRequestFinalized_afterTrackRegistration_completesSuccessfully() { TransformationRequest transformationRequest = new TransformationRequest.Builder().build(); FallbackListener fallbackListener = - new FallbackListener(PLACEHOLDER_MEDIA_ITEM, createListenerSet(), transformationRequest); + new FallbackListener( + PLACEHOLDER_MEDIA_ITEM, createListenerSet(), createHandler(), transformationRequest); fallbackListener.registerTrack(); fallbackListener.onTransformationRequestFinalized(transformationRequest); + ShadowLooper.idleMainLooper(); } @Test @@ -66,10 +71,14 @@ public class FallbackListenerTest { Transformer.Listener mockListener = mock(Transformer.Listener.class); FallbackListener fallbackListener = new FallbackListener( - PLACEHOLDER_MEDIA_ITEM, createListenerSet(mockListener), originalRequest); + PLACEHOLDER_MEDIA_ITEM, + createListenerSet(mockListener), + createHandler(), + originalRequest); fallbackListener.registerTrack(); fallbackListener.onTransformationRequestFinalized(unchangedRequest); + ShadowLooper.idleMainLooper(); verify(mockListener, never()).onFallbackApplied(any(), any(), any()); } @@ -83,10 +92,14 @@ public class FallbackListenerTest { Transformer.Listener mockListener = mock(Transformer.Listener.class); FallbackListener fallbackListener = new FallbackListener( - PLACEHOLDER_MEDIA_ITEM, createListenerSet(mockListener), originalRequest); + PLACEHOLDER_MEDIA_ITEM, + createListenerSet(mockListener), + createHandler(), + originalRequest); fallbackListener.registerTrack(); fallbackListener.onTransformationRequestFinalized(audioFallbackRequest); + ShadowLooper.idleMainLooper(); verify(mockListener) .onFallbackApplied(PLACEHOLDER_MEDIA_ITEM, originalRequest, audioFallbackRequest); @@ -109,12 +122,16 @@ public class FallbackListenerTest { Transformer.Listener mockListener = mock(Transformer.Listener.class); FallbackListener fallbackListener = new FallbackListener( - PLACEHOLDER_MEDIA_ITEM, createListenerSet(mockListener), originalRequest); + PLACEHOLDER_MEDIA_ITEM, + createListenerSet(mockListener), + createHandler(), + originalRequest); fallbackListener.registerTrack(); fallbackListener.registerTrack(); fallbackListener.onTransformationRequestFinalized(audioFallbackRequest); fallbackListener.onTransformationRequestFinalized(videoFallbackRequest); + ShadowLooper.idleMainLooper(); verify(mockListener) .onFallbackApplied(PLACEHOLDER_MEDIA_ITEM, originalRequest, mergedFallbackRequest); @@ -130,4 +147,8 @@ public class FallbackListenerTest { private static ListenerSet createListenerSet() { return new ListenerSet<>(Looper.myLooper(), Clock.DEFAULT, (listener, flags) -> {}); } + + private static HandlerWrapper createHandler() { + return Clock.DEFAULT.createHandler(Looper.myLooper(), /* callback= */ null); + } } diff --git a/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/VideoEncoderWrapperTest.java b/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/VideoEncoderWrapperTest.java index 58a10c6380..0103f1e385 100644 --- a/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/VideoEncoderWrapperTest.java +++ b/library/transformer/src/test/java/com/google/android/exoplayer2/transformer/VideoEncoderWrapperTest.java @@ -50,6 +50,7 @@ public final class VideoEncoderWrapperTest { new FallbackListener( MediaItem.fromUri(Uri.EMPTY), new ListenerSet<>(Looper.myLooper(), Clock.DEFAULT, (listener, flags) -> {}), + Clock.DEFAULT.createHandler(Looper.myLooper(), /* callback= */ null), emptyTransformationRequest); private final VideoTranscodingSamplePipeline.EncoderWrapper encoderWrapper = new VideoTranscodingSamplePipeline.EncoderWrapper(