Skip to content

Commit

Permalink
Add large renderer position offset.
Browse files Browse the repository at this point in the history 
This helps to prevent issues where decoders can't handle negative
timestamps. In particular it avoids issues when the media accidentally
or intentionally starts with small negative timestamps. But it also
helps to prevent other renderer resets at a later point, for example
if a live stream with a large start offset is enqueued in the playlist.

#minor-release

PiperOrigin-RevId: 406786977
  • Loading branch information
@tonihei
tonihei committed Nov 1, 2021
1 parent 4c40e80 commit 4e6960e
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 79 deletions.
9 changes: 5 additions & 4 deletions library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1266,7 +1266,8 @@ private long seekToPeriodPosition(
queue.advancePlayingPeriod();
}
queue.removeAfter(newPlayingPeriodHolder);
newPlayingPeriodHolder.setRendererOffset(/* rendererPositionOffsetUs= */ 0);
newPlayingPeriodHolder.setRendererOffset(
MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US);
enableRenderers();
}
}
Expand Down Expand Up @@ -1299,7 +1300,7 @@ private void resetRendererPosition(long periodPositionUs) throws ExoPlaybackExce
MediaPeriodHolder playingMediaPeriod = queue.getPlayingPeriod();
rendererPositionUs =
playingMediaPeriod == null
? periodPositionUs
? MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US + periodPositionUs
: playingMediaPeriod.toRendererTime(periodPositionUs);
mediaClock.resetPosition(rendererPositionUs);
for (Renderer renderer : renderers) {
Expand Down Expand Up @@ -1375,7 +1376,7 @@ private void resetInternal(
pendingRecoverableRendererError = null;
isRebuffering = false;
mediaClock.stop();
rendererPositionUs = 0;
rendererPositionUs = MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US;
for (Renderer renderer : renderers) {
try {
disableRenderer(renderer);
Expand Down Expand Up @@ -1963,7 +1964,7 @@ private void maybeUpdateLoadingPeriod() throws ExoPlaybackException {
emptyTrackSelectorResult);
mediaPeriodHolder.mediaPeriod.prepare(this, info.startPositionUs);
if (queue.getPlayingPeriod() == mediaPeriodHolder) {
resetRendererPosition(mediaPeriodHolder.getStartPositionRendererTime());
resetRendererPosition(info.startPositionUs);
}
handleLoadingMediaPeriodChanged(/* loadingTrackSelectionChanged= */ false);
}
Expand Down
24 changes: 21 additions & 3 deletions library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodQueue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,26 @@
*/
/* package */ final class MediaPeriodQueue {

/**
* Initial renderer position offset used for the first item in the queue, in microseconds.
*
* <p>Choosing a positive value, larger than any reasonable single media duration, ensures three
* things:
*
* <ul>
* <li>Media that accidentally or intentionally starts with small negative timestamps doesn't
* send samples with negative timestamps to decoders. This makes rendering more robust as
* many decoders are known to have problems with negative timestamps.
* <li>Enqueueing media after the initial item with a non-zero start offset (e.g. content after
* ad breaks or live streams) is virtually guaranteed to stay in the positive timestamp
* range even when seeking back. This prevents renderer resets that are required if the
* allowed timestamp range may become negative.
* <li>Choosing a large value with zeros at all relevant digits simplifies debugging as the
* original timestamp of the media is still visible.
* </ul>
*/
public static final long INITIAL_RENDERER_POSITION_OFFSET_US = 1_000_000_000_000L;

/**
* Limits the maximum number of periods to buffer ahead of the current playing period. The
* buffering policy normally prevents buffering too far ahead, but the policy could allow too many
Expand Down Expand Up @@ -163,9 +183,7 @@ public MediaPeriodHolder enqueueNextMediaPeriodHolder(
TrackSelectorResult emptyTrackSelectorResult) {
long rendererPositionOffsetUs =
loading == null
? (info.id.isAd() && info.requestedContentPositionUs != C.TIME_UNSET
? info.requestedContentPositionUs
: 0)
? INITIAL_RENDERER_POSITION_OFFSET_US
: (loading.getRendererOffset() + loading.info.durationUs - info.startPositionUs);
MediaPeriodHolder newPeriodHolder =
new MediaPeriodHolder(
Expand Down
47 changes: 33 additions & 14 deletions library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,10 +493,13 @@ public void getNextMediaPeriodInfo_inMultiPeriodWindow_returnsCorrectMediaPeriod
// Change position of first ad (= change duration of playing content before first ad).
updateAdPlaybackStateAndTimeline(/* adGroupTimesUs...= */ FIRST_AD_START_TIME_US - 2000);
setAdGroupLoaded(/* adGroupIndex= */ 0);
long maxRendererReadPositionUs = FIRST_AD_START_TIME_US - 3000;
long maxRendererReadPositionUs =
MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US + FIRST_AD_START_TIME_US - 3000;
boolean changeHandled =
mediaPeriodQueue.updateQueuedPeriods(
playbackInfo.timeline, /* rendererPositionUs= */ 0, maxRendererReadPositionUs);
playbackInfo.timeline,
/* rendererPositionUs= */ MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US,
maxRendererReadPositionUs);

assertThat(changeHandled).isTrue();
assertThat(getQueueLength()).isEqualTo(1);
Expand All @@ -518,10 +521,13 @@ public void getNextMediaPeriodInfo_inMultiPeriodWindow_returnsCorrectMediaPeriod
// Change position of first ad (= change duration of playing content before first ad).
updateAdPlaybackStateAndTimeline(/* adGroupTimesUs...= */ FIRST_AD_START_TIME_US - 2000);
setAdGroupLoaded(/* adGroupIndex= */ 0);
long maxRendererReadPositionUs = FIRST_AD_START_TIME_US - 1000;
long maxRendererReadPositionUs =
MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US + FIRST_AD_START_TIME_US - 1000;
boolean changeHandled =
mediaPeriodQueue.updateQueuedPeriods(
playbackInfo.timeline, /* rendererPositionUs= */ 0, maxRendererReadPositionUs);
playbackInfo.timeline,
/* rendererPositionUs= */ MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US,
maxRendererReadPositionUs);

assertThat(changeHandled).isFalse();
assertThat(getQueueLength()).isEqualTo(1);
Expand Down Expand Up @@ -552,10 +558,13 @@ public void getNextMediaPeriodInfo_inMultiPeriodWindow_returnsCorrectMediaPeriod
.withIsServerSideInserted(/* adGroupIndex= */ 0, /* isServerSideInserted= */ true);
updateTimeline();
setAdGroupLoaded(/* adGroupIndex= */ 0);
long maxRendererReadPositionUs = FIRST_AD_START_TIME_US - 1000;
long maxRendererReadPositionUs =
MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US + FIRST_AD_START_TIME_US - 1000;
boolean changeHandled =
mediaPeriodQueue.updateQueuedPeriods(
playbackInfo.timeline, /* rendererPositionUs= */ 0, maxRendererReadPositionUs);
playbackInfo.timeline,
/* rendererPositionUs= */ MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US,
maxRendererReadPositionUs);

