Misc ad handling improvements.

1. Clarify intention of getAdGroupIndexForPositionUs and
   getAdGroupIndexAfterPositionUs. Both methods are used for very
   specific but different purposes and encode the logic of which
   ads should be played at which time, so it's helpful to clarify
   this in the documentation as well.
2. Change one usage getAdGroupIndexForPositionUs to use the already
   existing nextAdGroupIndex. This is also more in line with the
   intended usage as clarified in step 1.
3. Update MediaPeriodQueueTest for updateQueuedPeriods to only
   look for duration changes in future periods, not in the
   current one, because that's not handled MediaPeriodQueue for ads
   and the test is just passing by chance now. Also remove wrong
   advancePlaying() calls that are already implicitly included in
   the preceding enqueueNext() call.
4. Fix a minor bug where post-roll ads are not checked whether they
   are played already before using them as the next ad group. Also
   added a test covering this case.

#minor-release

PiperOrigin-RevId: 370664131
This commit is contained in:
tonihei 2021-04-27 14:15:24 +01:00 committed by bachinger
parent 13a34b8b4a
commit 26a6aad3e4
4 changed files with 88 additions and 26 deletions

View file

@ -752,9 +752,10 @@ public abstract class Timeline implements Bundleable {
}
/**
* Returns the index of the ad group at or before {@code positionUs} in the period, if that ad
* group is unplayed. Returns {@link C#INDEX_UNSET} if the ad group at or before {@code
* positionUs} has no ads remaining to be played, or if there is no such ad group.
* Returns the index of the ad group at or before {@code positionUs} in the period that should
* be played before the content at {@code positionUs}. Returns {@link C#INDEX_UNSET} if the ad
* group at or before {@code positionUs} has no ads remaining to be played, or if there is no
* such ad group.
*
* @param positionUs The period position at or before which to find an ad group, in
* microseconds.
@ -766,7 +767,7 @@ public abstract class Timeline implements Bundleable {
/**
* Returns the index of the next ad group after {@code positionUs} in the period that has ads
* remaining to be played. Returns {@link C#INDEX_UNSET} if there is no such ad group.
* that should be played. Returns {@link C#INDEX_UNSET} if there is no such ad group.
*
* @param positionUs The period position after which to find an ad group, in microseconds.
* @return The index of the ad group, or {@link C#INDEX_UNSET}.

View file

@ -380,9 +380,9 @@ public final class AdPlaybackState implements Bundleable {
}
/**
* Returns the index of the ad group at or before {@code positionUs}, if that ad group is
* unplayed. Returns {@link C#INDEX_UNSET} if the ad group at or before {@code positionUs} has no
* ads remaining to be played, or if there is no such ad group.
* Returns the index of the ad group at or before {@code positionUs} that should be played before
* the content at {@code positionUs}. Returns {@link C#INDEX_UNSET} if the ad group at or before
* {@code positionUs} has no ads remaining to be played, or if there is no such ad group.
*
* @param positionUs The period position at or before which to find an ad group, in microseconds,
* or {@link C#TIME_END_OF_SOURCE} for the end of the stream (in which case the index of any
@ -402,8 +402,8 @@ public final class AdPlaybackState implements Bundleable {
}
/**
* Returns the index of the next ad group after {@code positionUs} that has ads remaining to be
* played. Returns {@link C#INDEX_UNSET} if there is no such ad group.
* Returns the index of the next ad group after {@code positionUs} that should be played. Returns
* {@link C#INDEX_UNSET} if there is no such ad group.
*
* @param positionUs The period position after which to find an ad group, in microseconds, or
* {@link C#TIME_END_OF_SOURCE} for the end of the stream (in which case there can be no ad
@ -421,8 +421,8 @@ public final class AdPlaybackState implements Bundleable {
// In practice we expect there to be few ad groups so the search shouldn't be expensive.
int index = 0;
while (index < adGroupTimesUs.length
&& adGroupTimesUs[index] != C.TIME_END_OF_SOURCE
&& (positionUs >= adGroupTimesUs[index] || !adGroups[index].hasUnplayedAds())) {
&& ((adGroupTimesUs[index] != C.TIME_END_OF_SOURCE && adGroupTimesUs[index] <= positionUs)
|| !adGroups[index].hasUnplayedAds())) {
index++;
}
return index < adGroupTimesUs.length ? index : C.INDEX_UNSET;

View file

@ -706,10 +706,10 @@ import com.google.common.collect.ImmutableList;
currentPeriodId.windowSequenceNumber);
}
} else {
// Play the next ad group if it's available.
int nextAdGroupIndex = period.getAdGroupIndexForPositionUs(mediaPeriodInfo.endPositionUs);
if (nextAdGroupIndex == C.INDEX_UNSET) {
// The next ad group can't be played. Play content from the previous end position instead.
// Play the next ad group if it's still available.
int adIndexInAdGroup = period.getFirstAdIndexToPlay(currentPeriodId.nextAdGroupIndex);
if (adIndexInAdGroup == period.getAdCountInAdGroup(currentPeriodId.nextAdGroupIndex)) {
// The next ad group has no ads left to play. Play content from the end position instead.
return getMediaPeriodInfoForContent(
timeline,
currentPeriodId.periodUid,
@ -717,11 +717,10 @@ import com.google.common.collect.ImmutableList;
/* requestedContentPositionUs= */ mediaPeriodInfo.durationUs,
currentPeriodId.windowSequenceNumber);
}
int adIndexInAdGroup = period.getFirstAdIndexToPlay(nextAdGroupIndex);
return getMediaPeriodInfoForAd(
timeline,
currentPeriodId.periodUid,
nextAdGroupIndex,
currentPeriodId.nextAdGroupIndex,
adIndexInAdGroup,
/* contentPositionUs= */ mediaPeriodInfo.durationUs,
currentPeriodId.windowSequenceNumber);

