Skip to content

Commit

Permalink
Prevent negative renderer timestamps when seeking back.
Browse files Browse the repository at this point in the history
We are currently queuing periods in a way such that the new start position
lines up with the end of the previous period (to ensure continuous playback).
However, if the start position of the new period is larger than the total of
all previously played period durations, we may end up with negative renderer
timestamps when seeking back to the beginning of this new period. Negative
timestamps should be avoided as most decoders have problems handling them
correctly.

This change forces a renderer reset if we detect such a seek to a negative
renderer time and also resets the renderer offset to 0 every time all
renderers are disabled, as this is the only time where we can savely change
the offset of an existing media period.

Also, if playback starts with an ad, we choose the content position as
renderer offset to prevent the whole issue from occurring for the seek-behind-
midroll case.

Issue:#6009
Issue:#5323
PiperOrigin-RevId: 253790054
  • Loading branch information
tonihei authored and ojw28 committed Jun 19, 2019
1 parent def01f6 commit 8286e1d
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 7 deletions.
2 changes: 2 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
([#6019](https://github.com/google/ExoPlayer/issues/6019)).
* Fix application of `maxAudioBitrate` for adaptive audio track groups
([#6006](https://github.com/google/ExoPlayer/issues/6006)).
* Fix decoding problems when seeking back after seeking beyond a mid-roll ad
([#6009](https://github.com/google/ExoPlayer/issues/6009)).

### 2.10.1 ###

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -729,13 +729,20 @@ private long seekToPeriodPosition(
newPlayingPeriodHolder = queue.advancePlayingPeriod();
}

// Disable all the renderers if the period being played is changing, or if forced.
if (oldPlayingPeriodHolder != newPlayingPeriodHolder || forceDisableRenderers) {
// Disable all renderers if the period being played is changing, if the seek results in negative
// renderer timestamps, or if forced.
if (forceDisableRenderers
|| oldPlayingPeriodHolder != newPlayingPeriodHolder
|| (newPlayingPeriodHolder != null
&& newPlayingPeriodHolder.toRendererTime(periodPositionUs) < 0)) {
for (Renderer renderer : enabledRenderers) {
disableRenderer(renderer);
}
enabledRenderers = new Renderer[0];
oldPlayingPeriodHolder = null;
if (newPlayingPeriodHolder != null) {
newPlayingPeriodHolder.setRendererOffset(/* rendererPositionOffsetUs= */ 0);
}
}

// Update the holders.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@
* Creates a new holder with information required to play it as part of a timeline.
*
* @param rendererCapabilities The renderer capabilities.
* @param rendererPositionOffsetUs The time offset of the start of the media period to provide to
* renderers.
* @param rendererPositionOffsetUs The renderer time of the start of the period, in microseconds.
* @param trackSelector The track selector.
* @param allocator The allocator.
* @param mediaSource The media source that produced the media period.
Expand All @@ -82,7 +81,7 @@ public MediaPeriodHolder(
MediaSource mediaSource,
MediaPeriodInfo info) {
this.rendererCapabilities = rendererCapabilities;
this.rendererPositionOffsetUs = rendererPositionOffsetUs - info.startPositionUs;
this.rendererPositionOffsetUs = rendererPositionOffsetUs;
this.trackSelector = trackSelector;
this.mediaSource = mediaSource;
this.uid = info.id.periodUid;
Expand Down Expand Up @@ -115,6 +114,15 @@ public long getRendererOffset() {
return rendererPositionOffsetUs;
}

/**
* Sets the renderer time of the start of the period, in microseconds.
*
* @param rendererPositionOffsetUs The new renderer position offset, in microseconds.
*/
public void setRendererOffset(long rendererPositionOffsetUs) {
this.rendererPositionOffsetUs = rendererPositionOffsetUs;
}

/** Returns start position of period in renderer time. */
public long getStartPositionRendererTime() {
return info.startPositionUs + rendererPositionOffsetUs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ public MediaPeriod enqueueNextMediaPeriod(
MediaPeriodInfo info) {
long rendererPositionOffsetUs =
loading == null
? info.startPositionUs
: (loading.getRendererOffset() + loading.info.durationUs);
? (info.id.isAd() ? info.contentPositionUs : 0)
: (loading.getRendererOffset() + loading.info.durationUs - info.startPositionUs);
MediaPeriodHolder newPeriodHolder =
new MediaPeriodHolder(
rendererCapabilities,
Expand Down

0 comments on commit 8286e1d

Please sign in to comment.