From 0943886cbd7ee6c4e33ed731ddff0b23e5699abd Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 7 Jul 2020 12:21:13 +0100 Subject: [PATCH] Use last queue format instead of previous decision to select new track We currently use the saved selectionIndex to base our new track selection decision on. This index might be stale if the previous selection didn't result in a queue update (e.g. when loading live streams where the new chunk isn't available yet). Fix this by using the format of the last chunk to make the new decision. Issue: #7582 PiperOrigin-RevId: 319957980 --- .../AdaptiveTrackSelection.java | 29 ++++----- .../trackselection/BaseTrackSelection.java | 17 +---- .../AdaptiveTrackSelectionTest.java | 62 +++++++++++++++++++ .../exoplayer2/testutil/FakeMediaChunk.java | 23 +++++-- 4 files changed, 96 insertions(+), 35 deletions(-) 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 db616c7ffa..dd8a13e726 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 @@ -428,33 +428,30 @@ public class AdaptiveTrackSelection extends BaseTrackSelection { return; } - // Stash the current selection, then make a new one. - int currentSelectedIndex = selectedIndex; - selectedIndex = determineIdealSelectedIndex(nowMs); - if (selectedIndex == currentSelectedIndex) { - return; - } - - if (!isBlacklisted(currentSelectedIndex, nowMs)) { - // Revert back to the current selection if conditions are not suitable for switching. - Format currentFormat = getFormat(currentSelectedIndex); - Format selectedFormat = getFormat(selectedIndex); + int previousSelectedIndex = + queue.isEmpty() ? selectedIndex : indexOf(Iterables.getLast(queue).trackFormat); + int previousReason = queue.isEmpty() ? reason : Iterables.getLast(queue).trackSelectionReason; + int newSelectedIndex = determineIdealSelectedIndex(nowMs); + if (!isBlacklisted(previousSelectedIndex, nowMs)) { + // Revert back to the previous selection if conditions are not suitable for switching. + Format currentFormat = getFormat(previousSelectedIndex); + Format selectedFormat = getFormat(newSelectedIndex); if (selectedFormat.bitrate > currentFormat.bitrate && bufferedDurationUs < minDurationForQualityIncreaseUs(availableDurationUs)) { // The selected track is a higher quality, but we have insufficient buffer to safely switch // up. Defer switching up for now. - selectedIndex = currentSelectedIndex; + newSelectedIndex = previousSelectedIndex; } else if (selectedFormat.bitrate < currentFormat.bitrate && bufferedDurationUs >= maxDurationForQualityDecreaseUs) { // The selected track is a lower quality, but we have sufficient buffer to defer switching // down for now. - selectedIndex = currentSelectedIndex; + newSelectedIndex = previousSelectedIndex; } } // If we adapted, update the trigger. - if (selectedIndex != currentSelectedIndex) { - reason = C.SELECTION_REASON_ADAPTIVE; - } + reason = + newSelectedIndex == previousSelectedIndex ? previousReason : C.SELECTION_REASON_ADAPTIVE; + selectedIndex = newSelectedIndex; } @Override diff --git a/library/core/src/main/java/com/google/android/exoplayer2/trackselection/BaseTrackSelection.java b/library/core/src/main/java/com/google/android/exoplayer2/trackselection/BaseTrackSelection.java index 0fe001df0a..33af011f63 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/trackselection/BaseTrackSelection.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/trackselection/BaseTrackSelection.java @@ -24,7 +24,6 @@ import com.google.android.exoplayer2.source.chunk.MediaChunk; import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Util; import java.util.Arrays; -import java.util.Comparator; import java.util.List; /** @@ -69,7 +68,8 @@ public abstract class BaseTrackSelection implements TrackSelection { for (int i = 0; i < tracks.length; i++) { formats[i] = group.getFormat(tracks[i]); } - Arrays.sort(formats, new DecreasingBandwidthComparator()); + // Sort in order of decreasing bandwidth. + Arrays.sort(formats, (a, b) -> b.bitrate - a.bitrate); // Set the format indices in the same order. this.tracks = new int[length]; for (int i = 0; i < length; i++) { @@ -199,17 +199,4 @@ public abstract class BaseTrackSelection implements TrackSelection { BaseTrackSelection other = (BaseTrackSelection) obj; return group == other.group && Arrays.equals(tracks, other.tracks); } - - /** - * Sorts {@link Format} objects in order of decreasing bandwidth. - */ - private static final class DecreasingBandwidthComparator implements Comparator { - - @Override - public int compare(Format a, Format b) { - return b.bitrate - a.bitrate; - } - - } - } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/trackselection/AdaptiveTrackSelectionTest.java b/library/core/src/test/java/com/google/android/exoplayer2/trackselection/AdaptiveTrackSelectionTest.java index ea95e50d96..532b3949f1 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/trackselection/AdaptiveTrackSelectionTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/trackselection/AdaptiveTrackSelectionTest.java @@ -33,6 +33,7 @@ import com.google.android.exoplayer2.testutil.FakeMediaChunk; import com.google.android.exoplayer2.trackselection.TrackSelection.Definition; import com.google.android.exoplayer2.upstream.BandwidthMeter; import com.google.android.exoplayer2.util.MimeTypes; +import com.google.common.collect.ImmutableList; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -328,6 +329,67 @@ public final class AdaptiveTrackSelectionTest { assertThat(newSize).isEqualTo(2); } + @Test + public void updateSelectedTrack_usesFormatOfLastChunkInTheQueueForSelection() { + Format format1 = videoFormat(/* bitrate= */ 500, /* width= */ 320, /* height= */ 240); + Format format2 = videoFormat(/* bitrate= */ 1000, /* width= */ 640, /* height= */ 480); + TrackGroup trackGroup = new TrackGroup(format1, format2); + adaptiveTrackSelection = + new AdaptiveTrackSelection.Factory( + /* minDurationForQualityIncreaseMs= */ 10_000, + /* maxDurationForQualityDecreaseMs= */ 10_000, + /* minDurationToRetainAfterDiscardMs= */ 25_000, + /* bandwidthFraction= */ 1f) + .createAdaptiveTrackSelection( + trackGroup, + mockBandwidthMeter, + /* tracks= */ new int[] {0, 1}, + /* totalFixedTrackBandwidth= */ 0); + + // Make initial selection. + when(mockBandwidthMeter.getBitrateEstimate()).thenReturn(1000L); + prepareTrackSelection(adaptiveTrackSelection); + + assertThat(adaptiveTrackSelection.getSelectedFormat()).isEqualTo(format2); + assertThat(adaptiveTrackSelection.getSelectionReason()).isEqualTo(C.SELECTION_REASON_INITIAL); + + // Ensure that track selection wants to switch down due to low bandwidth. + FakeMediaChunk chunk1 = + new FakeMediaChunk( + format2, /* startTimeUs= */ 0, /* endTimeUs= */ 2_000_000, C.SELECTION_REASON_INITIAL); + FakeMediaChunk chunk2 = + new FakeMediaChunk( + format2, + /* startTimeUs= */ 2_000_000, + /* endTimeUs= */ 4_000_000, + C.SELECTION_REASON_INITIAL); + List queue = ImmutableList.of(chunk1, chunk2); + when(mockBandwidthMeter.getBitrateEstimate()).thenReturn(500L); + adaptiveTrackSelection.updateSelectedTrack( + /* playbackPositionUs= */ 0, + /* bufferedDurationUs= */ 4_000_000, + /* availableDurationUs= */ C.TIME_UNSET, + queue, + /* mediaChunkIterators= */ THREE_EMPTY_MEDIA_CHUNK_ITERATORS); + + assertThat(adaptiveTrackSelection.getSelectedFormat()).isEqualTo(format1); + assertThat(adaptiveTrackSelection.getSelectionReason()).isEqualTo(C.SELECTION_REASON_ADAPTIVE); + + // Assert that an improved bandwidth selects the last chunk's format and ignores the previous + // decision. Switching up from the previous decision wouldn't be possible yet because the + // buffered duration is less than minDurationForQualityIncreaseMs. + when(mockBandwidthMeter.getBitrateEstimate()).thenReturn(1000L); + adaptiveTrackSelection.updateSelectedTrack( + /* playbackPositionUs= */ 0, + /* bufferedDurationUs= */ 4_000_000, + /* availableDurationUs= */ C.TIME_UNSET, + queue, + /* mediaChunkIterators= */ THREE_EMPTY_MEDIA_CHUNK_ITERATORS); + + assertThat(adaptiveTrackSelection.getSelectedFormat()).isEqualTo(format2); + assertThat(adaptiveTrackSelection.getSelectionReason()).isEqualTo(C.SELECTION_REASON_INITIAL); + } + private AdaptiveTrackSelection adaptiveTrackSelection(TrackGroup trackGroup) { return adaptiveTrackSelectionWithMinDurationForQualityIncreaseMs( trackGroup, AdaptiveTrackSelection.DEFAULT_MIN_DURATION_FOR_QUALITY_INCREASE_MS); diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaChunk.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaChunk.java index 0286531fc8..f81c866019 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaChunk.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaChunk.java @@ -28,16 +28,31 @@ public final class FakeMediaChunk extends MediaChunk { private static final DataSource DATA_SOURCE = new DefaultHttpDataSource("TEST_AGENT"); + /** + * Creates a fake media chunk. + * + * @param trackFormat The {@link Format}. + * @param startTimeUs The start time of the media, in microseconds. + * @param endTimeUs The end time of the media, in microseconds. + */ public FakeMediaChunk(Format trackFormat, long startTimeUs, long endTimeUs) { - this(new DataSpec(Uri.EMPTY), trackFormat, startTimeUs, endTimeUs); + this(trackFormat, startTimeUs, endTimeUs, C.SELECTION_REASON_UNKNOWN); } - public FakeMediaChunk(DataSpec dataSpec, Format trackFormat, long startTimeUs, long endTimeUs) { + /** + * Creates a fake media chunk. + * + * @param trackFormat The {@link Format}. + * @param startTimeUs The start time of the media, in microseconds. + * @param endTimeUs The end time of the media, in microseconds. + * @param selectionReason The reason for selecting this format. + */ + public FakeMediaChunk(Format trackFormat, long startTimeUs, long endTimeUs, int selectionReason) { super( DATA_SOURCE, - dataSpec, + new DataSpec(Uri.EMPTY), trackFormat, - C.SELECTION_REASON_ADAPTIVE, + selectionReason, /* trackSelectionData= */ null, startTimeUs, endTimeUs,