From 75eab31d79aae73196a60fb24474da2a36855c4a Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 7 Jul 2020 16:50:44 +0100 Subject: [PATCH] Rollback of https://github.com/google/ExoPlayer/commit/0943886cbd7ee6c4e33ed731ddff0b23e5699abd *** Original commit *** 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: 319991676 --- .../AdaptiveTrackSelection.java | 29 +++++---- .../trackselection/BaseTrackSelection.java | 17 ++++- .../AdaptiveTrackSelectionTest.java | 62 ------------------- .../exoplayer2/testutil/FakeMediaChunk.java | 23 ++----- 4 files changed, 35 insertions(+), 96 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 dd8a13e726..db616c7ffa 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,30 +428,33 @@ public class AdaptiveTrackSelection extends BaseTrackSelection { return; } - 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); + // 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); 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. - newSelectedIndex = previousSelectedIndex; + selectedIndex = currentSelectedIndex; } 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. - newSelectedIndex = previousSelectedIndex; + selectedIndex = currentSelectedIndex; } } // If we adapted, update the trigger. - reason = - newSelectedIndex == previousSelectedIndex ? previousReason : C.SELECTION_REASON_ADAPTIVE; - selectedIndex = newSelectedIndex; + if (selectedIndex != currentSelectedIndex) { + reason = C.SELECTION_REASON_ADAPTIVE; + } } @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 33af011f63..0fe001df0a 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,6 +24,7 @@ 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; /** @@ -68,8 +69,7 @@ public abstract class BaseTrackSelection implements TrackSelection { for (int i = 0; i < tracks.length; i++) { formats[i] = group.getFormat(tracks[i]); } - // Sort in order of decreasing bandwidth. - Arrays.sort(formats, (a, b) -> b.bitrate - a.bitrate); + Arrays.sort(formats, new DecreasingBandwidthComparator()); // Set the format indices in the same order. this.tracks = new int[length]; for (int i = 0; i < length; i++) { @@ -199,4 +199,17 @@ 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 532b3949f1..ea95e50d96 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,7 +33,6 @@ 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; @@ -329,67 +328,6 @@ 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 f81c866019..0286531fc8 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,31 +28,16 @@ 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(trackFormat, startTimeUs, endTimeUs, C.SELECTION_REASON_UNKNOWN); + this(new DataSpec(Uri.EMPTY), trackFormat, startTimeUs, 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) { + public FakeMediaChunk(DataSpec dataSpec, Format trackFormat, long startTimeUs, long endTimeUs) { super( DATA_SOURCE, - new DataSpec(Uri.EMPTY), + dataSpec, trackFormat, - selectionReason, + C.SELECTION_REASON_ADAPTIVE, /* trackSelectionData= */ null, startTimeUs, endTimeUs,