Seek nearest SYNC does not adjust stale playlists #10484
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stevemayhew
commented
Jul 28, 2022
| // is non-empty, and the playlist is fresh enough that the period position falls in side of | ||
| // the playlist bound, so we can use segment start times as sync points. | ||
| // | ||
| // If (a) an adaptive quality switch occurs between the adjustment and the seek being | ||
| // performed, and (b) segment start times are not aligned across variants, it's possible that |
There was a problem hiding this comment.
Add comment about position exceeds duration check, note track switch on seek is not really a rare condition. And the stale playlist issue happens for live if there are lots of ABR changes.
stevemayhew
force-pushed
the
p-stale-playlist-seek-adjust
branch
from
201ece9
to
ef93727
Compare
stevemayhew
commented
Jul 28, 2022
| long startOfPlaylistInPeriodUs = | ||
| mediaPlaylist.startTimeUs - playlistTracker.getInitialStartTimeUs(); | ||
| long relativePositionUs = positionUs - startOfPlaylistInPeriodUs; | ||
| int segmentIndex = |
There was a problem hiding this comment.
Moved this up to allow early return style.
stevemayhew
commented
Jul 28, 2022
library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java
Show resolved
Hide resolved
stevemayhew
force-pushed
the
p-stale-playlist-seek-adjust
branch
from
ef93727
to
71a4fb1
Compare
`getPlaylistSnapshot()` can return a cached but potentially quite stale playlist
that may be unsuitable for resolving a seek position.
This change fixes a race conditon that can cause a failed attempt to resolve
a seek position with a stale playlist.
Pre-conditions for the race:
* A is the cached snapshot from previous playback of playlist A
* B is the selected playlist at time of the seek
* \* — is current playback position in period
* QQQ — positions in A cannot be seek targets from B, they have rolled out from `Timeline.Window`
* WWW — are positions in A that cannot be resolved in A, but would be in A`
```
+=================*==+
| Period |
+=================*==+
+QQQ-------+
| A |
+QQQ-------+
+-------WWW+
| B |
+-------WWW+
seek and switch back to A occur here...
+----------+
| A' |
+----------+
```
The seek request is issued with `Timeline.Window` from playlist B, yet the call to `getAdjustedSeekPositionUs()`
occurs:
1. After the current track selection swithces back to A
2. But before playlist snapshot A` is loaded, replacing the old A
The check is simple, does basic sanity check on `targetPositionInPlaylistUs` (that it
is < durationUs of the selected playlist). This covers the positions WWW.
stevemayhew
force-pushed
the
p-stale-playlist-seek-adjust
branch
from
71a4fb1
to
f6488d3
Compare
stevemayhew
commented
Jul 28, 2022
library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsChunkSource.java
Show resolved
Hide resolved
library/hls/src/test/java/com/google/android/exoplayer2/source/hls/HlsChunkSourceTest.java
Show resolved
Hide resolved
stevemayhew
referenced
this pull request
Jul 28, 2022
The comment "all should be same" is not correct, it is extremely likely they will be the same but not assured. In either case the seek position will work, it just may not land exactly on a sync point in every variant.
|
Closing all PRs on this deprecated project. We are now unable to merge PRs from here. If you would like us to consider this change again please file a new PR on the media3 project: https:/androidx/media/pulls |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
getPlaylistSnapshot()can return a cached but potentially quite stale playlistthat may be unsuitable for resolving a seek position.
This change fixes a race conditon that can cause a failed attempt to resolve
a seek position with a stale playlist.
Pre-conditions for the race:
Timeline.WindowThe seek request is issued with
Timeline.Windowfrom playlist B, yet the call togetAdjustedSeekPositionUs()occurs:
The check is simple, does basic sanity check on
targetPositionInPlaylistUs(that itis < durationUs of the selected playlist). This covers the positions WWW.