From adeef60c140dfc612ad0df61c7441b2e2a8df602 Mon Sep 17 00:00:00 2001 From: tonihei Date: Wed, 26 Jun 2024 09:06:20 -0700 Subject: [PATCH] Send pending updates before adding discontinuity for error When handling a playback error that originates from a future item in the playlist, we added support for jumping to that item first, ensuring the errors 'happen' for the right 'current item'. See https://github.com/androidx/media/commit/79b688ef30d4a3306d4d321ddf4464b428074ea2. However, when we add this new position discontinuity to the playback state, there may already be other position discontinuities pending from other parts of the code that executed before the error. As we can't control that in this case (because it's part of a generic try/catch block), we need to send any pending updates first before handling the new change. Issue: androidx/media#1483 PiperOrigin-RevId: 646968309 (cherry picked from commit 727645179b3c26e495540f4445216fd035cc7654) --- RELEASENOTES.md | 6 ++ .../exoplayer/ExoPlayerImplInternal.java | 2 + .../media3/exoplayer/ExoPlayerTest.java | 88 +++++++++++++++++++ 3 files changed, 96 insertions(+) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 43bc040a42..eaf56bab1a 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -12,6 +12,12 @@ * Fix potential `IndexOutOfBoundsException` caused by extractors reporting additional tracks after the initial preparation step ([#1476](https://github.com/androidx/media/issues/1476)). + * `Effects` in `ExoPlayer.setVideoEffect()` will receive the timestamps + with the renderer offset removed + ([#1098](https://github.com/androidx/media/issues/1098)). + * Fix potential `IllegalArgumentException` when handling player error that + happened while reading ahead into another playlist item + ([#1483](https://github.com/androidx/media/issues/1483)). * Transformer: * Track Selection: * Extractors: 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 0b2bc58010..7ea8f7372e 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java @@ -672,6 +672,8 @@ import java.util.concurrent.atomic.AtomicBoolean; queue.advancePlayingPeriod(); } MediaPeriodHolder newPlayingPeriodHolder = checkNotNull(queue.getPlayingPeriod()); + // Send already pending updates if needed before making further changes to PlaybackInfo. + maybeNotifyPlaybackInfoChanged(); playbackInfo = handlePositionDiscontinuity( newPlayingPeriodHolder.info.id, 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 9a3384823c..ed2ea706c2 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java @@ -14052,6 +14052,94 @@ public class ExoPlayerTest { eventsInOrder.verify(mockListener).onPlayerError(any(), any()); } + @Test + public void + rendererError_whileReadingAheadAndOverlappingWithOtherDiscontinuity_isReportedAfterMediaItemTransition() + throws Exception { + // Throw an exception as soon as we try to process a buffer for the second item. This happens + // while the player is still playing the first item. At the same time, also constantly report + // silence skip to force a situation where we have already a pending discontinuity at the point + // of handling the error. + AtomicInteger silenceSkipCount = new AtomicInteger(0); + FakeMediaClockRenderer fakeRenderer = + new FakeMediaClockRenderer(C.TRACK_TYPE_AUDIO) { + private long positionUs; + int streamChangeCount = 0; + + @Override + protected void onStreamChanged( + Format[] formats, long startPositionUs, long offsetUs, MediaPeriodId mediaPeriodId) { + positionUs = startPositionUs; + streamChangeCount++; + } + + @Override + public long getPositionUs() { + // Advance position to make playback progress. + positionUs += 10_000; + return positionUs; + } + + @Override + public boolean hasSkippedSilenceSinceLastCall() { + // Continuously report skipped silences to ensure they will overlap with other + // discontinuities like the AUTO_TRANSITION triggered after the error. + silenceSkipCount.incrementAndGet(); + return true; + } + + @Override + protected boolean shouldProcessBuffer(long bufferTimeUs, long playbackPositionUs) { + boolean shouldProcess = super.shouldProcessBuffer(bufferTimeUs, playbackPositionUs); + if (streamChangeCount == 2 && shouldProcess) { + Util.sneakyThrow( + createRendererException( + new IllegalStateException(), + /* format= */ null, + PlaybackException.ERROR_CODE_DECODING_FAILED)); + } + return shouldProcess; + } + + @Override + public void setPlaybackParameters(PlaybackParameters playbackParameters) {} + + @Override + public PlaybackParameters getPlaybackParameters() { + return PlaybackParameters.DEFAULT; + } + }; + TestExoPlayerBuilder playerBuilder = + new TestExoPlayerBuilder(context).setRenderers(fakeRenderer); + ExoPlayer player = parameterizeTestExoPlayerBuilder(playerBuilder).build(); + Player.Listener mockListener = mock(Player.Listener.class); + player.addListener(mockListener); + + player.setMediaSources( + ImmutableList.of( + new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.AUDIO_FORMAT), + new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.AUDIO_FORMAT))); + player.prepare(); + player.play(); + runUntilError(player); + int mediaItemIndexAfterError = player.getCurrentMediaItemIndex(); + player.release(); + + assertThat(mediaItemIndexAfterError).isEqualTo(1); + InOrder eventsInOrder = inOrder(mockListener); + // Verify that all expected discontinuities and the error arrived in order. + eventsInOrder + .verify(mockListener, times(silenceSkipCount.get())) + .onPositionDiscontinuity(any(), any(), eq(Player.DISCONTINUITY_REASON_SILENCE_SKIP)); + eventsInOrder + .verify(mockListener) + .onPositionDiscontinuity(any(), any(), eq(Player.DISCONTINUITY_REASON_AUTO_TRANSITION)); + eventsInOrder + .verify(mockListener) + .onMediaItemTransition(any(), eq(Player.MEDIA_ITEM_TRANSITION_REASON_AUTO)); + eventsInOrder.verify(mockListener).onPlayerError(any()); + } + @Test public void play_withReadingAheadWithNewRenderer_enablesButNotStartsNewRenderer() throws Exception {