From 582adb748ae1c1beff071b962f5a3ef08d6f01b7 Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 25 Jan 2019 11:42:22 +0000 Subject: [PATCH] Decouple end position from MediaPeriodId again. If are allowing changing durations of periods, we shouldn't use the end position of clipped content as part of the id as it may change. This change moves the end position back to MediaPeriodInfo and adds the next ad group index to the id instead to ensure we still have unique ids for all content parts. PiperOrigin-RevId: 230878389 --- .../android/exoplayer2/MediaPeriodHolder.java | 23 +++++---- .../android/exoplayer2/MediaPeriodInfo.java | 29 ++++++++++-- .../android/exoplayer2/MediaPeriodQueue.java | 23 +++++---- .../exoplayer2/source/MediaSource.java | 47 ++++++++++++------- .../exoplayer2/MediaPeriodQueueTest.java | 40 +++++++++++----- 5 files changed, 108 insertions(+), 54 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodHolder.java b/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodHolder.java index 19622c6801..be3fde0fca 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodHolder.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodHolder.java @@ -89,7 +89,9 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; this.info = info; sampleStreams = new SampleStream[rendererCapabilities.length]; mayRetainStreamFlags = new boolean[rendererCapabilities.length]; - mediaPeriod = createMediaPeriod(info.id, mediaSource, allocator, info.startPositionUs); + mediaPeriod = + createMediaPeriod( + info.id, mediaSource, allocator, info.startPositionUs, info.endPositionUs); } /** @@ -294,7 +296,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; public void release() { disableTrackSelectionsInResult(); trackSelectorResult = null; - releaseMediaPeriod(info.id, mediaSource, mediaPeriod); + releaseMediaPeriod(info.endPositionUs, mediaSource, mediaPeriod); } /** @@ -399,24 +401,25 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; /** Returns a media period corresponding to the given {@code id}. */ private static MediaPeriod createMediaPeriod( - MediaPeriodId id, MediaSource mediaSource, Allocator allocator, long startPositionUs) { + MediaPeriodId id, + MediaSource mediaSource, + Allocator allocator, + long startPositionUs, + long endPositionUs) { MediaPeriod mediaPeriod = mediaSource.createPeriod(id, allocator, startPositionUs); - if (id.endPositionUs != C.TIME_UNSET && id.endPositionUs != C.TIME_END_OF_SOURCE) { + if (endPositionUs != C.TIME_UNSET && endPositionUs != C.TIME_END_OF_SOURCE) { mediaPeriod = new ClippingMediaPeriod( - mediaPeriod, - /* enableInitialDiscontinuity= */ true, - /* startUs= */ 0, - id.endPositionUs); + mediaPeriod, /* enableInitialDiscontinuity= */ true, /* startUs= */ 0, endPositionUs); } return mediaPeriod; } /** Releases the given {@code mediaPeriod}, logging and suppressing any errors. */ private static void releaseMediaPeriod( - MediaPeriodId id, MediaSource mediaSource, MediaPeriod mediaPeriod) { + long endPositionUs, MediaSource mediaSource, MediaPeriod mediaPeriod) { try { - if (id.endPositionUs != C.TIME_UNSET && id.endPositionUs != C.TIME_END_OF_SOURCE) { + if (endPositionUs != C.TIME_UNSET && endPositionUs != C.TIME_END_OF_SOURCE) { mediaSource.releasePeriod(((ClippingMediaPeriod) mediaPeriod).mediaPeriod); } else { mediaSource.releasePeriod(mediaPeriod); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodInfo.java b/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodInfo.java index 01de53c4fb..cd4e74b2ef 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodInfo.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodInfo.java @@ -33,7 +33,14 @@ import com.google.android.exoplayer2.util.Util; */ public final long contentPositionUs; /** - * The duration of the media period, like {@link MediaPeriodId#endPositionUs} but with {@link + * The end position to which the media period's content is clipped in order to play a following ad + * group, in microseconds, or {@link C#TIME_UNSET} if there is no following ad group or if this + * media period is an ad. The value {@link C#TIME_END_OF_SOURCE} indicates that a postroll ad + * follows at the end of this content media period. + */ + public final long endPositionUs; + /** + * The duration of the media period, like {@link #endPositionUs} but with {@link * C#TIME_END_OF_SOURCE} and {@link C#TIME_UNSET} resolved to the timeline period duration if * known. */ @@ -53,12 +60,14 @@ import com.google.android.exoplayer2.util.Util; MediaPeriodId id, long startPositionUs, long contentPositionUs, + long endPositionUs, long durationUs, boolean isLastInTimelinePeriod, boolean isFinal) { this.id = id; this.startPositionUs = startPositionUs; this.contentPositionUs = contentPositionUs; + this.endPositionUs = endPositionUs; this.durationUs = durationUs; this.isLastInTimelinePeriod = isLastInTimelinePeriod; this.isFinal = isFinal; @@ -72,7 +81,13 @@ import com.google.android.exoplayer2.util.Util; return startPositionUs == this.startPositionUs ? this : new MediaPeriodInfo( - id, startPositionUs, contentPositionUs, durationUs, isLastInTimelinePeriod, isFinal); + id, + startPositionUs, + contentPositionUs, + endPositionUs, + durationUs, + isLastInTimelinePeriod, + isFinal); } /** @@ -83,7 +98,13 @@ import com.google.android.exoplayer2.util.Util; return contentPositionUs == this.contentPositionUs ? this : new MediaPeriodInfo( - id, startPositionUs, contentPositionUs, durationUs, isLastInTimelinePeriod, isFinal); + id, + startPositionUs, + contentPositionUs, + endPositionUs, + durationUs, + isLastInTimelinePeriod, + isFinal); } @Override @@ -97,6 +118,7 @@ import com.google.android.exoplayer2.util.Util; MediaPeriodInfo that = (MediaPeriodInfo) o; return startPositionUs == that.startPositionUs && contentPositionUs == that.contentPositionUs + && endPositionUs == that.endPositionUs && durationUs == that.durationUs && isLastInTimelinePeriod == that.isLastInTimelinePeriod && isFinal == that.isFinal @@ -109,6 +131,7 @@ import com.google.android.exoplayer2.util.Util; result = 31 * result + id.hashCode(); result = 31 * result + (int) startPositionUs; result = 31 * result + (int) contentPositionUs; + result = 31 * result + (int) endPositionUs; result = 31 * result + (int) durationUs; result = 31 * result + (isLastInTimelinePeriod ? 1 : 0); result = 31 * result + (isFinal ? 1 : 0); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodQueue.java b/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodQueue.java index 4585b97529..d6ff320295 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodQueue.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodQueue.java @@ -353,13 +353,14 @@ import com.google.android.exoplayer2.util.Assertions; long durationUs = id.isAd() ? period.getAdDurationUs(id.adGroupIndex, id.adIndexInAdGroup) - : (id.endPositionUs == C.TIME_UNSET || id.endPositionUs == C.TIME_END_OF_SOURCE + : (info.endPositionUs == C.TIME_UNSET || info.endPositionUs == C.TIME_END_OF_SOURCE ? period.getDurationUs() - : id.endPositionUs); + : info.endPositionUs); return new MediaPeriodInfo( id, info.startPositionUs, info.contentPositionUs, + info.endPositionUs, durationUs, isLastInPeriod, isLastInTimeline); @@ -398,11 +399,7 @@ import com.google.android.exoplayer2.util.Assertions; int adGroupIndex = period.getAdGroupIndexForPositionUs(positionUs); if (adGroupIndex == C.INDEX_UNSET) { int nextAdGroupIndex = period.getAdGroupIndexAfterPositionUs(positionUs); - long endPositionUs = - nextAdGroupIndex == C.INDEX_UNSET - ? C.TIME_UNSET - : period.getAdGroupTimeUs(nextAdGroupIndex); - return new MediaPeriodId(periodUid, windowSequenceNumber, endPositionUs); + return new MediaPeriodId(periodUid, windowSequenceNumber, nextAdGroupIndex); } else { int adIndexInAdGroup = period.getFirstAdIndexToPlay(adGroupIndex); return new MediaPeriodId(periodUid, adGroupIndex, adIndexInAdGroup, windowSequenceNumber); @@ -630,7 +627,7 @@ import com.google.android.exoplayer2.util.Assertions; } } else { // Play the next ad group if it's available. - int nextAdGroupIndex = period.getAdGroupIndexForPositionUs(mediaPeriodInfo.id.endPositionUs); + 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. return getMediaPeriodInfoForContent( @@ -688,6 +685,7 @@ import com.google.android.exoplayer2.util.Assertions; id, startPositionUs, contentPositionUs, + /* endPositionUs= */ C.TIME_UNSET, durationUs, /* isLastInTimelinePeriod= */ false, /* isFinal= */ false); @@ -696,13 +694,13 @@ import com.google.android.exoplayer2.util.Assertions; private MediaPeriodInfo getMediaPeriodInfoForContent( Object periodUid, long startPositionUs, long windowSequenceNumber) { int nextAdGroupIndex = period.getAdGroupIndexAfterPositionUs(startPositionUs); + MediaPeriodId id = new MediaPeriodId(periodUid, windowSequenceNumber, nextAdGroupIndex); + boolean isLastInPeriod = isLastInPeriod(id); + boolean isLastInTimeline = isLastInTimeline(id, isLastInPeriod); long endPositionUs = nextAdGroupIndex != C.INDEX_UNSET ? period.getAdGroupTimeUs(nextAdGroupIndex) : C.TIME_UNSET; - MediaPeriodId id = new MediaPeriodId(periodUid, windowSequenceNumber, endPositionUs); - boolean isLastInPeriod = isLastInPeriod(id); - boolean isLastInTimeline = isLastInTimeline(id, isLastInPeriod); long durationUs = endPositionUs == C.TIME_UNSET || endPositionUs == C.TIME_END_OF_SOURCE ? period.durationUs @@ -711,13 +709,14 @@ import com.google.android.exoplayer2.util.Assertions; id, startPositionUs, /* contentPositionUs= */ C.TIME_UNSET, + endPositionUs, durationUs, isLastInPeriod, isLastInTimeline); } private boolean isLastInPeriod(MediaPeriodId id) { - return !id.isAd() && id.endPositionUs == C.TIME_UNSET; + return !id.isAd() && id.nextAdGroupIndex == C.INDEX_UNSET; } private boolean isLastInTimeline(MediaPeriodId id, boolean isLastMediaPeriodInPeriod) { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/MediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/MediaSource.java index 1419f9a98f..20346b781f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/MediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/MediaSource.java @@ -89,12 +89,10 @@ public interface MediaSource { public final long windowSequenceNumber; /** - * The end position to which the media period's content is clipped in order to play a following - * ad group, in microseconds, or {@link C#TIME_UNSET} if there is no following ad group or if - * this media period is an ad. The value {@link C#TIME_END_OF_SOURCE} indicates that a postroll - * ad follows at the end of this content media period. + * The index of the next ad group to which the media period's content is clipped, or {@link + * C#INDEX_UNSET} if there is no following ad group or if this media period is an ad. */ - public final long endPositionUs; + public final int nextAdGroupIndex; /** * Creates a media period identifier for a dummy period which is not part of a buffered sequence @@ -103,7 +101,7 @@ public interface MediaSource { * @param periodUid The unique id of the timeline period. */ public MediaPeriodId(Object periodUid) { - this(periodUid, C.INDEX_UNSET); + this(periodUid, /* windowSequenceNumber= */ C.INDEX_UNSET); } /** @@ -114,7 +112,12 @@ public interface MediaSource { * windows this media period is part of. */ public MediaPeriodId(Object periodUid, long windowSequenceNumber) { - this(periodUid, C.INDEX_UNSET, C.INDEX_UNSET, windowSequenceNumber, C.TIME_UNSET); + this( + periodUid, + /* adGroupIndex= */ C.INDEX_UNSET, + /* adIndexInAdGroup= */ C.INDEX_UNSET, + windowSequenceNumber, + /* nextAdGroupIndex= */ C.INDEX_UNSET); } /** @@ -123,11 +126,16 @@ public interface MediaSource { * @param periodUid The unique id of the timeline period. * @param windowSequenceNumber The sequence number of the window in the buffered sequence of * windows this media period is part of. - * @param endPositionUs The end position of the media period within the timeline period, in - * microseconds. + * @param nextAdGroupIndex The index of the next ad group to which the media period's content is + * clipped. */ - public MediaPeriodId(Object periodUid, long windowSequenceNumber, long endPositionUs) { - this(periodUid, C.INDEX_UNSET, C.INDEX_UNSET, windowSequenceNumber, endPositionUs); + public MediaPeriodId(Object periodUid, long windowSequenceNumber, int nextAdGroupIndex) { + this( + periodUid, + /* adGroupIndex= */ C.INDEX_UNSET, + /* adIndexInAdGroup= */ C.INDEX_UNSET, + windowSequenceNumber, + nextAdGroupIndex); } /** @@ -142,7 +150,12 @@ public interface MediaSource { */ public MediaPeriodId( Object periodUid, int adGroupIndex, int adIndexInAdGroup, long windowSequenceNumber) { - this(periodUid, adGroupIndex, adIndexInAdGroup, windowSequenceNumber, C.TIME_UNSET); + this( + periodUid, + adGroupIndex, + adIndexInAdGroup, + windowSequenceNumber, + /* nextAdGroupIndex= */ C.INDEX_UNSET); } private MediaPeriodId( @@ -150,12 +163,12 @@ public interface MediaSource { int adGroupIndex, int adIndexInAdGroup, long windowSequenceNumber, - long endPositionUs) { + int nextAdGroupIndex) { this.periodUid = periodUid; this.adGroupIndex = adGroupIndex; this.adIndexInAdGroup = adIndexInAdGroup; this.windowSequenceNumber = windowSequenceNumber; - this.endPositionUs = endPositionUs; + this.nextAdGroupIndex = nextAdGroupIndex; } /** Returns a copy of this period identifier but with {@code newPeriodUid} as its period uid. */ @@ -163,7 +176,7 @@ public interface MediaSource { return periodUid.equals(newPeriodUid) ? this : new MediaPeriodId( - newPeriodUid, adGroupIndex, adIndexInAdGroup, windowSequenceNumber, endPositionUs); + newPeriodUid, adGroupIndex, adIndexInAdGroup, windowSequenceNumber, nextAdGroupIndex); } /** @@ -187,7 +200,7 @@ public interface MediaSource { && adGroupIndex == periodId.adGroupIndex && adIndexInAdGroup == periodId.adIndexInAdGroup && windowSequenceNumber == periodId.windowSequenceNumber - && endPositionUs == periodId.endPositionUs; + && nextAdGroupIndex == periodId.nextAdGroupIndex; } @Override @@ -197,7 +210,7 @@ public interface MediaSource { result = 31 * result + adGroupIndex; result = 31 * result + adIndexInAdGroup; result = 31 * result + (int) windowSequenceNumber; - result = 31 * result + (int) endPositionUs; + result = 31 * result + nextAdGroupIndex; return result; } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java b/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java index e8f43e3fe6..6016ec1db7 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java @@ -71,7 +71,8 @@ public final class MediaPeriodQueueTest { /* startPositionUs= */ 0, /* endPositionUs= */ C.TIME_UNSET, /* durationUs= */ CONTENT_DURATION_US, - /* isLast= */ true); + /* isLast= */ true, + /* nextAdGroupIndex= */ C.INDEX_UNSET); } @Test @@ -84,7 +85,8 @@ public final class MediaPeriodQueueTest { /* startPositionUs= */ 0, /* endPositionUs= */ C.TIME_UNSET, /* durationUs= */ CONTENT_DURATION_US, - /* isLast= */ true); + /* isLast= */ true, + /* nextAdGroupIndex= */ C.INDEX_UNSET); } @Test @@ -97,7 +99,8 @@ public final class MediaPeriodQueueTest { /* startPositionUs= */ 0, /* endPositionUs= */ FIRST_AD_START_TIME_US, /* durationUs= */ FIRST_AD_START_TIME_US, - /* isLast= */ false); + /* isLast= */ false, + /* nextAdGroupIndex= */ 0); // The next media period info should be null as we haven't loaded the ad yet. advance(); assertNull(getNextMediaPeriodInfo()); @@ -109,7 +112,8 @@ public final class MediaPeriodQueueTest { /* startPositionUs= */ FIRST_AD_START_TIME_US, /* endPositionUs= */ SECOND_AD_START_TIME_US, /* durationUs= */ SECOND_AD_START_TIME_US, - /* isLast= */ false); + /* isLast= */ false, + /* nextAdGroupIndex= */ 1); advance(); setAdGroupLoaded(/* adGroupIndex= */ 1); assertNextMediaPeriodInfoIsAd( @@ -119,7 +123,8 @@ public final class MediaPeriodQueueTest { /* startPositionUs= */ SECOND_AD_START_TIME_US, /* endPositionUs= */ C.TIME_UNSET, /* durationUs= */ CONTENT_DURATION_US, - /* isLast= */ true); + /* isLast= */ true, + /* nextAdGroupIndex= */ C.INDEX_UNSET); } @Test @@ -132,7 +137,8 @@ public final class MediaPeriodQueueTest { /* startPositionUs= */ 0, /* endPositionUs= */ FIRST_AD_START_TIME_US, /* durationUs= */ FIRST_AD_START_TIME_US, - /* isLast= */ false); + /* isLast= */ false, + /* nextAdGroupIndex= */ 0); advance(); setAdGroupLoaded(/* adGroupIndex= */ 0); assertNextMediaPeriodInfoIsAd( @@ -142,7 +148,8 @@ public final class MediaPeriodQueueTest { /* startPositionUs= */ FIRST_AD_START_TIME_US, /* endPositionUs= */ C.TIME_END_OF_SOURCE, /* durationUs= */ CONTENT_DURATION_US, - /* isLast= */ false); + /* isLast= */ false, + /* nextAdGroupIndex= */ 1); advance(); setAdGroupLoaded(/* adGroupIndex= */ 1); assertNextMediaPeriodInfoIsAd( @@ -152,7 +159,8 @@ public final class MediaPeriodQueueTest { /* startPositionUs= */ CONTENT_DURATION_US, /* endPositionUs= */ C.TIME_UNSET, /* durationUs= */ CONTENT_DURATION_US, - /* isLast= */ true); + /* isLast= */ true, + /* nextAdGroupIndex= */ C.INDEX_UNSET); } @Test @@ -162,14 +170,16 @@ public final class MediaPeriodQueueTest { /* startPositionUs= */ 0, /* endPositionUs= */ C.TIME_END_OF_SOURCE, /* durationUs= */ CONTENT_DURATION_US, - /* isLast= */ false); + /* isLast= */ false, + /* nextAdGroupIndex= */ 0); advance(); setAdGroupFailedToLoad(/* adGroupIndex= */ 0); assertGetNextMediaPeriodInfoReturnsContentMediaPeriod( /* startPositionUs= */ CONTENT_DURATION_US, /* endPositionUs= */ C.TIME_UNSET, /* durationUs= */ CONTENT_DURATION_US, - /* isLast= */ true); + /* isLast= */ true, + /* nextAdGroupIndex= */ C.INDEX_UNSET); } private void setupInitialTimeline(long initialPositionUs, long... adGroupTimesUs) { @@ -227,13 +237,18 @@ public final class MediaPeriodQueueTest { } private void assertGetNextMediaPeriodInfoReturnsContentMediaPeriod( - long startPositionUs, long endPositionUs, long durationUs, boolean isLast) { + long startPositionUs, + long endPositionUs, + long durationUs, + boolean isLast, + int nextAdGroupIndex) { assertThat(getNextMediaPeriodInfo()) .isEqualTo( new MediaPeriodInfo( - new MediaPeriodId(periodUid, /* windowSequenceNumber= */ 0, endPositionUs), + new MediaPeriodId(periodUid, /* windowSequenceNumber= */ 0, nextAdGroupIndex), startPositionUs, /* contentPositionUs= */ C.TIME_UNSET, + endPositionUs, durationUs, /* isLastInTimelinePeriod= */ isLast, /* isFinal= */ isLast)); @@ -250,6 +265,7 @@ public final class MediaPeriodQueueTest { /* windowSequenceNumber= */ 0), /* startPositionUs= */ 0, contentPositionUs, + /* endPositionUs= */ C.TIME_UNSET, /* durationUs= */ C.TIME_UNSET, /* isLastInTimelinePeriod= */ false, /* isFinal= */ false));