Skip to content

Commit

Permalink
Add start position to MediaSource.createPeriod.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tonihei authored and ojw28 committed Jan 17, 2019
1 parent 76baa57 commit 22413b8
Show file tree
Hide file tree
Showing 17 changed files with 74 additions and 46 deletions.
4 changes: 4 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 ###

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ public void maybeThrowSourceInfoRefreshError() throws IOException {
}

@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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public MediaPeriodHolder(
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);
}

/**
Expand Down Expand Up @@ -399,8 +399,8 @@ private boolean isLoadingMediaPeriod() {

/** 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,10 @@ public void maybeThrowSourceInfoRefreshError() throws IOException {
}

@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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -438,15 +438,17 @@ public void maybeThrowSourceInfoRefreshError() throws IOException {
}

@Override
public final MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) {
public final MediaPeriod createPeriod(
MediaPeriodId id, Allocator allocator, long startPositionUs) {
Object mediaSourceHolderUid = getMediaSourceHolderUid(id.periodUid);
MediaSourceHolder holder = mediaSourceByUid.get(mediaSourceHolderUid);
if (holder == null) {
// Stale event. The media source has already been removed.
holder = new MediaSourceHolder(new DummyMediaSource());
holder.hasStartedPreparing = true;
}
DeferredMediaPeriod mediaPeriod = new DeferredMediaPeriod(holder.mediaSource, id, allocator);
DeferredMediaPeriod mediaPeriod =
new DeferredMediaPeriod(holder.mediaSource, id, allocator, startPositionUs);
mediaSourceByMediaPeriod.put(mediaPeriod, holder);
holder.activeMediaPeriods.add(mediaPeriod);
if (!holder.hasStartedPreparing) {
Expand Down Expand Up @@ -1144,7 +1146,7 @@ public void maybeThrowSourceInfoRefreshError() throws IOException {
}

@Override
public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) {
public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long startPositionUs) {
throw new UnsupportedOperationException();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

/**
* Media period that wraps a media source and defers calling its {@link
* MediaSource#createPeriod(MediaPeriodId, Allocator)} method until {@link
* MediaSource#createPeriod(MediaPeriodId, Allocator, long)} method until {@link
* #createPeriod(MediaPeriodId)} has been called. This is useful if you need to return a media
* period immediately but the media source that should create it is not yet prepared.
*/
Expand Down Expand Up @@ -60,11 +60,14 @@ public interface PrepareErrorListener {
* @param mediaSource The media source to wrap.
* @param id The identifier used to create the deferred media period.
* @param allocator The allocator used to create the media period.
* @param preparePositionUs The expected start position, in microseconds.
*/
public DeferredMediaPeriod(MediaSource mediaSource, MediaPeriodId id, Allocator allocator) {
public DeferredMediaPeriod(
MediaSource mediaSource, MediaPeriodId id, Allocator allocator, long preparePositionUs) {
this.id = id;
this.allocator = allocator;
this.mediaSource = mediaSource;
this.preparePositionUs = preparePositionUs;
preparePositionOverrideUs = C.TIME_UNSET;
}

Expand All @@ -86,28 +89,25 @@ public long getPreparePositionUs() {

/**
* Overrides the default prepare position at which to prepare the media period. This value is only
* used if the call to {@link MediaPeriod#prepare(Callback, long)} is being deferred.
* used if called before {@link #createPeriod(MediaPeriodId)}.
*
* @param defaultPreparePositionUs The default prepare position to use, in microseconds.
* @param preparePositionUs The default prepare position to use, in microseconds.
*/
public void overridePreparePositionUs(long defaultPreparePositionUs) {
preparePositionOverrideUs = defaultPreparePositionUs;
public void overridePreparePositionUs(long preparePositionUs) {
preparePositionOverrideUs = preparePositionUs;
}

/**
* Calls {@link MediaSource#createPeriod(MediaPeriodId, Allocator)} on the wrapped source then
* prepares it if {@link #prepare(Callback, long)} has been called. Call {@link #releasePeriod()}
* to release the period.
* Calls {@link MediaSource#createPeriod(MediaPeriodId, Allocator, long)} on the wrapped source
* then prepares it if {@link #prepare(Callback, long)} has been called. Call {@link
* #releasePeriod()} to release the period.
*
* @param id The identifier that should be used to create the media period from the media source.
*/
public void createPeriod(MediaPeriodId id) {
mediaPeriod = mediaSource.createPeriod(id, allocator);
long preparePositionUs = getPreparePositionWithOverride(this.preparePositionUs);
mediaPeriod = mediaSource.createPeriod(id, allocator, preparePositionUs);
if (callback != null) {
long preparePositionUs =
preparePositionOverrideUs != C.TIME_UNSET
? preparePositionOverrideUs
: this.preparePositionUs;
mediaPeriod.prepare(this, preparePositionUs);
}
}
Expand All @@ -124,9 +124,8 @@ public void releasePeriod() {
@Override
public void prepare(Callback callback, long preparePositionUs) {
this.callback = callback;
this.preparePositionUs = preparePositionUs;
if (mediaPeriod != null) {
mediaPeriod.prepare(this, preparePositionUs);
mediaPeriod.prepare(this, getPreparePositionWithOverride(this.preparePositionUs));
}
}

Expand Down Expand Up @@ -217,4 +216,9 @@ public void onPrepared(MediaPeriod mediaPeriod) {
callback.onPrepared(this);
}

private long getPreparePositionWithOverride(long preparePositionUs) {
return preparePositionOverrideUs != C.TIME_UNSET
? preparePositionOverrideUs
: preparePositionUs;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ public void maybeThrowSourceInfoRefreshError() throws IOException {
}

@Override
public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) {
public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long startPositionUs) {
DataSource dataSource = dataSourceFactory.createDataSource();
if (transferListener != null) {
dataSource.addTransferListener(transferListener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,15 @@ public void prepareSourceInternal(@Nullable TransferListener mediaTransferListen
}

@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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
* on the {@link SourceInfoRefreshListener}s passed to {@link
* #prepareSource(SourceInfoRefreshListener, TransferListener)}.
* <li>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.
* </ul>
*
* All methods are called on the player's internal playback thread, as described in the {@link
Expand Down Expand Up @@ -261,9 +261,10 @@ void prepareSource(
*
* @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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,13 @@ public void maybeThrowSourceInfoRefreshError() throws IOException {
}

@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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ public void maybeThrowSourceInfoRefreshError() throws IOException {
}

@Override
public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) {
public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long startPositionUs) {
return new SingleSampleMediaPeriod(
dataSpec,
dataSourceFactory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ public void prepareSourceInternal(@Nullable TransferListener mediaTransferListen
}

@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;
Expand All @@ -353,7 +353,8 @@ public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) {
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<DeferredMediaPeriod> mediaPeriods = deferredMediaPeriodByAdMediaSource.get(mediaSource);
Expand All @@ -369,7 +370,8 @@ public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) {
}
return deferredMediaPeriod;
} else {
DeferredMediaPeriod mediaPeriod = new DeferredMediaPeriod(contentMediaSource, id, allocator);
DeferredMediaPeriod mediaPeriod =
new DeferredMediaPeriod(contentMediaSource, id, allocator, startPositionUs);
mediaPeriod.createPeriod(id);
return mediaPeriod;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,8 @@ public void maybeThrowSourceInfoRefreshError() throws IOException {
}

@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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ public void maybeThrowSourceInfoRefreshError() throws IOException {
}

@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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ public void maybeThrowSourceInfoRefreshError() throws IOException {
}

@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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public void maybeThrowSourceInfoRefreshError() throws IOException {
}

@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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,28 @@ public Timeline prepareSource() throws IOException {
}

/**
* 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];
}
Expand Down

0 comments on commit 22413b8

Please sign in to comment.