From 1a171a004a5d3db7936b6e2dff34650baf930d9a Mon Sep 17 00:00:00 2001 From: tonihei Date: Wed, 18 May 2022 13:01:55 +0100 Subject: [PATCH] Add missing equals to MergingMediaPeriod.ForwardingTrackSelection This causes a bug where the forwarded selections are no longer assumed equal and the child MediaPeriods will think they need to reset streams even though the selection stayed the same. Issue: Issue: google/ExoPlayer#10248 PiperOrigin-RevId: 449454038 --- RELEASENOTES.md | 4 ++ .../exoplayer/source/MergingMediaPeriod.java | 20 ++++++ .../source/MergingMediaPeriodTest.java | 61 ++++++++++++++++--- 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 883e3271ad..f8e31e09f1 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -12,6 +12,10 @@ by the user of the device. Apps can opt-out of contributing to platform diagnostics for ExoPlayer with `ExoPlayer.Builder.setUsePlatformDiagnostics(false)`. + * Fix bug that tracks are reset too often when using `MergingMediaSource`, + for example when side-loading subtitles and changing the selected + subtitle mid-playback + ([#10248](https://github.com/google/ExoPlayer/issues/10248)). * Track selection: * Flatten `TrackSelectionOverrides` class into `TrackSelectionParameters`, and promote `TrackSelectionOverride` to a top level class. diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/MergingMediaPeriod.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/MergingMediaPeriod.java index 925998ea1c..c71b5cffd0 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/MergingMediaPeriod.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/MergingMediaPeriod.java @@ -608,5 +608,25 @@ public boolean blacklist(int index, long exclusionDurationMs) { public boolean isBlacklisted(int index, long nowMs) { return trackSelection.isBlacklisted(index, nowMs); } + + @Override + public boolean equals(@Nullable Object o) { + if (this == o) { + return true; + } + if (!(o instanceof ForwardingTrackSelection)) { + return false; + } + ForwardingTrackSelection that = (ForwardingTrackSelection) o; + return trackSelection.equals(that.trackSelection) && trackGroup.equals(that.trackGroup); + } + + @Override + public int hashCode() { + int result = 17; + result = 31 * result + trackGroup.hashCode(); + result = 31 * result + trackSelection.hashCode(); + return result; + } } } diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/MergingMediaPeriodTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/MergingMediaPeriodTest.java index 02b35f9295..740e468c23 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/MergingMediaPeriodTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/MergingMediaPeriodTest.java @@ -35,6 +35,7 @@ import androidx.media3.test.utils.FakeMediaPeriod; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.common.collect.ImmutableList; +import java.util.Arrays; import java.util.concurrent.CountDownLatch; import org.checkerframework.checker.nullness.compatqual.NullableType; import org.junit.Test; @@ -139,21 +140,64 @@ public void selectTracks_createsSampleStreamsFromChildPeriods() throws Exception streams[0].readData(formatHolder, inputBuffer, FLAG_REQUIRE_FORMAT); streams[1].readData(formatHolder, inputBuffer, FLAG_REQUIRE_FORMAT); - FakeMediaPeriodWithSelectTracksPosition childMediaPeriod1 = - (FakeMediaPeriodWithSelectTracksPosition) mergingMediaPeriod.getChildPeriod(0); + FakeMediaPeriodWithSelectionParameters childMediaPeriod1 = + (FakeMediaPeriodWithSelectionParameters) mergingMediaPeriod.getChildPeriod(0); assertThat(childMediaPeriod1.selectTracksPositionUs).isEqualTo(0); assertThat(streams[0].readData(formatHolder, inputBuffer, /* readFlags= */ 0)) .isEqualTo(C.RESULT_BUFFER_READ); assertThat(inputBuffer.timeUs).isEqualTo(123_000L); - FakeMediaPeriodWithSelectTracksPosition childMediaPeriod2 = - (FakeMediaPeriodWithSelectTracksPosition) mergingMediaPeriod.getChildPeriod(1); + FakeMediaPeriodWithSelectionParameters childMediaPeriod2 = + (FakeMediaPeriodWithSelectionParameters) mergingMediaPeriod.getChildPeriod(1); assertThat(childMediaPeriod2.selectTracksPositionUs).isEqualTo(3000L); assertThat(streams[1].readData(formatHolder, inputBuffer, /* readFlags= */ 0)) .isEqualTo(C.RESULT_BUFFER_READ); assertThat(inputBuffer.timeUs).isEqualTo(456_000 - 3000); } + @Test + public void selectTracks_withSameArguments_forwardsEqualSelectionsToChildSources() + throws Exception { + MergingMediaPeriod mergingMediaPeriod = + prepareMergingPeriod( + new MergingPeriodDefinition( + /* timeOffsetUs= */ 0, /* singleSampleTimeUs= */ 0, childFormat11), + new MergingPeriodDefinition( + /* timeOffsetUs= */ 0, /* singleSampleTimeUs= */ 0, childFormat22)); + FakeMediaPeriodWithSelectionParameters childMediaPeriod1 = + (FakeMediaPeriodWithSelectionParameters) mergingMediaPeriod.getChildPeriod(0); + FakeMediaPeriodWithSelectionParameters childMediaPeriod2 = + (FakeMediaPeriodWithSelectionParameters) mergingMediaPeriod.getChildPeriod(1); + + TrackGroupArray mergedTrackGroups = mergingMediaPeriod.getTrackGroups(); + ExoTrackSelection[] selectionArray = + new ExoTrackSelection[] { + new FixedTrackSelection(mergedTrackGroups.get(0), /* track= */ 0), + new FixedTrackSelection(mergedTrackGroups.get(1), /* track= */ 0) + }; + + mergingMediaPeriod.selectTracks( + selectionArray, + /* mayRetainStreamFlags= */ new boolean[2], + /* streams= */ new SampleStream[2], + /* streamResetFlags= */ new boolean[2], + /* positionUs= */ 0); + ExoTrackSelection firstSelectionChild1 = childMediaPeriod1.selectTracksSelections[0]; + ExoTrackSelection firstSelectionChild2 = childMediaPeriod2.selectTracksSelections[1]; + + mergingMediaPeriod.selectTracks( + selectionArray, + /* mayRetainStreamFlags= */ new boolean[2], + /* streams= */ new SampleStream[2], + /* streamResetFlags= */ new boolean[2], + /* positionUs= */ 0); + ExoTrackSelection secondSelectionChild1 = childMediaPeriod1.selectTracksSelections[0]; + ExoTrackSelection secondSelectionChild2 = childMediaPeriod2.selectTracksSelections[1]; + + assertThat(firstSelectionChild1).isEqualTo(secondSelectionChild1); + assertThat(firstSelectionChild2).isEqualTo(secondSelectionChild2); + } + private MergingMediaPeriod prepareMergingPeriod(MergingPeriodDefinition... definitions) throws Exception { MediaPeriod[] mediaPeriods = new MediaPeriod[definitions.length]; @@ -166,7 +210,7 @@ private MergingMediaPeriod prepareMergingPeriod(MergingPeriodDefinition... defin trackGroups[j] = new TrackGroup(definition.formats[j]); } mediaPeriods[i] = - new FakeMediaPeriodWithSelectTracksPosition( + new FakeMediaPeriodWithSelectionParameters( new TrackGroupArray(trackGroups), new EventDispatcher() .withParameters( @@ -201,11 +245,12 @@ public void onContinueLoadingRequested(MediaPeriod source) { return mergingMediaPeriod; } - private static final class FakeMediaPeriodWithSelectTracksPosition extends FakeMediaPeriod { + private static final class FakeMediaPeriodWithSelectionParameters extends FakeMediaPeriod { + public @NullableType ExoTrackSelection[] selectTracksSelections; public long selectTracksPositionUs; - public FakeMediaPeriodWithSelectTracksPosition( + public FakeMediaPeriodWithSelectionParameters( TrackGroupArray trackGroupArray, EventDispatcher mediaSourceEventDispatcher, TrackDataFactory trackDataFactory) { @@ -217,6 +262,7 @@ public FakeMediaPeriodWithSelectTracksPosition( DrmSessionManager.DRM_UNSUPPORTED, new DrmSessionEventListener.EventDispatcher(), /* deferOnPrepared= */ false); + selectTracksSelections = new ExoTrackSelection[trackGroupArray.length]; selectTracksPositionUs = C.TIME_UNSET; } @@ -227,6 +273,7 @@ public long selectTracks( @NullableType SampleStream[] streams, boolean[] streamResetFlags, long positionUs) { + selectTracksSelections = Arrays.copyOf(selections, selections.length); selectTracksPositionUs = positionUs; return super.selectTracks( selections, mayRetainStreamFlags, streams, streamResetFlags, positionUs);