assertThat(changeHandled).isTrue();
assertThat(getQueueLength()).isEqualTo(1);
Expand Down Expand Up @@ -583,7 +592,9 @@ public void getNextMediaPeriodInfo_inMultiPeriodWindow_returnsCorrectMediaPeriod
setAdGroupLoaded(/* adGroupIndex= */ 1);
boolean changeHandled =
mediaPeriodQueue.updateQueuedPeriods(
playbackInfo.timeline, /* rendererPositionUs= */ 0, /* maxRendererReadPositionUs= */ 0);
playbackInfo.timeline,
/* rendererPositionUs= */ MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US,
/* maxRendererReadPositionUs= */ MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US);

assertThat(changeHandled).isTrue();
assertThat(getQueueLength()).isEqualTo(3);
Expand All @@ -608,11 +619,13 @@ public void getNextMediaPeriodInfo_inMultiPeriodWindow_returnsCorrectMediaPeriod
/* adGroupTimesUs...= */ FIRST_AD_START_TIME_US, SECOND_AD_START_TIME_US - 1000);
setAdGroupLoaded(/* adGroupIndex= */ 0);
setAdGroupLoaded(/* adGroupIndex= */ 1);
long maxRendererReadPositionUs =
MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US + FIRST_AD_START_TIME_US;
boolean changeHandled =
mediaPeriodQueue.updateQueuedPeriods(
playbackInfo.timeline,
/* rendererPositionUs= */ 0,
/* maxRendererReadPositionUs= */ FIRST_AD_START_TIME_US);
/* rendererPositionUs= */ MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US,
maxRendererReadPositionUs);

