Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TrackSelection does not jump to live with HlsMediaPeriod #9386

Closed
wants to merge 2 commits into from
Closed

TrackSelection does not jump to live with HlsMediaPeriod #9386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f8779b
TrackSelection does not jump to live with HlsMediaPeriod
stevemayhew Sep 2, 2021
9f9621d
Tests trackselection does not seek to default period start
stevemayhew Sep 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaPeriod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,6 @@ public long selectTracks(
@NullableType SampleStream[] streams,
boolean[] streamResetFlags,
long positionUs) {
if (preparePositionOverrideUs != C.TIME_UNSET && positionUs == preparePositionUs) {
positionUs = preparePositionOverrideUs;
preparePositionOverrideUs = C.TIME_UNSET;
}
return castNonNull(mediaPeriod)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The position change to start at the default position has already happened when the onPrepare() callback is processed by ExoPlayerImplInternal. Removing this causes no existing test cases to fail.

.selectTracks(selections, mayRetainStreamFlags, streams, streamResetFlags, positionUs);
}
Expand Down
5 changes: 0 additions & 5 deletions library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,6 @@ protected void onChildSourceInfoRefreshed(
@Nullable MediaPeriodId idForMaskingPeriodPreparation = null;
if (isPrepared) {
timeline = timeline.cloneWithUpdatedTimeline(newTimeline);
if (unpreparedMaskingMediaPeriod != null) {
// Reset override in case the duration changed and we need to update our override.
setPreparePositionOverrideToUnpreparedMaskingPeriod(
unpreparedMaskingMediaPeriod.getPreparePositionOverrideUs());
}
Comment on lines -143 to -147
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems remotely possible, perhaps if the timeline is growing when it is a newly started live playback where it has not reached its target window size. @marcbaechinger what do you think? My new test case still passes if we remove this change, however I'd suggest if we leave it that it is covered by a unit test case (right now it is not).

} else if (newTimeline.isEmpty()) {
timeline =
hasRealTimeline
Expand Down
71 changes: 71 additions & 0 deletions library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3249,6 +3249,77 @@ public void run(SimpleExoPlayer player) {
assertThat(positionWhenReady.get()).isEqualTo(10);
}

@Test
public void trackSelectionDoesNotResetToDefaultPeriodPosition()
throws Exception {
FakeMediaSource mediaSource =
new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.VIDEO_FORMAT, ExoPlayerTestRunner.AUDIO_FORMAT);
FakeTrackSelector trackSelector = new FakeTrackSelector();
FakeRenderer videoRenderer = new FakeRenderer(C.TRACK_TYPE_VIDEO);
FakeRenderer audioRenderer = new FakeRenderer(C.TRACK_TYPE_AUDIO);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Start with a MediaSource with both audio and video

MediaItem mediaItem =
new MediaItem.Builder().setUri(Uri.EMPTY).build();
final long windowDefaultStartPositionUs = 5 * C.MICROS_PER_SECOND;
final AtomicLong expectedPosition = new AtomicLong(C.TIME_UNSET);
final int lastSeekPosition = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Window start override is 5 seconds, the seekTo 0 is what does not get honored.


Timeline liveTimeline =
new SinglePeriodTimeline(
/* presentationStartTimeMs= */ C.TIME_UNSET,
/* windowStartTimeMs= */ C.TIME_UNSET,
/* elapsedRealtimeEpochOffsetMs= */ C.TIME_UNSET,
/* periodDurationUs= */ 10 * C.MICROS_PER_SECOND,
/* windowDurationUs= */ 10 * C.MICROS_PER_SECOND,
/* windowPositionInPeriodUs= */ 0,
/* windowDefaultStartPositionUs= */ windowDefaultStartPositionUs,
/* isSeekable= */ true,
/* isDynamic= */ true,
/* manifest= */ null,
mediaItem,
mediaItem.liveConfiguration);

ActionSchedule actionSchedule =
new ActionSchedule.Builder(TAG)
.pause()
.waitForPlaybackState(Player.STATE_BUFFERING)
// Finish preparation.
.executeRunnable(() -> mediaSource.setNewSourceInfo(liveTimeline))
.waitForTimelineChanged()
.waitForPlaybackState(Player.STATE_READY)
.play()
.playUntilPosition(0, windowDefaultStartPositionUs)
.pause()
.seekAndWait(lastSeekPosition) // paused at this position, should not change from here
.executeRunnable(() -> {
DefaultTrackSelector.Parameters parameters = trackSelector.getParameters();
trackSelector.setParameters(parameters.buildUpon()
.setMaxAudioBitrate(0) // Cause track selection change
.build()
);
})
Comment on lines +3290 to +3299
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Playback starts, reaches the default window start and is paused there. Issue a track selection that changes parameters so as to invalidate the current selection.

.waitForPendingPlayerCommands()
.executeRunnable(new PlayerRunnable() {
@Override
public void run(SimpleExoPlayer player) {
expectedPosition.set(player.getCurrentPosition());
}
})
Comment on lines +3304 to +3306
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, after track selection position should still be 0, but the removed code would have reset it to the default position (causing this test to fail)

.stop() // End live playback
.build();

new ExoPlayerTestRunner.Builder(context)
.setMediaSources(mediaSource)
.setTrackSelector(trackSelector)
.setRenderers(audioRenderer, videoRenderer)
.setActionSchedule(actionSchedule)
.setUseLazyPreparation(true) // defer prepare until first source update (this is default)
.build()
.start()
.blockUntilEnded(TIMEOUT_MS);

assertThat(expectedPosition.get()).isEqualTo(lastSeekPosition);
}

@Test
public void seekToUnpreparedWindowWithMultiplePeriodsInConcatenationStartsAtCorrectPeriod()
throws Exception {
Expand Down