Skip to content

Commit

Permalink
Send pending updates before adding discontinuity for error
Browse files Browse the repository at this point in the history
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 79b688e.

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: #1483
PiperOrigin-RevId: 646968309
(cherry picked from commit 7276451)
  • Loading branch information
tonihei authored and tianyif committed Jul 2, 2024
1 parent 708dc6c commit adeef60
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 0 deletions.
6 changes: 6 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,8 @@ public boolean handleMessage(Message msg) {
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14052,6 +14052,94 @@ protected boolean shouldProcessBuffer(
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 {
Expand Down

0 comments on commit adeef60

Please sign in to comment.