assertThat(changeHandled).isFalse();
assertThat(getQueueLength()).isEqualTo(3);
Expand All @@ -636,11 +649,14 @@ public void getNextMediaPeriodInfo_inMultiPeriodWindow_returnsCorrectMediaPeriod
/* adGroupTimesUs...= */ FIRST_AD_START_TIME_US, SECOND_AD_START_TIME_US - 1000);
setAdGroupLoaded(/* adGroupIndex= */ 0);
setAdGroupLoaded(/* adGroupIndex= */ 1);
long readingPositionAtStartOfContentBetweenAds = FIRST_AD_START_TIME_US + AD_DURATION_US;
long readingPositionAtStartOfContentBetweenAds =
MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US
+ FIRST_AD_START_TIME_US
+ AD_DURATION_US;
boolean changeHandled =
mediaPeriodQueue.updateQueuedPeriods(
playbackInfo.timeline,
/* rendererPositionUs= */ 0,
/* rendererPositionUs= */ MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US,
/* maxRendererReadPositionUs= */ readingPositionAtStartOfContentBetweenAds);

assertThat(changeHandled).isTrue();
Expand All @@ -665,11 +681,14 @@ public void getNextMediaPeriodInfo_inMultiPeriodWindow_returnsCorrectMediaPeriod
/* adGroupTimesUs...= */ FIRST_AD_START_TIME_US, SECOND_AD_START_TIME_US - 1000);
setAdGroupLoaded(/* adGroupIndex= */ 0);
setAdGroupLoaded(/* adGroupIndex= */ 1);
long readingPositionAtEndOfContentBetweenAds = SECOND_AD_START_TIME_US + AD_DURATION_US;
long readingPositionAtEndOfContentBetweenAds =
MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US
+ SECOND_AD_START_TIME_US
+ AD_DURATION_US;
boolean changeHandled =
mediaPeriodQueue.updateQueuedPeriods(
playbackInfo.timeline,
/* rendererPositionUs= */ 0,
/* rendererPositionUs= */ MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US,
/* maxRendererReadPositionUs= */ readingPositionAtEndOfContentBetweenAds);

assertThat(changeHandled).isFalse();
Expand Down Expand Up @@ -697,7 +716,7 @@ public void getNextMediaPeriodInfo_inMultiPeriodWindow_returnsCorrectMediaPeriod
boolean changeHandled =
mediaPeriodQueue.updateQueuedPeriods(
playbackInfo.timeline,
/* rendererPositionUs= */ 0,
/* rendererPositionUs= */ MediaPeriodQueue.INITIAL_RENDERER_POSITION_OFFSET_US,
/* maxRendererReadPositionUs= */ C.TIME_END_OF_SOURCE);

