From 22413b8037426ed0c40b479aa09680bca40e5df8 Mon Sep 17 00:00:00 2001 From: tonihei Date: Thu, 17 Jan 2019 17:00:11 +0000 Subject: [PATCH] Add start position to MediaSource.createPeriod. That's the same position set in MediaPeriod.prepare (where it may be removed in the future). Having the position at an earlier point is necessary to fix an issue with lazy preparation in ConcatenatingMediaSource where the prepare position was assumed to be known but MediaPeriod.prepare hasn't been called yet. Issue:#5350 PiperOrigin-RevId: 229756637 --- RELEASENOTES.md | 4 +++ .../exoplayer2/ext/ima/ImaAdsMediaSource.java | 4 +-- .../android/exoplayer2/MediaPeriodHolder.java | 6 ++-- .../source/ClippingMediaSource.java | 4 +-- .../source/ConcatenatingMediaSource.java | 8 +++-- .../source/DeferredMediaPeriod.java | 36 ++++++++++--------- .../source/ExtractorMediaSource.java | 2 +- .../exoplayer2/source/LoopingMediaSource.java | 7 ++-- .../exoplayer2/source/MediaSource.java | 7 ++-- .../exoplayer2/source/MergingMediaSource.java | 4 +-- .../source/SingleSampleMediaSource.java | 2 +- .../exoplayer2/source/ads/AdsMediaSource.java | 8 +++-- .../source/dash/DashMediaSource.java | 3 +- .../exoplayer2/source/hls/HlsMediaSource.java | 2 +- .../source/smoothstreaming/SsMediaSource.java | 2 +- .../exoplayer2/testutil/FakeMediaSource.java | 2 +- .../testutil/MediaSourceTestRunner.java | 19 ++++++++-- 17 files changed, 74 insertions(+), 46 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 96d1fb1ff3..51264dcfe0 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -39,6 +39,10 @@ * Remove `player` and `isTopLevelSource` parameters from `MediaSource.prepare`. * Change signature of `PlayerNotificationManager.NotificationListener` to better fit service requirements. Remove ability to set a custom stop action. +* Add `startPositionUs` to `MediaSource.createPeriod`. This fixes an issue where + using lazy preparation in `ConcatenatingMediaSource` with an + `ExtractorMediaSource` overrides initial seek positions + ([#5350](https://github.com/google/ExoPlayer/issues/5350)). ### 2.9.4 ### diff --git a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsMediaSource.java b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsMediaSource.java index 574bac5d35..bcccd6cec7 100644 --- a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsMediaSource.java +++ b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsMediaSource.java @@ -93,8 +93,8 @@ public final class ImaAdsMediaSource extends BaseMediaSource implements SourceIn } @Override - public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { - return adsMediaSource.createPeriod(id, allocator); + public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long startPositionUs) { + return adsMediaSource.createPeriod(id, allocator, startPositionUs); } @Override 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 7becac7b55..19622c6801 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,7 @@ 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); + mediaPeriod = createMediaPeriod(info.id, mediaSource, allocator, info.startPositionUs); } /** @@ -399,8 +399,8 @@ 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) { - MediaPeriod mediaPeriod = mediaSource.createPeriod(id, allocator); + MediaPeriodId id, MediaSource mediaSource, Allocator allocator, long startPositionUs) { + MediaPeriod mediaPeriod = mediaSource.createPeriod(id, allocator, startPositionUs); if (id.endPositionUs != C.TIME_UNSET && id.endPositionUs != C.TIME_END_OF_SOURCE) { mediaPeriod = new ClippingMediaPeriod( diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ClippingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ClippingMediaSource.java index 0ab6eeae18..d3b8226822 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ClippingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ClippingMediaSource.java @@ -206,10 +206,10 @@ public final class ClippingMediaSource extends CompositeMediaSource { } @Override - public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { + public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long startPositionUs) { ClippingMediaPeriod mediaPeriod = new ClippingMediaPeriod( - mediaSource.createPeriod(id, allocator), + mediaSource.createPeriod(id, allocator, startPositionUs), enableInitialDiscontinuity, periodStartUs, periodEndUs); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java index e6d409c659..6dc7a0a327 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java @@ -438,7 +438,8 @@ public class ConcatenatingMediaSource extends CompositeMediaSource { } @Override - public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { + public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long startPositionUs) { if (loopCount == Integer.MAX_VALUE) { - return childSource.createPeriod(id, allocator); + return childSource.createPeriod(id, allocator, startPositionUs); } Object childPeriodUid = LoopingTimeline.getChildPeriodUidFromConcatenatedUid(id.periodUid); MediaPeriodId childMediaPeriodId = id.copyWithPeriodUid(childPeriodUid); childMediaPeriodIdToMediaPeriodId.put(childMediaPeriodId, id); - MediaPeriod mediaPeriod = childSource.createPeriod(childMediaPeriodId, allocator); + MediaPeriod mediaPeriod = + childSource.createPeriod(childMediaPeriodId, allocator, startPositionUs); mediaPeriodToChildMediaPeriodId.put(mediaPeriod, childMediaPeriodId); return mediaPeriod; } 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 2d80fb7f13..1419f9a98f 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 @@ -34,8 +34,8 @@ import java.io.IOException; * on the {@link SourceInfoRefreshListener}s passed to {@link * #prepareSource(SourceInfoRefreshListener, TransferListener)}. *
  • To provide {@link MediaPeriod} instances for the periods in its timeline. MediaPeriods are - * obtained by calling {@link #createPeriod(MediaPeriodId, Allocator)}, and provide a way for - * the player to load and read the media. + * obtained by calling {@link #createPeriod(MediaPeriodId, Allocator, long)}, and provide a + * way for the player to load and read the media. * * * All methods are called on the player's internal playback thread, as described in the {@link @@ -261,9 +261,10 @@ public interface MediaSource { * * @param id The identifier of the period. * @param allocator An {@link Allocator} from which to obtain media buffer allocations. + * @param startPositionUs The expected start position, in microseconds. * @return A new {@link MediaPeriod}. */ - MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator); + MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long startPositionUs); /** * Releases the period. diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/MergingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/MergingMediaSource.java index fdeb2b6184..1ea3404e81 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/MergingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/MergingMediaSource.java @@ -120,13 +120,13 @@ public final class MergingMediaSource extends CompositeMediaSource { } @Override - public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { + public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long startPositionUs) { MediaPeriod[] periods = new MediaPeriod[mediaSources.length]; int periodIndex = timelines[0].getIndexOfPeriod(id.periodUid); for (int i = 0; i < periods.length; i++) { MediaPeriodId childMediaPeriodId = id.copyWithPeriodUid(timelines[i].getUidOfPeriod(periodIndex)); - periods[i] = mediaSources[i].createPeriod(childMediaPeriodId, allocator); + periods[i] = mediaSources[i].createPeriod(childMediaPeriodId, allocator, startPositionUs); } return new MergingMediaPeriod(compositeSequenceableLoaderFactory, periods); } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/SingleSampleMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/SingleSampleMediaSource.java index 358875eb1e..d13fa06434 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/SingleSampleMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/SingleSampleMediaSource.java @@ -313,7 +313,7 @@ public final class SingleSampleMediaSource extends BaseMediaSource { } @Override - public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { + public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long startPositionUs) { return new SingleSampleMediaPeriod( dataSpec, dataSourceFactory, diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java index 663f9c64fc..4754466235 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java @@ -334,7 +334,7 @@ public final class AdsMediaSource extends CompositeMediaSource { } @Override - public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { + public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long startPositionUs) { if (adPlaybackState.adGroupCount > 0 && id.isAd()) { int adGroupIndex = id.adGroupIndex; int adIndexInAdGroup = id.adIndexInAdGroup; @@ -353,7 +353,8 @@ public final class AdsMediaSource extends CompositeMediaSource { prepareChildSource(id, adMediaSource); } MediaSource mediaSource = adGroupMediaSources[adGroupIndex][adIndexInAdGroup]; - DeferredMediaPeriod deferredMediaPeriod = new DeferredMediaPeriod(mediaSource, id, allocator); + DeferredMediaPeriod deferredMediaPeriod = + new DeferredMediaPeriod(mediaSource, id, allocator, startPositionUs); deferredMediaPeriod.setPrepareErrorListener( new AdPrepareErrorListener(adUri, adGroupIndex, adIndexInAdGroup)); List mediaPeriods = deferredMediaPeriodByAdMediaSource.get(mediaSource); @@ -369,7 +370,8 @@ public final class AdsMediaSource extends CompositeMediaSource { } return deferredMediaPeriod; } else { - DeferredMediaPeriod mediaPeriod = new DeferredMediaPeriod(contentMediaSource, id, allocator); + DeferredMediaPeriod mediaPeriod = + new DeferredMediaPeriod(contentMediaSource, id, allocator, startPositionUs); mediaPeriod.createPeriod(id); return mediaPeriod; } diff --git a/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java b/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java index b31b770b03..cfdbdac1ea 100644 --- a/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java +++ b/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DashMediaSource.java @@ -654,7 +654,8 @@ public final class DashMediaSource extends BaseMediaSource { } @Override - public MediaPeriod createPeriod(MediaPeriodId periodId, Allocator allocator) { + public MediaPeriod createPeriod( + MediaPeriodId periodId, Allocator allocator, long startPositionUs) { int periodIndex = (Integer) periodId.periodUid - firstPeriodId; EventDispatcher periodEventDispatcher = createEventDispatcher(periodId, manifest.getPeriod(periodIndex).startMs); diff --git a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaSource.java b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaSource.java index 8058460b9f..f2b76dddc4 100644 --- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaSource.java +++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaSource.java @@ -428,7 +428,7 @@ public final class HlsMediaSource extends BaseMediaSource } @Override - public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { + public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long startPositionUs) { EventDispatcher eventDispatcher = createEventDispatcher(id); return new HlsMediaPeriod( extractorFactory, diff --git a/library/smoothstreaming/src/main/java/com/google/android/exoplayer2/source/smoothstreaming/SsMediaSource.java b/library/smoothstreaming/src/main/java/com/google/android/exoplayer2/source/smoothstreaming/SsMediaSource.java index 39f707b09c..0f5544a993 100644 --- a/library/smoothstreaming/src/main/java/com/google/android/exoplayer2/source/smoothstreaming/SsMediaSource.java +++ b/library/smoothstreaming/src/main/java/com/google/android/exoplayer2/source/smoothstreaming/SsMediaSource.java @@ -552,7 +552,7 @@ public final class SsMediaSource extends BaseMediaSource } @Override - public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { + public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long startPositionUs) { EventDispatcher eventDispatcher = createEventDispatcher(id); SsMediaPeriod period = new SsMediaPeriod( diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java index 8165eebaea..de4be82b38 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java @@ -112,7 +112,7 @@ public class FakeMediaSource extends BaseMediaSource { } @Override - public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { + public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long startPositionUs) { assertThat(preparedSource).isTrue(); assertThat(releasedSource).isFalse(); int periodIndex = timeline.getIndexOfPeriod(id.periodUid); diff --git a/testutils_robolectric/src/main/java/com/google/android/exoplayer2/testutil/MediaSourceTestRunner.java b/testutils_robolectric/src/main/java/com/google/android/exoplayer2/testutil/MediaSourceTestRunner.java index 5de1ab87b6..9514768416 100644 --- a/testutils_robolectric/src/main/java/com/google/android/exoplayer2/testutil/MediaSourceTestRunner.java +++ b/testutils_robolectric/src/main/java/com/google/android/exoplayer2/testutil/MediaSourceTestRunner.java @@ -132,15 +132,28 @@ public class MediaSourceTestRunner { } /** - * Calls {@link MediaSource#createPeriod(MediaSource.MediaPeriodId, Allocator)} on the playback - * thread, asserting that a non-null {@link MediaPeriod} is returned. + * Calls {@link MediaSource#createPeriod(MediaSource.MediaPeriodId, Allocator, long)} with a zero + * start position on the playback thread, asserting that a non-null {@link MediaPeriod} is + * returned. * * @param periodId The id of the period to create. * @return The created {@link MediaPeriod}. */ public MediaPeriod createPeriod(final MediaPeriodId periodId) { + return createPeriod(periodId, /* startPositionUs= */ 0); + } + + /** + * Calls {@link MediaSource#createPeriod(MediaSource.MediaPeriodId, Allocator, long)} on the + * playback thread, asserting that a non-null {@link MediaPeriod} is returned. + * + * @param periodId The id of the period to create. + * @return The created {@link MediaPeriod}. + */ + public MediaPeriod createPeriod(final MediaPeriodId periodId, long startPositionUs) { final MediaPeriod[] holder = new MediaPeriod[1]; - runOnPlaybackThread(() -> holder[0] = mediaSource.createPeriod(periodId, allocator)); + runOnPlaybackThread( + () -> holder[0] = mediaSource.createPeriod(periodId, allocator, startPositionUs)); assertThat(holder[0]).isNotNull(); return holder[0]; }