From f11130d59e6609305de36180d18aa5ff0965f7aa Mon Sep 17 00:00:00 2001 From: michaelkatz Date: Wed, 23 Oct 2024 07:13:57 -0700 Subject: [PATCH] Move setVideoOutput message handling to playback thread PiperOrigin-RevId: 688951964 --- .../media3/exoplayer/ExoPlayerImpl.java | 31 ++---------- .../exoplayer/ExoPlayerImplInternal.java | 48 +++++++++++++++++++ .../media3/exoplayer/RendererHolder.java | 7 +++ 3 files changed, 60 insertions(+), 26 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java index e76fa35f29..558777d075 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java @@ -35,7 +35,6 @@ import static androidx.media3.exoplayer.Renderer.MSG_SET_SCALING_MODE; import static androidx.media3.exoplayer.Renderer.MSG_SET_SKIP_SILENCE_ENABLED; import static androidx.media3.exoplayer.Renderer.MSG_SET_VIDEO_EFFECTS; import static androidx.media3.exoplayer.Renderer.MSG_SET_VIDEO_FRAME_METADATA_LISTENER; -import static androidx.media3.exoplayer.Renderer.MSG_SET_VIDEO_OUTPUT; import static androidx.media3.exoplayer.Renderer.MSG_SET_VIDEO_OUTPUT_RESOLUTION; import static androidx.media3.exoplayer.Renderer.MSG_SET_VOLUME; import static java.lang.Math.max; @@ -124,7 +123,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArraySet; -import java.util.concurrent.TimeoutException; /** The default implementation of {@link ExoPlayer}. */ /* package */ final class ExoPlayerImpl extends BasePlayer @@ -2702,31 +2700,12 @@ import java.util.concurrent.TimeoutException; } private void setVideoOutputInternal(@Nullable Object videoOutput) { + boolean isReplacingVideoOutput = this.videoOutput != null && this.videoOutput != videoOutput; + long timeoutMs = isReplacingVideoOutput ? detachSurfaceTimeoutMs : C.TIME_UNSET; // Note: We don't turn this method into a no-op if the output is being replaced with itself so // as to ensure onRenderedFirstFrame callbacks are still called in this case. - List messages = new ArrayList<>(); - for (Renderer renderer : renderers) { - if (renderer.getTrackType() == TRACK_TYPE_VIDEO) { - messages.add( - createMessageInternal(renderer) - .setType(MSG_SET_VIDEO_OUTPUT) - .setPayload(videoOutput) - .send()); - } - } - boolean messageDeliveryTimedOut = false; - if (this.videoOutput != null && this.videoOutput != videoOutput) { - // We're replacing an output. Block to ensure that this output will not be accessed by the - // renderers after this method returns. - try { - for (PlayerMessage message : messages) { - message.blockUntilDelivered(detachSurfaceTimeoutMs); - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } catch (TimeoutException e) { - messageDeliveryTimedOut = true; - } + boolean isSuccess = internalPlayer.setVideoOutput(videoOutput, timeoutMs); + if (isReplacingVideoOutput) { if (this.videoOutput == ownedSurface) { // We're replacing a surface that we are responsible for releasing. ownedSurface.release(); @@ -2734,7 +2713,7 @@ import java.util.concurrent.TimeoutException; } } this.videoOutput = videoOutput; - if (messageDeliveryTimedOut) { + if (!isSuccess) { stopInternal( ExoPlaybackException.createForUnexpected( new ExoTimeoutException(ExoTimeoutException.TIMEOUT_OPERATION_DETACH_SURFACE), diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java index 34fcffbb20..60081a621e 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java @@ -154,6 +154,7 @@ import java.util.concurrent.atomic.AtomicBoolean; private static final int MSG_UPDATE_MEDIA_SOURCES_WITH_MEDIA_ITEMS = 27; private static final int MSG_SET_PRELOAD_CONFIGURATION = 28; private static final int MSG_PREPARE = 29; + private static final int MSG_SET_VIDEO_OUTPUT = 30; private static final long BUFFERING_MAXIMUM_INTERVAL_MS = Util.usToMs(Renderer.DEFAULT_DURATION_TO_PROGRESS_US); @@ -465,6 +466,32 @@ import java.util.concurrent.atomic.AtomicBoolean; } } + /** + * Sets the video output. + * + *

If the provided {@code timeoutMs} is {@link C#TIME_UNSET} then this method will not wait on + * the message delivery. + * + * @param videoOutput Surface onto which which video will be rendered. + * @param timeoutMs Timeout duration to wait for successful message delivery. If {@link + * C#TIME_UNSET} then the method will not block on the message delivery. + * @return Whether the operation succeeded. If false, the operation timed out. + */ + public synchronized boolean setVideoOutput(@Nullable Object videoOutput, long timeoutMs) { + if (released || !playbackLooper.getThread().isAlive()) { + return true; + } + AtomicBoolean processedFlag = new AtomicBoolean(); + handler + .obtainMessage(MSG_SET_VIDEO_OUTPUT, new Pair<>(videoOutput, processedFlag)) + .sendToTarget(); + if (timeoutMs != C.TIME_UNSET) { + waitUninterruptibly(/* condition= */ processedFlag::get, timeoutMs); + return processedFlag.get(); + } + return true; + } + /** * Releases the player. * @@ -566,6 +593,12 @@ import java.util.concurrent.atomic.AtomicBoolean; setForegroundModeInternal( /* foregroundMode= */ msg.arg1 != 0, /* processedFlag= */ (AtomicBoolean) msg.obj); break; + case MSG_SET_VIDEO_OUTPUT: + Pair setVideoOutputPayload = (Pair) msg.obj; + setVideoOutputInternal( + /* videoOutput= */ setVideoOutputPayload.first, + /* processedFlag= */ setVideoOutputPayload.second); + break; case MSG_STOP: stopInternal(/* forceResetRenderers= */ false, /* acknowledgeStop= */ true); break; @@ -1513,6 +1546,21 @@ import java.util.concurrent.atomic.AtomicBoolean; } } + private void setVideoOutputInternal( + @Nullable Object videoOutput, @Nullable AtomicBoolean processedFlag) + throws ExoPlaybackException { + for (RendererHolder renderer : renderers) { + renderer.setVideoOutput(videoOutput); + } + + if (processedFlag != null) { + synchronized (this) { + processedFlag.set(true); + notifyAll(); + } + } + } + private void stopInternal(boolean forceResetRenderers, boolean acknowledgeStop) { resetInternal( /* resetRenderers= */ forceResetRenderers || !foregroundMode, diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/RendererHolder.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/RendererHolder.java index 136767a2ef..783310996f 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/RendererHolder.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/RendererHolder.java @@ -15,6 +15,7 @@ */ package androidx.media3.exoplayer; +import static androidx.media3.common.C.TRACK_TYPE_VIDEO; import static androidx.media3.exoplayer.Renderer.STATE_DISABLED; import static androidx.media3.exoplayer.Renderer.STATE_ENABLED; import static androidx.media3.exoplayer.Renderer.STATE_STARTED; @@ -440,6 +441,12 @@ import java.io.IOException; requiresReset = false; } + public void setVideoOutput(@Nullable Object videoOutput) throws ExoPlaybackException { + if (renderer.getTrackType() == TRACK_TYPE_VIDEO) { + renderer.handleMessage(Renderer.MSG_SET_VIDEO_OUTPUT, videoOutput); + } + } + private static boolean isRendererEnabled(Renderer renderer) { return renderer.getState() != STATE_DISABLED; }