From d09f872179d511fb2c620b085734dd06c1b7964e Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 19 Oct 2017 06:30:50 -0700 Subject: [PATCH] Fix positions passed to TrackSelection - When transitioning to a new period, the value of bufferedDurationUs passed to TrackSelection.updateSelectedTrack was incorrectly set to 0. It should have been set to correctly reflect buffered media in previous periods still being played out. - This change fixes the issue described above, and also propagates the playback position through to this method. The position of the next load within the period can be calculated by adding the position and bufferedDurationUs. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=172736101 --- .../exoplayer2/source/MediaPeriod.java | 4 +-- .../exoplayer2/source/SequenceableLoader.java | 4 +-- .../source/chunk/ChunkSampleStream.java | 13 ++++++-- .../exoplayer2/source/chunk/ChunkSource.java | 13 +++++--- .../AdaptiveTrackSelection.java | 3 +- .../trackselection/FixedTrackSelection.java | 3 +- .../trackselection/RandomTrackSelection.java | 3 +- .../trackselection/TrackSelection.java | 30 +++++++++++++------ .../source/dash/DefaultDashChunkSource.java | 9 +++--- .../exoplayer2/source/hls/HlsChunkSource.java | 24 +++++++++------ .../source/hls/HlsSampleStreamWrapper.java | 16 +++++++--- .../smoothstreaming/DefaultSsChunkSource.java | 9 +++--- .../exoplayer2/testutil/FakeChunkSource.java | 7 +++-- 13 files changed, 91 insertions(+), 47 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/MediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/MediaPeriod.java index 7d16f794cd..211da1d666 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/MediaPeriod.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/MediaPeriod.java @@ -177,8 +177,8 @@ public interface MediaPeriod extends SequenceableLoader { * and after preparation. * * @param positionUs The current playback position in microseconds. If playback of this period has - * not yet started, the value will be the starting position minus the duration of any media in - * previous periods still to be played. + * not yet started, the value will be the starting position in this period minus the duration + * of any media in previous periods still to be played. * @return True if progress was made, meaning that {@link #getNextLoadPositionUs()} will return * a different value than prior to the call. False otherwise. */ diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/SequenceableLoader.java b/library/core/src/main/java/com/google/android/exoplayer2/source/SequenceableLoader.java index e40ceab976..6daa1e847a 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/SequenceableLoader.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/SequenceableLoader.java @@ -53,8 +53,8 @@ public interface SequenceableLoader { * Attempts to continue loading. * * @param positionUs The current playback position in microseconds. If playback of the period to - * which this loader belongs has not yet started, the value will be the period's starting - * position minus the duration of any media in previous periods still to be played. + * which this loader belongs has not yet started, the value will be the starting position + * in the period minus the duration of any media in previous periods still to be played. * @return True if progress was made, meaning that {@link #getNextLoadPositionUs()} will return * a different value than prior to the call. False otherwise. */ diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSampleStream.java b/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSampleStream.java index f2609a0ffd..9d5a405c2f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSampleStream.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSampleStream.java @@ -351,9 +351,16 @@ public class ChunkSampleStream implements SampleStream, S return false; } - chunkSource.getNextChunk(mediaChunks.isEmpty() ? null : mediaChunks.getLast(), - pendingResetPositionUs != C.TIME_UNSET ? pendingResetPositionUs : positionUs, - nextChunkHolder); + MediaChunk previousChunk; + long loadPositionUs; + if (isPendingReset()) { + previousChunk = null; + loadPositionUs = pendingResetPositionUs; + } else { + previousChunk = mediaChunks.getLast(); + loadPositionUs = previousChunk.endTimeUs; + } + chunkSource.getNextChunk(previousChunk, positionUs, loadPositionUs, nextChunkHolder); boolean endOfStream = nextChunkHolder.endOfStream; Chunk loadable = nextChunkHolder.chunk; nextChunkHolder.clear(); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSource.java index 00865822e1..6dffc457d6 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSource.java @@ -54,12 +54,17 @@ public interface ChunkSource { * end of the stream has not been reached, the {@link ChunkHolder} is not modified. * * @param previous The most recently loaded media chunk. - * @param playbackPositionUs The current playback position. If {@code previous} is null then this - * parameter is the position from which playback is expected to start (or restart) and hence - * should be interpreted as a seek position. + * @param playbackPositionUs The current playback position in microseconds. If playback of the + * period to which this chunk source belongs has not yet started, the value will be the + * starting position in the period minus the duration of any media in previous periods still + * to be played. + * @param loadPositionUs The current load position in microseconds. If {@code previous} is null, + * this is the starting position from which chunks should be provided. Else it's equal to + * {@code previous.endTimeUs}. * @param out A holder to populate. */ - void getNextChunk(MediaChunk previous, long playbackPositionUs, ChunkHolder out); + void getNextChunk(MediaChunk previous, long playbackPositionUs, long loadPositionUs, + ChunkHolder out); /** * Called when the {@link ChunkSampleStream} has finished loading a chunk obtained from this diff --git a/library/core/src/main/java/com/google/android/exoplayer2/trackselection/AdaptiveTrackSelection.java b/library/core/src/main/java/com/google/android/exoplayer2/trackselection/AdaptiveTrackSelection.java index b999164f00..f9eddab286 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/trackselection/AdaptiveTrackSelection.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/trackselection/AdaptiveTrackSelection.java @@ -201,7 +201,8 @@ public class AdaptiveTrackSelection extends BaseTrackSelection { } @Override - public void updateSelectedTrack(long bufferedDurationUs, long availableDurationUs) { + public void updateSelectedTrack(long playbackPositionUs, long bufferedDurationUs, + long availableDurationUs) { long nowMs = SystemClock.elapsedRealtime(); // Stash the current selection, then make a new one. int currentSelectedIndex = selectedIndex; diff --git a/library/core/src/main/java/com/google/android/exoplayer2/trackselection/FixedTrackSelection.java b/library/core/src/main/java/com/google/android/exoplayer2/trackselection/FixedTrackSelection.java index ca43258e3f..50873d372d 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/trackselection/FixedTrackSelection.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/trackselection/FixedTrackSelection.java @@ -78,7 +78,8 @@ public final class FixedTrackSelection extends BaseTrackSelection { } @Override - public void updateSelectedTrack(long bufferedDurationUs, long availableDurationUs) { + public void updateSelectedTrack(long playbackPositionUs, long bufferedDurationUs, + long availableDurationUs) { // Do nothing. } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/trackselection/RandomTrackSelection.java b/library/core/src/main/java/com/google/android/exoplayer2/trackselection/RandomTrackSelection.java index b70cc8e0d5..d11344a6f6 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/trackselection/RandomTrackSelection.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/trackselection/RandomTrackSelection.java @@ -88,7 +88,8 @@ public final class RandomTrackSelection extends BaseTrackSelection { } @Override - public void updateSelectedTrack(long bufferedDurationUs, long availableDurationUs) { + public void updateSelectedTrack(long playbackPositionUs, long bufferedDurationUs, + long availableDurationUs) { // Count the number of non-blacklisted formats. long nowMs = SystemClock.elapsedRealtime(); int nonBlacklistedFormatCount = 0; diff --git a/library/core/src/main/java/com/google/android/exoplayer2/trackselection/TrackSelection.java b/library/core/src/main/java/com/google/android/exoplayer2/trackselection/TrackSelection.java index aeb1d1d6e3..ad02b6c775 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/trackselection/TrackSelection.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/trackselection/TrackSelection.java @@ -26,7 +26,7 @@ import java.util.List; * {@link TrackGroup}, and a possibly varying individual selected track from the subset. *

* Tracks belonging to the subset are exposed in decreasing bandwidth order. The individual selected - * track may change as a result of calling {@link #updateSelectedTrack(long, long)}. + * track may change as a result of calling {@link #updateSelectedTrack(long, long, long)}. */ public interface TrackSelection { @@ -125,12 +125,21 @@ public interface TrackSelection { /** * Updates the selected track. * - * @param bufferedDurationUs The duration of media currently buffered in microseconds. + * @param playbackPositionUs The current playback position in microseconds. If playback of the + * period to which this track selection belongs has not yet started, the value will be the + * starting position in the period minus the duration of any media in previous periods still + * to be played. + * @param bufferedDurationUs The duration of media currently buffered from the current playback + * position, in microseconds. Note that the next load position can be calculated as + * {@code (playbackPositionUs + bufferedDurationUs)}. * @param availableDurationUs The duration of media available for buffering from the current * playback position, in microseconds, or {@link C#TIME_UNSET} if media can be buffered - * to the end of the current period. + * to the end of the current period. Note that if not set to {@link C#TIME_UNSET}, the + * position up to which media is available for buffering can be calculated as + * {@code (playbackPositionUs + availableDurationUs)}. */ - void updateSelectedTrack(long bufferedDurationUs, long availableDurationUs); + void updateSelectedTrack(long playbackPositionUs, long bufferedDurationUs, + long availableDurationUs); /** * May be called periodically by sources that load media in discrete {@link MediaChunk}s and @@ -143,7 +152,10 @@ public interface TrackSelection { * significantly higher quality. Discarding chunks may allow faster switching to a higher quality * track in this case. * - * @param playbackPositionUs The current playback position in microseconds. + * @param playbackPositionUs The current playback position in microseconds. If playback of the + * period to which this track selection belongs has not yet started, the value will be the + * starting position in the period minus the duration of any media in previous periods still + * to be played. * @param queue The queue of buffered {@link MediaChunk}s. Must not be modified. * @return The number of chunks to retain in the queue. */ @@ -151,10 +163,10 @@ public interface TrackSelection { /** * Attempts to blacklist the track at the specified index in the selection, making it ineligible - * for selection by calls to {@link #updateSelectedTrack(long, long)} for the specified period of - * time. Blacklisting will fail if all other tracks are currently blacklisted. If blacklisting the - * currently selected track, note that it will remain selected until the next call to - * {@link #updateSelectedTrack(long, long)}. + * for selection by calls to {@link #updateSelectedTrack(long, long, long)} for the specified + * period of time. Blacklisting will fail if all other tracks are currently blacklisted. If + * blacklisting the currently selected track, note that it will remain selected until the next + * call to {@link #updateSelectedTrack(long, long, long)}. * * @param index The index of the track in the selection. * @param blacklistDurationMs The duration of time for which the track should be blacklisted, in diff --git a/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DefaultDashChunkSource.java b/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DefaultDashChunkSource.java index bb798b0af9..1eac1b5616 100644 --- a/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DefaultDashChunkSource.java +++ b/library/dash/src/main/java/com/google/android/exoplayer2/source/dash/DefaultDashChunkSource.java @@ -175,14 +175,15 @@ public class DefaultDashChunkSource implements DashChunkSource { } @Override - public void getNextChunk(MediaChunk previous, long playbackPositionUs, ChunkHolder out) { + public void getNextChunk(MediaChunk previous, long playbackPositionUs, long loadPositionUs, + ChunkHolder out) { if (fatalError != null) { return; } - long bufferedDurationUs = previous != null ? (previous.endTimeUs - playbackPositionUs) : 0; + long bufferedDurationUs = loadPositionUs - playbackPositionUs; long timeToLiveEdgeUs = resolveTimeToLiveEdgeUs(playbackPositionUs); - trackSelection.updateSelectedTrack(bufferedDurationUs, timeToLiveEdgeUs); + trackSelection.updateSelectedTrack(playbackPositionUs, bufferedDurationUs, timeToLiveEdgeUs); RepresentationHolder representationHolder = representationHolders[trackSelection.getSelectedIndex()]; @@ -237,7 +238,7 @@ public class DefaultDashChunkSource implements DashChunkSource { int segmentNum; if (previous == null) { - segmentNum = Util.constrainValue(representationHolder.getSegmentNum(playbackPositionUs), + segmentNum = Util.constrainValue(representationHolder.getSegmentNum(loadPositionUs), firstAvailableSegmentNum, lastAvailableSegmentNum); } else { segmentNum = previous.getNextChunkIndex(); diff --git a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java index b8a0c3ddb7..2b1ece4eee 100644 --- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java +++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java @@ -204,17 +204,22 @@ import java.util.List; * contain the {@link HlsUrl} that refers to the playlist that needs refreshing. * * @param previous The most recently loaded media chunk. - * @param playbackPositionUs The current playback position. If {@code previous} is null then this - * parameter is the position from which playback is expected to start (or restart) and hence - * should be interpreted as a seek position. + * @param playbackPositionUs The current playback position in microseconds. If playback of the + * period to which this chunk source belongs has not yet started, the value will be the + * starting position in the period minus the duration of any media in previous periods still + * to be played. + * @param loadPositionUs The current load position in microseconds. If {@code previous} is null, + * this is the starting position from which chunks should be provided. Else it's equal to + * {@code previous.endTimeUs}. * @param out A holder to populate. */ - public void getNextChunk(HlsMediaChunk previous, long playbackPositionUs, HlsChunkHolder out) { + public void getNextChunk(HlsMediaChunk previous, long playbackPositionUs, long loadPositionUs, + HlsChunkHolder out) { int oldVariantIndex = previous == null ? C.INDEX_UNSET : trackGroup.indexOf(previous.trackFormat); expectedPlaylistUrl = null; - long bufferedDurationUs = previous == null ? 0 : previous.endTimeUs - playbackPositionUs; + long bufferedDurationUs = loadPositionUs - playbackPositionUs; long timeToLiveEdgeUs = resolveTimeToLiveEdgeUs(playbackPositionUs); if (previous != null && !independentSegments) { // Unless segments are known to be independent, switching variant will require downloading @@ -231,7 +236,7 @@ import java.util.List; } // Select the variant. - trackSelection.updateSelectedTrack(bufferedDurationUs, timeToLiveEdgeUs); + trackSelection.updateSelectedTrack(playbackPositionUs, bufferedDurationUs, timeToLiveEdgeUs); int selectedVariantIndex = trackSelection.getSelectedIndexInTrackGroup(); boolean switchingVariant = oldVariantIndex != selectedVariantIndex; @@ -250,8 +255,8 @@ import java.util.List; // Select the chunk. int chunkMediaSequence; if (previous == null || switchingVariant) { - long targetPositionUs = previous == null ? playbackPositionUs - : independentSegments ? previous.endTimeUs : previous.startTimeUs; + long targetPositionUs = (previous == null || independentSegments) ? loadPositionUs + : previous.startTimeUs; if (!mediaPlaylist.hasEndTag && targetPositionUs >= mediaPlaylist.getEndTimeUs()) { // If the playlist is too old to contain the chunk, we need to refresh it. chunkMediaSequence = mediaPlaylist.mediaSequence + mediaPlaylist.segments.size(); @@ -437,7 +442,8 @@ import java.util.List; } @Override - public void updateSelectedTrack(long bufferedDurationUs, long availableDurationUs) { + public void updateSelectedTrack(long playbackPositionUs, long bufferedDurationUs, + long availableDurationUs) { long nowMs = SystemClock.elapsedRealtime(); if (!isBlacklisted(selectedIndex, nowMs)) { return; diff --git a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java index 946ae24d17..3eae83624b 100644 --- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java +++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java @@ -255,7 +255,8 @@ import java.util.LinkedList; // may need to be discarded. boolean primarySampleQueueDirty = false; if (!seenFirstTrackSelection) { - primaryTrackSelection.updateSelectedTrack(0, C.TIME_UNSET); + long bufferedDurationUs = positionUs < 0 ? -positionUs : 0; + primaryTrackSelection.updateSelectedTrack(positionUs, bufferedDurationUs, C.TIME_UNSET); int chunkIndex = chunkSource.getTrackGroup().indexOf(mediaChunks.getLast().trackFormat); if (primaryTrackSelection.getSelectedIndexInTrackGroup() != chunkIndex) { // This is the first selection and the chunk loaded during preparation does not match @@ -438,9 +439,16 @@ import java.util.LinkedList; return false; } - chunkSource.getNextChunk(mediaChunks.isEmpty() ? null : mediaChunks.getLast(), - pendingResetPositionUs != C.TIME_UNSET ? pendingResetPositionUs : positionUs, - nextChunkHolder); + HlsMediaChunk previousChunk; + long loadPositionUs; + if (isPendingReset()) { + previousChunk = null; + loadPositionUs = pendingResetPositionUs; + } else { + previousChunk = mediaChunks.getLast(); + loadPositionUs = previousChunk.endTimeUs; + } + chunkSource.getNextChunk(previousChunk, positionUs, loadPositionUs, nextChunkHolder); boolean endOfStream = nextChunkHolder.endOfStream; Chunk loadable = nextChunkHolder.chunk; HlsMasterPlaylist.HlsUrl playlistToLoad = nextChunkHolder.playlist; diff --git a/library/smoothstreaming/src/main/java/com/google/android/exoplayer2/source/smoothstreaming/DefaultSsChunkSource.java b/library/smoothstreaming/src/main/java/com/google/android/exoplayer2/source/smoothstreaming/DefaultSsChunkSource.java index d9743649c5..5a6493b702 100644 --- a/library/smoothstreaming/src/main/java/com/google/android/exoplayer2/source/smoothstreaming/DefaultSsChunkSource.java +++ b/library/smoothstreaming/src/main/java/com/google/android/exoplayer2/source/smoothstreaming/DefaultSsChunkSource.java @@ -149,7 +149,8 @@ public class DefaultSsChunkSource implements SsChunkSource { } @Override - public final void getNextChunk(MediaChunk previous, long playbackPositionUs, ChunkHolder out) { + public final void getNextChunk(MediaChunk previous, long playbackPositionUs, long loadPositionUs, + ChunkHolder out) { if (fatalError != null) { return; } @@ -163,7 +164,7 @@ public class DefaultSsChunkSource implements SsChunkSource { int chunkIndex; if (previous == null) { - chunkIndex = streamElement.getChunkIndex(playbackPositionUs); + chunkIndex = streamElement.getChunkIndex(loadPositionUs); } else { chunkIndex = previous.getNextChunkIndex() - currentManifestChunkOffset; if (chunkIndex < 0) { @@ -179,9 +180,9 @@ public class DefaultSsChunkSource implements SsChunkSource { return; } - long bufferedDurationUs = previous != null ? (previous.endTimeUs - playbackPositionUs) : 0; + long bufferedDurationUs = loadPositionUs - playbackPositionUs; long timeToLiveEdgeUs = resolveTimeToLiveEdgeUs(playbackPositionUs); - trackSelection.updateSelectedTrack(bufferedDurationUs, timeToLiveEdgeUs); + trackSelection.updateSelectedTrack(playbackPositionUs, bufferedDurationUs, timeToLiveEdgeUs); long chunkStartTimeUs = streamElement.getStartTimeUs(chunkIndex); long chunkEndTimeUs = chunkStartTimeUs + streamElement.getChunkDurationUs(chunkIndex); diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeChunkSource.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeChunkSource.java index 40c91a5a81..28f5926bfa 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeChunkSource.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeChunkSource.java @@ -82,9 +82,10 @@ public final class FakeChunkSource implements ChunkSource { } @Override - public void getNextChunk(MediaChunk previous, long playbackPositionUs, ChunkHolder out) { - long bufferedDurationUs = previous != null ? (previous.endTimeUs - playbackPositionUs) : 0; - trackSelection.updateSelectedTrack(bufferedDurationUs, C.TIME_UNSET); + public void getNextChunk(MediaChunk previous, long playbackPositionUs, long loadPositionUs, + ChunkHolder out) { + long bufferedDurationUs = loadPositionUs - playbackPositionUs; + trackSelection.updateSelectedTrack(playbackPositionUs, bufferedDurationUs, C.TIME_UNSET); int chunkIndex = previous == null ? dataSet.getChunkIndexByPosition(playbackPositionUs) : previous.getNextChunkIndex(); if (chunkIndex >= dataSet.getChunkCount()) {