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.

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

SEEK_NEAREST does not adjust based on stale playlists #10477

Closed

Conversation

stevemayhew
Copy link
Contributor

getAdjustedSeekPosition() fails to return a valid value if a track selection
updates the selected track followed by a seek operation, but before the
updated track has issued the first playlist refersh.

This occurs because the playlist for the new selected track has not yet been:

  1. established as the primary playlist (getNextChunk() will do this)
  2. Performed an initial load
  3. Reported the onPrimaryPlaylistRefreshed() up to the main playback thread

The old code used getPlaylistSnapshot() can return a cached but potentially quite stale playlist
that is unsuitable for resolving a seek position.

The fix is to introduce a method, HlsPlaylistTracker.getFreshPrimaryPlaylist() that only returns
a playlist snapshot that is currently primary and recently loaded.

`getAdjustedSeekPosition()` fails to return a valid value if a track selection
updates the selected track followed by a seek operation, but before the
updated track has issued the first playlist refersh.

This occurs because the playlist for the new selected track has not yet been:

1. established as the primary playlist (`getNextChunk()` will do this)
2. Performed an initial load
3. Reported the `onPrimaryPlaylistRefreshed()` up to the main playback thread

The old code used `getPlaylistSnapshot()` can return a cached but potentially quite stale playlist
that is unsuitable for resolving a seek position.

The fix is to introduce a method, `HlsPlaylistTracker.getFreshPrimaryPlaylist()` that only returns
a playlist snapshot that is currently primary and recently loaded.
boolean isFresh = playlistBundle.isSnapshotFresh();
return isPrimary && isFresh ? playlistBundle.getPlaylistSnapshot() : null;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only the current primary HlsMediaPlaylist is up to date with the Period/Timeline used by ExoPlayerImplInternal, and only if it has been loaded (and thus reported the MediaSource.

|| playlistSnapshot.playlistType == HlsMediaPlaylist.PLAYLIST_TYPE_EVENT
|| playlistSnapshot.playlistType == HlsMediaPlaylist.PLAYLIST_TYPE_VOD
|| updateDelta <= C.usToMs(playlistSnapshot.targetDurationUs * 2);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possible the TYPE_EVENT should also be checked for freshness rather then assuming it is.

Note once it has updated once it may be overkill to check delta < target duration (as this is already covered by the stuck playlist check for primary), so just is primary and has updated (lastSnapshotLoadMs != 0 or just playlistSnapshot != null). If so, this method may not be needed.

stevemayhew referenced this pull request Jul 27, 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.
@tonihei
Copy link
Collaborator

tonihei commented Jul 27, 2022

See conversation in 530dd3f for more details. It may be we can solve the problem in a slightly simpler way than this approach.

@tonihei tonihei self-assigned this Jul 27, 2022
@stevemayhew
Copy link
Contributor Author

Canceling this, the simpler fix is in the works. Thanks @tonihei for your help on this!

@stevemayhew stevemayhew deleted the p-fix-staleplaylist-seek-adjust branch July 28, 2022 16:20
@google google locked and limited conversation to collaborators Sep 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants