From d7163534ff7c47dcc07bcf5c180d7fcb2f0d5790 Mon Sep 17 00:00:00 2001 From: michaelkatz Date: Wed, 5 Mar 2025 08:30:23 -0800 Subject: [PATCH] Use ExoPlaybackException.MediaPeriodId in prewarming error handling With using the `MediaPeriodId` from the `ExoPlaybackException`, ExoPlayerImplInternal can set the same id to the renderer group(primary and secondary). Tested with `ExoPlayerWithPrewarmingRenderersTest` error-based unit tests like `play_errorByPrewarmingSecondaryRendererAfterAdvancingReadingPeriod_doesNotResetPrimaryRenderer`. If the media period was not set in BaseRenderer then the exception would have the incorrect mediaperiod, attribute the error to the non-prewarming renderer, and the test would fail. PiperOrigin-RevId: 733744777 --- .../exoplayer/ExoPlayerImplInternal.java | 22 ++++++++++--------- .../media3/exoplayer/RendererHolder.java | 21 +++++++++++++----- 2 files changed, 27 insertions(+), 16 deletions(-) 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 56c753f188..e355fc08e5 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java @@ -311,7 +311,7 @@ import java.util.concurrent.atomic.AtomicBoolean; rendererCapabilities[i].setListener(rendererCapabilitiesListener); } if (secondaryRenderers[i] != null) { - secondaryRenderers[i].init(/* index= */ i + renderers.length, playerId, clock); + secondaryRenderers[i].init(/* index= */ i, playerId, clock); hasSecondaryRenderers = true; } this.renderers[i] = new RendererHolder(renderers[i], secondaryRenderers[i], /* index= */ i); @@ -741,13 +741,7 @@ import java.util.concurrent.atomic.AtomicBoolean; if (readingPeriod != null && e.mediaPeriodId == null) { // We can assume that all renderer errors happen in the context of the reading period. See // [internal: b/150584930#comment4] for exceptions that aren't covered by this assumption. - e = - e.copyWithMediaPeriodId( - (renderers[e.rendererIndex % renderers.length].isRendererPrewarming( - e.rendererIndex) - && readingPeriod.getNext() != null) - ? readingPeriod.getNext().info.id - : readingPeriod.info.id); + e = e.copyWithMediaPeriodId(readingPeriod.info.id); } } if (e.isRecoverable @@ -769,8 +763,8 @@ import java.util.concurrent.atomic.AtomicBoolean; handler.sendMessageAtFrontOfQueue( handler.obtainMessage(MSG_ATTEMPT_RENDERER_ERROR_RECOVERY, e)); } else if (e.type == ExoPlaybackException.TYPE_RENDERER - && renderers[e.rendererIndex % renderers.length].isRendererPrewarming( - /* id= */ e.rendererIndex)) { + && e.mediaPeriodId != null + && isRendererPrewarmingMediaPeriod(e.rendererIndex, e.mediaPeriodId)) { // TODO(b/380273486): Investigate recovery for pre-warming renderer errors isPrewarmingDisabledUntilNextTransition = true; disableAndResetPrewarmingRenderers(); @@ -2080,6 +2074,14 @@ import java.util.concurrent.atomic.AtomicBoolean; prewarmingMediaPeriodDiscontinuity = C.TIME_UNSET; } + private boolean isRendererPrewarmingMediaPeriod(int rendererIndex, MediaPeriodId mediaPeriodId) { + if (queue.getPrewarmingPeriod() == null + || !queue.getPrewarmingPeriod().info.id.equals(mediaPeriodId)) { + return false; + } + return renderers[rendererIndex].isPrewarmingPeriod(queue.getPrewarmingPeriod()); + } + private void reselectTracksInternalAndSeek() throws ExoPlaybackException { reselectTracksInternal(); seekToCurrentPosition(/* sendDiscontinuity= */ true); 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 dd23518ccb..756c881b2a 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/RendererHolder.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/RendererHolder.java @@ -82,12 +82,6 @@ import java.util.Objects; return isPrimaryRendererPrewarming() || isSecondaryRendererPrewarming(); } - public boolean isRendererPrewarming(int id) { - boolean isPrewarmingPrimaryRenderer = isPrimaryRendererPrewarming() && id == index; - boolean isPrewarmingSecondaryRenderer = isSecondaryRendererPrewarming() && id != index; - return isPrewarmingPrimaryRenderer || isPrewarmingSecondaryRenderer; - } - private boolean isPrimaryRendererPrewarming() { return prewarmingState == RENDERER_PREWARMING_STATE_PREWARMING_PRIMARY || prewarmingState == RENDERER_PREWARMING_STATE_TRANSITIONING_TO_PRIMARY; @@ -317,6 +311,21 @@ import java.util.Objects; return getRendererReadingFromPeriod(period) != null; } + /** + * Returns whether a {@link Renderer} is prewarming and enabled on a {@link MediaPeriodHolder + * media period}. + * + * @param period The {@link MediaPeriodHolder media period} to check. + */ + public boolean isPrewarmingPeriod(MediaPeriodHolder period) { + boolean isPrimaryRendererPrewarming = + isPrimaryRendererPrewarming() && getRendererReadingFromPeriod(period) == primaryRenderer; + boolean isSecondaryRendererPrewarming = + isSecondaryRendererPrewarming() + && getRendererReadingFromPeriod(period) == secondaryRenderer; + return isPrimaryRendererPrewarming || isSecondaryRendererPrewarming; + } + /** * Returns whether the {@link Renderer renderers} are still reading a {@link MediaPeriodHolder * media period}.