assertThat(changeHandled).isFalse();
Expand Down
1 change: 1 addition & 0 deletions ...mer/src/main/java/com/google/android/exoplayer2/transformer/TransformerAudioRenderer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ private boolean feedDecoderFromInput(MediaCodecAdapterWrapper decoder) {
int result = readSource(getFormatHolder(), decoderInputBuffer, /* readFlags= */ 0);
switch (result) {
case C.RESULT_BUFFER_READ:
decoderInputBuffer.timeUs -= streamOffsetUs;
mediaClock.updateTimeForTrackType(getTrackType(), decoderInputBuffer.timeUs);
decoderInputBuffer.flip();
decoder.queueInputBuffer(decoderInputBuffer);
Expand Down
7 changes: 7 additions & 0 deletions ...rmer/src/main/java/com/google/android/exoplayer2/transformer/TransformerBaseRenderer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
protected final Transformation transformation;

protected boolean isRendererStarted;
protected long streamOffsetUs;

public TransformerBaseRenderer(
int trackType,
Expand All @@ -46,6 +47,12 @@ public TransformerBaseRenderer(
this.transformation = transformation;
}

@Override
protected void onStreamChanged(Format[] formats, long startPositionUs, long offsetUs)
throws ExoPlaybackException {
this.streamOffsetUs = offsetUs;
}

@Override
@C.FormatSupport
public final int supportsFormat(Format format) {
Expand Down
1 change: 1 addition & 0 deletions ...c/main/java/com/google/android/exoplayer2/transformer/TransformerMuxingVideoRenderer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ private boolean readAndTransformBuffer() {
muxerWrapper.endTrack(getTrackType());
return false;
}
buffer.timeUs -= streamOffsetUs;
mediaClock.updateTimeForTrackType(getTrackType(), buffer.timeUs);
ByteBuffer data = checkNotNull(buffer.data);
data.flip();
Expand Down
1 change: 1 addition & 0 deletions ...n/java/com/google/android/exoplayer2/transformer/TransformerTranscodingVideoRenderer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ private boolean feedDecoderFromInput(MediaCodecAdapterWrapper decoder) {
case C.RESULT_FORMAT_READ:
throw new IllegalStateException("Format changes are not supported.");
case C.RESULT_BUFFER_READ:
decoderInputBuffer.timeUs -= streamOffsetUs;
mediaClock.updateTimeForTrackType(getTrackType(), decoderInputBuffer.timeUs);
ByteBuffer data = checkNotNull(decoderInputBuffer.data);
data.flip();
Expand Down
58 changes: 29 additions & 29 deletions testdata/src/test/assets/audiosinkdumps/mka/bear-flac-16bit.mka.audiosink.dump
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,89 +3,89 @@ config:
channelCount = 2
sampleRate = 48000
buffer:
time = 1000
time = 1000000001000
data = 1217833679
buffer:
time = 97000
time = 1000000097000
data = 558614672
buffer:
time = 193000
time = 1000000193000
data = -709714787
buffer:
time = 289000
time = 1000000289000
data = 1367870571
buffer:
time = 385000
time = 1000000385000
data = -141229457
buffer:
time = 481000
time = 1000000481000
data = 1287758361
buffer:
time = 577000
time = 1000000577000
data = 1125289147
buffer:
time = 673000
time = 1000000673000
data = -1677383475
buffer:
time = 769000
time = 1000000769000
data = 2130742861
buffer:
time = 865000
time = 1000000865000
data = -1292320253
buffer:
time = 961000
time = 1000000961000
data = -456587163
buffer:
time = 1057000
time = 1000001057000
data = 748981534
buffer:
time = 1153000
time = 1000001153000
data = 1550456016
buffer:
time = 1249000
time = 1000001249000
data = 1657906039
buffer:
time = 1345000
time = 1000001345000
data = -762677083
buffer:
time = 1441000
time = 1000001441000
data = -1343810763
buffer:
time = 1537000
time = 1000001537000
data = 1137318783
buffer:
time = 1633000
time = 1000001633000
data = -1891318229
buffer:
time = 1729000
time = 1000001729000
data = -472068495
buffer:
time = 1825000
time = 1000001825000
data = 832315001
buffer:
time = 1921000
time = 1000001921000
data = 2054935175
buffer:
time = 2017000
time = 1000002017000
data = 57921641
buffer:
time = 2113000
time = 1000002113000
data = 2132759067
buffer:
time = 2209000
time = 1000002209000
data = -1742540521
buffer:
time = 2305000
time = 1000002305000
data = 1657024301
buffer:
time = 2401000
time = 1000002401000
data = -585080145
buffer:
time = 2497000
time = 1000002497000
data = 427271397
buffer:
time = 2593000
time = 1000002593000
data = -364201340
buffer:
time = 2689000
time = 1000002689000
data = -627965287
Loading

2 comments on commit 4e6960e

@xiaos
Copy link

@xiaos xiaos commented on 4e6960e Nov 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the INITIAL_RENDERER_POSITION_OFFSET_US will shift the getCurrentPositionUs() in DefaultAudioSink.java.

@tonihei
Copy link
Collaborator Author

@tonihei tonihei commented on 4e6960e Nov 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the INITIAL_RENDERER_POSITION_OFFSET_US will shift the getCurrentPositionUs() in DefaultAudioSink.java.

This is working as intended. If you think this change causes problems somewhere, please open a new issue. Note that the AudioSink can't make any assumptions about the timestamps it receives as this is set by the player and may change depending on the media and where in the playlist the player currently is.

Please sign in to comment.