Skip to content

Commit

Permalink
Fix delayed discontinuity for failed ad insertions.
Browse files Browse the repository at this point in the history 
We have logic to not immediately interrupt playback when an ad group
fails to load and instead let the current content play and transition
at the point where the ad group should have been.

This logic was broken by dcbdbe5 because of one of the conditions
used MediaPeriodId.adGroupIndex, which is always -1 for content ids.
It still worked for the last ad group because the next ad group index
was C.INDEX_UNSET.

Fix the issue and amend the test that was meant to catch this to test
the ad failures for the last ad and previous ads.

Also fix the PositionInfo reported in such a case, which was also wrong.

Issue: google/ExoPlayer#9929

#minor-release

PiperOrigin-RevId: 427143223
  • Loading branch information
@tonihei @icbaker
tonihei authored and icbaker committed Feb 21, 2022
1 parent 6cf571a commit 09ede4a
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 28 deletions.
13 changes: 7 additions & 6 deletions libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1932,21 +1932,22 @@ private PositionInfo getPreviousPositionInfo(
long oldPositionUs;
long oldContentPositionUs;
if (positionDiscontinuityReason == DISCONTINUITY_REASON_AUTO_TRANSITION) {
oldPositionUs = oldPeriod.positionInWindowUs + oldPeriod.durationUs;
oldContentPositionUs = oldPositionUs;
if (oldPlaybackInfo.periodId.isAd()) {
// The old position is the end of the previous ad.
oldPositionUs =
oldPeriod.getAdDurationUs(
oldPlaybackInfo.periodId.adGroupIndex, oldPlaybackInfo.periodId.adIndexInAdGroup);
// The ad cue point is stored in the old requested content position.
oldContentPositionUs = getRequestedContentPositionUs(oldPlaybackInfo);
} else if (oldPlaybackInfo.periodId.nextAdGroupIndex != C.INDEX_UNSET
&& playbackInfo.periodId.isAd()) {
// If it's a transition from content to an ad in the same window, the old position is the
// ad cue point that is the same as current content position.
} else if (oldPlaybackInfo.periodId.nextAdGroupIndex != C.INDEX_UNSET) {
// The old position is the end of a clipped content before an ad group. Use the exact ad
// cue point as the transition position.
oldPositionUs = getRequestedContentPositionUs(playbackInfo);
oldContentPositionUs = oldPositionUs;
} else {
// The old position is the end of a Timeline period. Use the exact duration.
oldPositionUs = oldPeriod.positionInWindowUs + oldPeriod.durationUs;
oldContentPositionUs = oldPositionUs;
}
} else if (oldPlaybackInfo.periodId.isAd()) {
oldPositionUs = oldPlaybackInfo.positionUs;
Expand Down
2 changes: 1 addition & 1 deletion libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2672,7 +2672,7 @@ private static PositionUpdateForPlaylistChange resolvePositionForPlaylistChange(
boolean earliestCuePointIsUnchangedOrLater =
periodIdWithAds.nextAdGroupIndex == C.INDEX_UNSET
|| (oldPeriodId.nextAdGroupIndex != C.INDEX_UNSET
&& periodIdWithAds.adGroupIndex >= oldPeriodId.nextAdGroupIndex);
&& periodIdWithAds.nextAdGroupIndex >= oldPeriodId.nextAdGroupIndex);
// Drop update if we keep playing the same content (MediaPeriod.periodUid are identical) and
// the only change is that MediaPeriodId.nextAdGroupIndex increased. This postpones a potential
// discontinuity until we reach the former next ad group position.
Expand Down
179 changes: 158 additions & 21 deletions libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ public void shuffleModeEnabledChanges() throws Exception {
}

@Test
public void adGroupWithLoadErrorIsSkipped() throws Exception {
public void adGroupWithLoadError_noFurtherAdGroup_isSkipped() throws Exception {
AdPlaybackState initialAdPlaybackState =
FakeTimeline.createAdPlaybackState(
/* adsPerAdGroup= */ 1, /* adGroupTimesUs...= */
Expand All @@ -745,11 +745,12 @@ public void adGroupWithLoadErrorIsSkipped() throws Exception {
/* isDynamic= */ false,
/* isLive= */ false,
/* isPlaceholder= */ false,
/* durationUs= */ C.MICROS_PER_SECOND,
/* durationUs= */ 10 * C.MICROS_PER_SECOND,
/* defaultPositionUs= */ 0,
TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US,
initialAdPlaybackState));
AdPlaybackState errorAdPlaybackState = initialAdPlaybackState.withAdLoadError(0, 0);
AdPlaybackState errorAdPlaybackState =
initialAdPlaybackState.withAdLoadError(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0);
final Timeline adErrorTimeline =
new FakeTimeline(
new TimelineWindowDefinition(
Expand All @@ -759,30 +760,166 @@ public void adGroupWithLoadErrorIsSkipped() throws Exception {
/* isDynamic= */ false,
/* isLive= */ false,
/* isPlaceholder= */ false,
/* durationUs= */ C.MICROS_PER_SECOND,
/* durationUs= */ 10 * C.MICROS_PER_SECOND,
/* defaultPositionUs= */ 0,
TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US,
errorAdPlaybackState));
final FakeMediaSource fakeMediaSource =
new FakeMediaSource(fakeTimeline, ExoPlayerTestRunner.VIDEO_FORMAT);
ActionSchedule actionSchedule =
new ActionSchedule.Builder(TAG)
.pause()
.waitForPlaybackState(Player.STATE_READY)
.executeRunnable(() -> fakeMediaSource.setNewSourceInfo(adErrorTimeline))
.waitForTimelineChanged(
adErrorTimeline, /* expectedReason */ Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE)
.play()
.build();
ExoPlayerTestRunner testRunner =
new ExoPlayerTestRunner.Builder(context)
.setMediaSources(fakeMediaSource)
.setActionSchedule(actionSchedule)
.build()
.start()
.blockUntilEnded(TIMEOUT_MS);
ExoPlayer player = new TestExoPlayerBuilder(context).build();
Player.Listener mockListener = mock(Player.Listener.class);
player.addListener(mockListener);

player.setMediaSource(fakeMediaSource);
player.prepare();
runUntilPlaybackState(player, Player.STATE_READY);
fakeMediaSource.setNewSourceInfo(adErrorTimeline);
player.play();
runUntilPlaybackState(player, Player.STATE_ENDED);
Timeline.Window window =
player.getCurrentTimeline().getWindow(/* windowIndex= */ 0, new Timeline.Window());
Timeline.Period period =
player
.getCurrentTimeline()
.getPeriod(/* periodIndex= */ 0, new Timeline.Period(), /* setIds= */ true);
player.release();

// There is still one discontinuity from content to content for the failed ad insertion.
testRunner.assertPositionDiscontinuityReasonsEqual(Player.DISCONTINUITY_REASON_AUTO_TRANSITION);
PositionInfo positionInfo =
new PositionInfo(
window.uid,
/* mediaItemIndex= */ 0,
window.mediaItem,
period.uid,
/* periodIndex= */ 0,
/* positionMs= */ 5_000,
/* contentPositionMs= */ 5_000,
/* adGroupIndex= */ C.INDEX_UNSET,
/* adIndexInAdGroup= */ C.INDEX_UNSET);
verify(mockListener)
.onPositionDiscontinuity(
positionInfo, positionInfo, Player.DISCONTINUITY_REASON_AUTO_TRANSITION);
}

@Test
public void adGroupWithLoadError_withFurtherAdGroup_isSkipped() throws Exception {
AdPlaybackState initialAdPlaybackState =
FakeTimeline.createAdPlaybackState(
/* adsPerAdGroup= */ 1, /* adGroupTimesUs...= */
TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US
+ 5 * C.MICROS_PER_SECOND,
TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US
+ 8 * C.MICROS_PER_SECOND);
Timeline fakeTimeline =
new FakeTimeline(
new TimelineWindowDefinition(
/* periodCount= */ 1,
/* id= */ 0,
/* isSeekable= */ true,
/* isDynamic= */ false,
/* isLive= */ false,
/* isPlaceholder= */ false,
/* durationUs= */ 10 * C.MICROS_PER_SECOND,
/* defaultPositionUs= */ 0,
TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US,
initialAdPlaybackState));
AdPlaybackState errorAdPlaybackState =
initialAdPlaybackState.withAdLoadError(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0);
final Timeline adErrorTimeline =
new FakeTimeline(
new TimelineWindowDefinition(
/* periodCount= */ 1,
/* id= */ 0,
/* isSeekable= */ true,
/* isDynamic= */ false,
/* isLive= */ false,
/* isPlaceholder= */ false,
/* durationUs= */ 10 * C.MICROS_PER_SECOND,
/* defaultPositionUs= */ 0,
TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US,
errorAdPlaybackState));
final FakeMediaSource fakeMediaSource =
new FakeMediaSource(fakeTimeline, ExoPlayerTestRunner.VIDEO_FORMAT);
ExoPlayer player = new TestExoPlayerBuilder(context).build();
Player.Listener mockListener = mock(Player.Listener.class);
player.addListener(mockListener);

player.setMediaSource(fakeMediaSource);
player.prepare();
runUntilPlaybackState(player, Player.STATE_READY);
fakeMediaSource.setNewSourceInfo(adErrorTimeline);
player.play();
runUntilPlaybackState(player, Player.STATE_ENDED);
Timeline.Window window =
player.getCurrentTimeline().getWindow(/* windowIndex= */ 0, new Timeline.Window());
Timeline.Period period =
player
.getCurrentTimeline()
.getPeriod(/* periodIndex= */ 0, new Timeline.Period(), /* setIds= */ true);
player.release();

// There is still one discontinuity from content to content for the failed ad insertion and the
// normal ad transition for the successful ad insertion.
PositionInfo positionInfoFailedAd =
new PositionInfo(
window.uid,
/* mediaItemIndex= */ 0,
window.mediaItem,
period.uid,
/* periodIndex= */ 0,
/* positionMs= */ 5_000,
/* contentPositionMs= */ 5_000,
/* adGroupIndex= */ C.INDEX_UNSET,
/* adIndexInAdGroup= */ C.INDEX_UNSET);
verify(mockListener)
.onPositionDiscontinuity(
positionInfoFailedAd,
positionInfoFailedAd,
Player.DISCONTINUITY_REASON_AUTO_TRANSITION);
PositionInfo positionInfoContentAtSuccessfulAd =
new PositionInfo(
window.uid,
/* mediaItemIndex= */ 0,
window.mediaItem,
period.uid,
/* periodIndex= */ 0,
/* positionMs= */ 8_000,
/* contentPositionMs= */ 8_000,
/* adGroupIndex= */ C.INDEX_UNSET,
/* adIndexInAdGroup= */ C.INDEX_UNSET);
PositionInfo positionInfoSuccessfulAdStart =
new PositionInfo(
window.uid,
/* mediaItemIndex= */ 0,
window.mediaItem,
period.uid,
/* periodIndex= */ 0,
/* positionMs= */ 0,
/* contentPositionMs= */ 8_000,
/* adGroupIndex= */ 1,
/* adIndexInAdGroup= */ 0);
PositionInfo positionInfoSuccessfulAdEnd =
new PositionInfo(
window.uid,
/* mediaItemIndex= */ 0,
window.mediaItem,
period.uid,
/* periodIndex= */ 0,
/* positionMs= */ Util.usToMs(
period.getAdDurationUs(/* adGroupIndex= */ 1, /* adIndexInAdGroup= */ 0)),
/* contentPositionMs= */ 8_000,
/* adGroupIndex= */ 1,
/* adIndexInAdGroup= */ 0);
verify(mockListener)
.onPositionDiscontinuity(
positionInfoContentAtSuccessfulAd,
positionInfoSuccessfulAdStart,
Player.DISCONTINUITY_REASON_AUTO_TRANSITION);
verify(mockListener)
.onPositionDiscontinuity(
positionInfoSuccessfulAdEnd,
positionInfoContentAtSuccessfulAd,
Player.DISCONTINUITY_REASON_AUTO_TRANSITION);
}

@Test
Expand Down

0 comments on commit 09ede4a

Please sign in to comment.