loliball/media3
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix bug where content position of ads in moving live windows is updated

Browse Source 
The method to handle Timeline updates currently uses
 isAd() || isPlaceholder()
to trigger two things:
 1. Using the existing requested content position as the content
    position.
 2. Re-resolving the content position from window to period in case
    it changed since the last update.

The condition is correct for case (1) because ads must use the content
position (and not the position in the ad) and a placeholder period must
keep using the requested content position as well until the media
information is no longer a placeholder.

However, case (2) only needs to be done if the content position is
C.TIME_UNSET (to start at the default position) OR if the period is
still a placeholder and we want to re-resolve the position.

The case where re-resolution shouldn't be done is for ads with a non-
placeholder period and a concrete content position. This likely only
affects ads in live stream where the content position is currently
moving with the live stream instead of staying where it is.

PiperOrigin-RevId: 372929439
This commit is contained in:
tonihei 2021-05-10 16:08:47 +01:00 committed by Oliver Woodman
parent 8a5d21adef
commit 5167ca65fb
2 changed files with 77 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java
View File

@ -1369,7 +1369,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
MediaPeriodId mediaPeriodId = playbackInfo.periodId;
long startPositionUs = playbackInfo.positionUs;
long requestedContentPositionUs =
shouldUseRequestedContentPosition(playbackInfo, period)
playbackInfo.periodId.isAd() || isUsingPlaceholderPeriod(playbackInfo, period)
? playbackInfo.requestedContentPositionUs
: playbackInfo.positionUs;
boolean resetTrackInfo = false;
@ -2475,10 +2475,9 @@ import java.util.concurrent.atomic.AtomicBoolean;
}
MediaPeriodId oldPeriodId = playbackInfo.periodId;
Object newPeriodUid = oldPeriodId.periodUid;
boolean shouldUseRequestedContentPosition =
shouldUseRequestedContentPosition(playbackInfo, period);
boolean isUsingPlaceholderPeriod = isUsingPlaceholderPeriod(playbackInfo, period);
long oldContentPositionUs =
shouldUseRequestedContentPosition
playbackInfo.periodId.isAd() || isUsingPlaceholderPeriod
? playbackInfo.requestedContentPositionUs
: playbackInfo.positionUs;
long newContentPositionUs = oldContentPositionUs;
@ -2541,28 +2540,27 @@ import java.util.concurrent.atomic.AtomicBoolean;
startAtDefaultPositionWindowIndex =
timeline.getPeriodByUid(subsequentPeriodUid, period).windowIndex;
}
} else if (shouldUseRequestedContentPosition) {
// We previously requested a content position, but haven't used it yet. Re-resolve the
// requested window position to the period uid and position in case they changed.
if (oldContentPositionUs == C.TIME_UNSET) {
startAtDefaultPositionWindowIndex =
timeline.getPeriodByUid(newPeriodUid, period).windowIndex;
} else {
playbackInfo.timeline.getPeriodByUid(oldPeriodId.periodUid, period);
if (playbackInfo.timeline.getWindow(period.windowIndex, window).firstPeriodIndex
== playbackInfo.timeline.getIndexOfPeriod(oldPeriodId.periodUid)) {
// Only need to resolve the first period in a window because subsequent periods must start
// at position 0 and don't need to be resolved.
long windowPositionUs = oldContentPositionUs + period.getPositionInWindowUs();
int windowIndex = timeline.getPeriodByUid(newPeriodUid, period).windowIndex;
Pair<Object, Long> periodPosition =
timeline.getPeriodPosition(window, period, windowIndex, windowPositionUs);
newPeriodUid = periodPosition.first;
newContentPositionUs = periodPosition.second;
}
// Use an explicitly requested content position as new target live offset.
setTargetLiveOffset = true;
} else if (oldContentPositionUs == C.TIME_UNSET) {
// The content was requested to start from its default position and we haven't used the
// resolved position yet. Re-resolve in case the default position changed.
startAtDefaultPositionWindowIndex = timeline.getPeriodByUid(newPeriodUid, period).windowIndex;
} else if (isUsingPlaceholderPeriod) {
// We previously requested a content position for a placeholder period, but haven't used it
// yet. Re-resolve the requested window position to the period position in case it changed.
playbackInfo.timeline.getPeriodByUid(oldPeriodId.periodUid, period);
if (playbackInfo.timeline.getWindow(period.windowIndex, window).firstPeriodIndex
== playbackInfo.timeline.getIndexOfPeriod(oldPeriodId.periodUid)) {
// Only need to resolve the first period in a window because subsequent periods must start
// at position 0 and don't need to be resolved.
long windowPositionUs = oldContentPositionUs + period.getPositionInWindowUs();
int windowIndex = timeline.getPeriodByUid(newPeriodUid, period).windowIndex;
Pair<Object, Long> periodPosition =
timeline.getPeriodPosition(window, period, windowIndex, windowPositionUs);
newPeriodUid = periodPosition.first;
newContentPositionUs = periodPosition.second;
}
// Use an explicitly requested content position as new target live offset.
setTargetLiveOffset = true;
}
// Set period uid for default positions and resolve position for ad resolution.
@ -2618,15 +2616,11 @@ import java.util.concurrent.atomic.AtomicBoolean;
setTargetLiveOffset);
}
private static boolean shouldUseRequestedContentPosition(
private static boolean isUsingPlaceholderPeriod(
PlaybackInfo playbackInfo, Timeline.Period period) {
// Only use the actual position as content position if it's not an ad and we already have
// prepared media information. Otherwise use the requested position.
MediaPeriodId periodId = playbackInfo.periodId;
Timeline timeline = playbackInfo.timeline;
return periodId.isAd()
|| timeline.isEmpty()
|| timeline.getPeriodByUid(periodId.periodUid, period).isPlaceholder;
return timeline.isEmpty() || timeline.getPeriodByUid(periodId.periodUid, period).isPlaceholder;
}
/**

56
library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java
View File

@ -42,6 +42,7 @@ import static com.google.android.exoplayer2.robolectric.RobolectricUtil.runMainL
import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.playUntilStartOfWindow;
import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled;
import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilPlaybackState;
import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilPositionDiscontinuity;
import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilReceiveOffloadSchedulingEnabledNewState;
import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilSleepingForOffload;
import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilTimelineChanged;
@ -2731,8 +2732,7 @@ public final class ExoPlayerTest {
player.play();
// When the ad finishes, the player position should be at or after the requested seek position.
TestPlayerRunHelper.runUntilPositionDiscontinuity(
player, Player.DISCONTINUITY_REASON_AUTO_TRANSITION);
runUntilPositionDiscontinuity(player, Player.DISCONTINUITY_REASON_AUTO_TRANSITION);
assertThat(player.getCurrentPosition()).isAtLeast(seekPositionMs);
}
@ -3391,6 +3391,55 @@ public final class ExoPlayerTest {
assertThat(contentStartPositionMs.get()).isAtLeast(5_000L);
}
@Test
public void adInMovingLiveWindow_keepsContentPosition() throws Exception {
SimpleExoPlayer player = new TestExoPlayerBuilder(context).build();
AdPlaybackState adPlaybackState =
FakeTimeline.createAdPlaybackState(
/* adsPerAdGroup= */ 1, /* adGroupTimesUs...= */ 42_000_004_000_000L);
Timeline liveTimeline1 =
new FakeTimeline(
new TimelineWindowDefinition(
/* periodCount= */ 1,
/* id= */ 0,
/* isSeekable= */ true,
/* isDynamic= */ true,
/* isLive= */ true,
/* isPlaceholder= */ false,
/* durationUs= */ 10_000_000,
/* defaultPositionUs= */ 3_000_000,
/* windowOffsetInFirstPeriodUs= */ 42_000_000_000_000L,
adPlaybackState));
Timeline liveTimeline2 =
new FakeTimeline(
new TimelineWindowDefinition(
/* periodCount= */ 1,
/* id= */ 0,
/* isSeekable= */ true,
/* isDynamic= */ true,
/* isLive= */ true,
/* isPlaceholder= */ false,
/* durationUs= */ 10_000_000,
/* defaultPositionUs= */ 3_000_000,
/* windowOffsetInFirstPeriodUs= */ 42_000_002_000_000L,
adPlaybackState));
FakeMediaSource fakeMediaSource = new FakeMediaSource(liveTimeline1);
player.setMediaSource(fakeMediaSource);
player.prepare();
player.play();
// Wait until the ad is playing.
runUntilPositionDiscontinuity(player, Player.DISCONTINUITY_REASON_AUTO_TRANSITION);
long contentPositionBeforeLiveWindowUpdateMs = player.getContentPosition();
fakeMediaSource.setNewSourceInfo(liveTimeline2);
runUntilTimelineChanged(player);
long contentPositionAfterLiveWindowUpdateMs = player.getContentPosition();
player.release();
assertThat(contentPositionBeforeLiveWindowUpdateMs).isEqualTo(4000);
assertThat(contentPositionAfterLiveWindowUpdateMs).isEqualTo(2000);
}
@Test
public void setPlaybackSpeedConsecutivelyNotifiesListenerForEveryChangeOnceAndIsMasked()
throws Exception {
@ -7595,8 +7644,7 @@ public final class ExoPlayerTest {
// Update media with a non-zero default start position and window offset.
firstMediaSource.setNewSourceInfo(timelineWithOffsets);
// Wait until player transitions to second source (which also has non-zero offsets).
TestPlayerRunHelper.runUntilPositionDiscontinuity(
player, Player.DISCONTINUITY_REASON_AUTO_TRANSITION);
runUntilPositionDiscontinuity(player, Player.DISCONTINUITY_REASON_AUTO_TRANSITION);
assertThat(player.getCurrentWindowIndex()).isEqualTo(1);
player.release();