From aea0637c9515db9e0c5319af0274ca3a1f84b9d5 Mon Sep 17 00:00:00 2001 From: michaelkatz Date: Thu, 27 Mar 2025 11:01:59 -0700 Subject: [PATCH] Fix media period mismatch during recoverable renderer error processing If a recoverable renderer error occurred just before playing period transition(aka media item transition) then the player may enter a continuous loop of retrying to play the previous media item. This was most easily reproduced in an audio offload scenario where init in offload mode always fails. In initializing the following media, the process would fail with recoverable error to try in non-offload mode. The player would try to recover with playing the previous media item. Most times it would skip to the next track but not always. Issue: androidx/media#2229 PiperOrigin-RevId: 741213293 --- .../exoplayer/ExoPlayerImplInternal.java | 43 +++++----- .../media3/exoplayer/ExoPlayerTest.java | 79 +++++++++++++++++++ 2 files changed, 100 insertions(+), 22 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 135d14d2f7..3ff1963035 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java @@ -769,34 +769,17 @@ import java.util.concurrent.atomic.AtomicBoolean; maybeContinueLoading(); handler.sendEmptyMessage(MSG_DO_SOME_WORK); } - } else if (e.isRecoverable - && (pendingRecoverableRendererError == null - || e.errorCode == PlaybackException.ERROR_CODE_AUDIO_TRACK_OFFLOAD_INIT_FAILED - || e.errorCode == PlaybackException.ERROR_CODE_AUDIO_TRACK_OFFLOAD_WRITE_FAILED)) { - // If pendingRecoverableRendererError != null and error was - // ERROR_CODE_AUDIO_TRACK_OFFLOAD_WRITE_FAILED then upon retry, renderer will attempt with - // offload disabled. - Log.w(TAG, "Recoverable renderer error", e); - if (pendingRecoverableRendererError != null) { - pendingRecoverableRendererError.addSuppressed(e); - e = pendingRecoverableRendererError; - } else { - pendingRecoverableRendererError = e; - } - // Given that the player is now in an unhandled exception state, the error needs to be - // recovered or the player stopped before any other message is handled. - handler.sendMessageAtFrontOfQueue( - handler.obtainMessage(MSG_ATTEMPT_RENDERER_ERROR_RECOVERY, e)); } else { if (pendingRecoverableRendererError != null) { pendingRecoverableRendererError.addSuppressed(e); e = pendingRecoverableRendererError; } - Log.e(TAG, "Playback error", e); + if (e.type == ExoPlaybackException.TYPE_RENDERER && queue.getPlayingPeriod() != queue.getReadingPeriod()) { // We encountered a renderer error while reading ahead. Force-update the playback position - // to the failing item to ensure the user-visible error is reported after the transition. + // to the failing item to ensure correct retry or that the user-visible error is reported + // after the transition. while (queue.getPlayingPeriod() != queue.getReadingPeriod()) { queue.advancePlayingPeriod(); } @@ -812,8 +795,24 @@ import java.util.concurrent.atomic.AtomicBoolean; /* reportDiscontinuity= */ true, Player.DISCONTINUITY_REASON_AUTO_TRANSITION); } - stopInternal(/* forceResetRenderers= */ true, /* acknowledgeStop= */ false); - playbackInfo = playbackInfo.copyWithPlaybackError(e); + + if (e.isRecoverable + && (pendingRecoverableRendererError == null + || e.errorCode == PlaybackException.ERROR_CODE_AUDIO_TRACK_OFFLOAD_INIT_FAILED + || e.errorCode == PlaybackException.ERROR_CODE_AUDIO_TRACK_OFFLOAD_WRITE_FAILED)) { + // Given that the player is now in an unhandled exception state, the error needs to be + // recovered or the player stopped before any other message is handled. + Log.w(TAG, "Recoverable renderer error", e); + if (pendingRecoverableRendererError == null) { + pendingRecoverableRendererError = e; + } + handler.sendMessageAtFrontOfQueue( + handler.obtainMessage(MSG_ATTEMPT_RENDERER_ERROR_RECOVERY, e)); + } else { + Log.e(TAG, "Playback error", e); + stopInternal(/* forceResetRenderers= */ true, /* acknowledgeStop= */ false); + playbackInfo = playbackInfo.copyWithPlaybackError(e); + } } } catch (DrmSession.DrmSessionException e) { handleIoException(e, e.errorCode); diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java index d56c1c23ce..bd76c21760 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java @@ -147,6 +147,7 @@ import androidx.media3.exoplayer.analytics.PlayerId; import androidx.media3.exoplayer.audio.AudioRendererEventListener; import androidx.media3.exoplayer.drm.DrmSessionEventListener; import androidx.media3.exoplayer.drm.DrmSessionManager; +import androidx.media3.exoplayer.mediacodec.MediaCodecRenderer; import androidx.media3.exoplayer.metadata.MetadataOutput; import androidx.media3.exoplayer.source.ClippingMediaSource; import androidx.media3.exoplayer.source.ConcatenatingMediaSource; @@ -16748,6 +16749,84 @@ public class ExoPlayerTest { assertThat(videoScalingSetOnSecondaryVideoRenderer.get()).isTrue(); } + @Test + public void + play_withRecoverableErrorAfterAdvancingReadingPeriod_advancesPlayingPeriodWhileErrorHandling() + throws Exception { + Clock fakeClock = new FakeClock(/* isAutoAdvancing= */ true); + AtomicBoolean shouldRendererThrowRecoverableError = new AtomicBoolean(false); + AtomicInteger onStreamChangedCount = new AtomicInteger(0); + ExoPlayer player = + new TestExoPlayerBuilder(context) + .setClock(fakeClock) + .setRenderersFactory( + new RenderersFactory() { + @Override + public Renderer[] createRenderers( + Handler eventHandler, + VideoRendererEventListener videoRendererEventListener, + AudioRendererEventListener audioRendererEventListener, + TextOutput textRendererOutput, + MetadataOutput metadataRendererOutput) { + return new Renderer[] { + new FakeVideoRenderer( + SystemClock.DEFAULT.createHandler( + eventHandler.getLooper(), /* callback= */ null), + videoRendererEventListener) { + @Override + protected void onStreamChanged( + Format[] formats, + long startPositionUs, + long offsetUs, + MediaSource.MediaPeriodId mediaPeriodId) + throws ExoPlaybackException { + super.onStreamChanged(formats, startPositionUs, offsetUs, mediaPeriodId); + onStreamChangedCount.getAndIncrement(); + } + + @Override + public void render(long positionUs, long elapsedRealtimeUs) + throws ExoPlaybackException { + if (!shouldRendererThrowRecoverableError.get()) { + super.render(positionUs, elapsedRealtimeUs); + } else { + shouldRendererThrowRecoverableError.set(false); + throw createRendererException( + new MediaCodecRenderer.DecoderInitializationException( + new Format.Builder().build(), + new IllegalArgumentException(), + false, + 0), + this.getFormatHolder().format, + true, + PlaybackException.ERROR_CODE_DECODER_INIT_FAILED); + } + } + } + }; + } + }) + .build(); + player.setMediaSources( + ImmutableList.of( + new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.VIDEO_FORMAT), + new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.VIDEO_FORMAT))); + player.prepare(); + + // Play a bit until the reading period has advanced. + player.play(); + advance(player).untilBackgroundThreadCondition(() -> onStreamChangedCount.get() == 2); + shouldRendererThrowRecoverableError.set(true); + runUntilPlaybackState(player, Player.STATE_ENDED); + + player.release(); + + // onStreamChanged should occur thrice; + // 1 during first enable, 2 during replace stream, 3 during error recovery + assertThat(onStreamChangedCount.get()).isEqualTo(3); + assertThat(shouldRendererThrowRecoverableError.get()).isFalse(); + } + // Internal methods. private void addWatchAsSystemFeature() {