View file

@ -229,6 +229,49 @@ public final class MediaPeriodQueueTest {
/* nextAdGroupIndex= */ C.INDEX_UNSET);
}
@Test
public void getNextMediaPeriodInfo_withPlayedAdGroups_returnsCorrectMediaPeriodInfos() {
setupAdTimeline(/* adGroupTimesUs...= */ 0, FIRST_AD_START_TIME_US, C.TIME_END_OF_SOURCE);
setAdGroupLoaded(/* adGroupIndex= */ 0);
setAdGroupLoaded(/* adGroupIndex= */ 1);
setAdGroupLoaded(/* adGroupIndex= */ 2);
assertNextMediaPeriodInfoIsAd(
/* adGroupIndex= */ 0, AD_DURATION_US, /* contentPositionUs= */ C.TIME_UNSET);
setAdGroupPlayed(/* adGroupIndex= */ 0);
clear();
assertGetNextMediaPeriodInfoReturnsContentMediaPeriod(
/* periodUid= */ firstPeriodUid,
/* startPositionUs= */ 0,
/* requestedContentPositionUs= */ C.TIME_UNSET,
/* endPositionUs= */ FIRST_AD_START_TIME_US,
/* durationUs= */ FIRST_AD_START_TIME_US,
/* isLastInPeriod= */ false,
/* isLastInWindow= */ false,
/* nextAdGroupIndex= */ 1);
setAdGroupPlayed(/* adGroupIndex= */ 1);
clear();
assertGetNextMediaPeriodInfoReturnsContentMediaPeriod(
/* periodUid= */ firstPeriodUid,
/* startPositionUs= */ 0,
/* requestedContentPositionUs= */ C.TIME_UNSET,
/* endPositionUs= */ C.TIME_END_OF_SOURCE,
/* durationUs= */ CONTENT_DURATION_US,
/* isLastInPeriod= */ false,
/* isLastInWindow= */ false,
/* nextAdGroupIndex= */ 2);
setAdGroupPlayed(/* adGroupIndex= */ 2);
clear();
assertGetNextMediaPeriodInfoReturnsContentMediaPeriod(
/* periodUid= */ firstPeriodUid,
/* startPositionUs= */ 0,
/* requestedContentPositionUs= */ C.TIME_UNSET,
/* endPositionUs= */ C.TIME_UNSET,
/* durationUs= */ CONTENT_DURATION_US,
/* isLastInPeriod= */ true,
/* isLastInWindow= */ true,
/* nextAdGroupIndex= */ C.INDEX_UNSET);
}
@Test
public void getNextMediaPeriodInfo_inMultiPeriodWindow_returnsCorrectMediaPeriodInfos() {
setupTimeline(
@ -269,14 +312,13 @@ public final class MediaPeriodQueueTest {
setAdGroupLoaded(/* adGroupIndex= */ 0);
setAdGroupLoaded(/* adGroupIndex= */ 1);
enqueueNext(); // Content before first ad.
advancePlaying();
enqueueNext(); // First ad.
enqueueNext(); // Content between ads.
enqueueNext(); // Second ad.
// Change position of second ad (= change duration of content between ads).
updateAdPlaybackStateAndTimeline(
/* adGroupTimesUs...= */ FIRST_AD_START_TIME_US, SECOND_AD_START_TIME_US + 1);
/* adGroupTimesUs...= */ FIRST_AD_START_TIME_US, SECOND_AD_START_TIME_US - 1000);
setAdGroupLoaded(/* adGroupIndex= */ 0);
setAdGroupLoaded(/* adGroupIndex= */ 1);
boolean changeHandled =
@ -294,15 +336,16 @@ public final class MediaPeriodQueueTest {
setAdGroupLoaded(/* adGroupIndex= */ 0);
setAdGroupLoaded(/* adGroupIndex= */ 1);
enqueueNext(); // Content before first ad.
advancePlaying();
enqueueNext(); // First ad.
enqueueNext(); // Content between ads.
enqueueNext(); // Second ad.
advanceReading(); // Reading first ad.
advanceReading(); // Reading content between ads.
advanceReading(); // Reading second ad.
// Change position of first ad (= change duration of content before first ad).
// Change position of second ad (= change duration of content between ads).
updateAdPlaybackStateAndTimeline(
/* adGroupTimesUs...= */ FIRST_AD_START_TIME_US + 1, SECOND_AD_START_TIME_US);
/* adGroupTimesUs...= */ FIRST_AD_START_TIME_US, SECOND_AD_START_TIME_US - 1000);
setAdGroupLoaded(/* adGroupIndex= */ 0);
setAdGroupLoaded(/* adGroupIndex= */ 1);
boolean changeHandled =
@ -312,7 +355,7 @@ public final class MediaPeriodQueueTest {
/* maxRendererReadPositionUs= */ FIRST_AD_START_TIME_US);
assertThat(changeHandled).isFalse();
assertThat(getQueueLength()).isEqualTo(1);
assertThat(getQueueLength()).isEqualTo(3);
}
@Test
@ -322,7 +365,6 @@ public final class MediaPeriodQueueTest {
setAdGroupLoaded(/* adGroupIndex= */ 0);
setAdGroupLoaded(/* adGroupIndex= */ 1);
enqueueNext(); // Content before first ad.
advancePlaying();
enqueueNext(); // First ad.
enqueueNext(); // Content between ads.
enqueueNext(); // Second ad.
@ -352,7 +394,6 @@ public final class MediaPeriodQueueTest {
setAdGroupLoaded(/* adGroupIndex= */ 0);
setAdGroupLoaded(/* adGroupIndex= */ 1);
enqueueNext(); // Content before first ad.
advancePlaying();
enqueueNext(); // First ad.
enqueueNext(); // Content between ads.
enqueueNext(); // Second ad.
@ -382,7 +423,6 @@ public final class MediaPeriodQueueTest {
setAdGroupLoaded(/* adGroupIndex= */ 0);
setAdGroupLoaded(/* adGroupIndex= */ 1);
enqueueNext(); // Content before first ad.
advancePlaying();
enqueueNext(); // First ad.
enqueueNext(); // Content between ads.
enqueueNext(); // Second ad.
@ -474,6 +514,21 @@ public final class MediaPeriodQueueTest {
new RendererConfiguration[0], new ExoTrackSelection[0], /* info= */ null));
}
private void clear() {
mediaPeriodQueue.clear();
playbackInfo =
playbackInfo.copyWithNewPosition(
mediaPeriodQueue.resolveMediaPeriodIdForAds(
mediaSourceList.createTimeline(), firstPeriodUid, /* positionUs= */ 0),
/* positionUs= */ 0,
/* requestedContentPositionUs= */ C.TIME_UNSET,
/* discontinuityStartPositionUs= */ 0,
/* totalBufferedDurationUs= */ 0,
/* trackGroups= */ null,
/* trackSelectorResult= */ null,
/* staticMetadata= */ ImmutableList.of());
}
private MediaPeriodInfo getNextMediaPeriodInfo() {
return mediaPeriodQueue.getNextMediaPeriodInfo(/* rendererPositionUs= */ 0, playbackInfo);
}
@ -492,6 +547,13 @@ public final class MediaPeriodQueueTest {
updateTimeline();
}
private void setAdGroupPlayed(int adGroupIndex) {
for (int i = 0; i < adPlaybackState.adGroups[adGroupIndex].count; i++) {
adPlaybackState = adPlaybackState.withPlayedAd(adGroupIndex, /* adIndexInAdGroup= */ i);
}
updateTimeline();
}
private void setAdGroupFailedToLoad(int adGroupIndex) {
adPlaybackState =
adPlaybackState