Skip to content

Commit

Permalink
Seek nearest SYNC does not adjust stale playlists
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
stevemayhew committed Jul 28, 2022
1 parent 6288182 commit ef93727
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -270,28 +270,35 @@ public long getAdjustedSeekPositionUs(long positionUs, SeekParameters seekParame
return positionUs;
}

long startOfPlaylistInPeriodUs =
mediaPlaylist.startTimeUs - playlistTracker.getInitialStartTimeUs();
long targetPositionInPlaylistUs = positionUs - startOfPlaylistInPeriodUs;

if (targetPositionInPlaylistUs > mediaPlaylist.durationUs) {
return positionUs;
}

// Segments start with sync samples (i.e., EXT-X-INDEPENDENT-SEGMENTS is set) and the playlist
// is non-empty, so we can use segment start times as sync points. Note that in the rare case
// that (a) an adaptive quality switch occurs between the adjustment and the seek being
// 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
// the adjusted position may not be at a sync point when it was intended to be. However, this is
// very much an edge case, and getting it wrong is worth it for getting the vast majority of
// cases right whilst keeping the implementation relatively simple.
long startOfPlaylistInPeriodUs =
mediaPlaylist.startTimeUs - playlistTracker.getInitialStartTimeUs();
long relativePositionUs = positionUs - startOfPlaylistInPeriodUs;
int segmentIndex =
int segIndex =
Util.binarySearchFloor(
mediaPlaylist.segments,
relativePositionUs,
targetPositionInPlaylistUs,
/* inclusive= */ true,
/* stayInBounds= */ true);
long firstSyncUs = mediaPlaylist.segments.get(segmentIndex).relativeStartTimeUs;
long firstSyncUs = mediaPlaylist.segments.get(segIndex).relativeStartTimeUs;
long secondSyncUs = firstSyncUs;
if (segmentIndex != mediaPlaylist.segments.size() - 1) {
secondSyncUs = mediaPlaylist.segments.get(segmentIndex + 1).relativeStartTimeUs;
if (segIndex != mediaPlaylist.segments.size() - 1) {
secondSyncUs = mediaPlaylist.segments.get(segIndex + 1).relativeStartTimeUs;
}
return seekParameters.resolveSeekPositionUs(relativePositionUs, firstSyncUs, secondSyncUs)
return seekParameters.resolveSeekPositionUs(targetPositionInPlaylistUs, firstSyncUs, secondSyncUs)
+ startOfPlaylistInPeriodUs;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@
public class HlsChunkSourceTest {

private static final String PLAYLIST = "media/m3u8/media_playlist";
private static final String PLAYLIST_INDEPENDENT_SEGMENTS =
"media/m3u8/media_playlist_independent_segments";
private static final String PLAYLIST_LIVE_FIRST =
"media/m3u8/media_playlist_live_first";
private static final String PLAYLIST_LIVE_SECOND =
"media/m3u8/media_playlist_live_second";
private static final String PLAYLIST_EMPTY = "media/m3u8/media_playlist_empty";
private static final Uri PLAYLIST_URI = Uri.parse("http://example.com/");
private static final long PLAYLIST_START_PERIOD_OFFSET_US = 8_000_000L;
Expand All @@ -71,7 +73,7 @@ public void setup() throws IOException {

InputStream inputStream =
TestUtil.getInputStream(
ApplicationProvider.getApplicationContext(), PLAYLIST_INDEPENDENT_SEGMENTS);
ApplicationProvider.getApplicationContext(), PLAYLIST_LIVE_FIRST);
HlsMediaPlaylist playlist =
(HlsMediaPlaylist) new HlsPlaylistParser().parse(PLAYLIST_URI, inputStream);
when(mockPlaylistTracker.getPlaylistSnapshot(eq(PLAYLIST_URI), anyBoolean()))
Expand Down Expand Up @@ -161,7 +163,22 @@ public void getAdjustedSeekPositionUs_noIndependentSegments() throws IOException

long adjustedPositionUs =
testChunkSource.getAdjustedSeekPositionUs(
playlistTimeToPeriodTimeUs(100_000_000), SeekParameters.EXACT);
playlistTimeToPeriodTimeUs(100_000_000), SeekParameters.NEXT_SYNC);

assertThat(periodTimeToPlaylistTimeUs(adjustedPositionUs)).isEqualTo(100_000_000);
}

@Test
public void getAdjustedSeekPositionUs_stalePlaylist() throws IOException {
InputStream inputStream =
TestUtil.getInputStream(ApplicationProvider.getApplicationContext(), PLAYLIST_LIVE_SECOND);
HlsMediaPlaylist playlist =
(HlsMediaPlaylist) new HlsPlaylistParser().parse(PLAYLIST_URI, inputStream);
when(mockPlaylistTracker.getPlaylistSnapshot(eq(PLAYLIST_URI), anyBoolean()))
.thenReturn(playlist);
long adjustedPositionUs =
testChunkSource.getAdjustedSeekPositionUs(
playlistTimeToPeriodTimeUs(100_000_000), SeekParameters.NEXT_SYNC);

assertThat(periodTimeToPlaylistTimeUs(adjustedPositionUs)).isEqualTo(100_000_000);
}
Expand All @@ -177,7 +194,7 @@ public void getAdjustedSeekPositionUs_emptyPlaylist() throws IOException {

long adjustedPositionUs =
testChunkSource.getAdjustedSeekPositionUs(
playlistTimeToPeriodTimeUs(100_000_000), SeekParameters.EXACT);
playlistTimeToPeriodTimeUs(100_000_000), SeekParameters.NEXT_SYNC);

assertThat(periodTimeToPlaylistTimeUs(adjustedPositionUs)).isEqualTo(100_000_000);
}
Expand Down
17 changes: 17 additions & 0 deletions testdata/src/test/assets/media/m3u8/media_playlist_live_first
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#EXTM3U
#EXT-X-MEDIA-SEQUENCE:2
#EXT-X-INDEPENDENT-SEGMENTS
#EXT-X-PROGRAM-DATE-TIME:2019-02-14T02:00:00.106Z
#EXT-X-MAP:URI="init.mp4"
#EXTINF:4,
2.mp4
#EXTINF:4,
3.mp4
#EXTINF:4,
4.mp4
#EXTINF:4,
5.mp4
#EXTINF:4,
6.mp4
#EXTINF:4,
7.mp4
15 changes: 15 additions & 0 deletions testdata/src/test/assets/media/m3u8/media_playlist_live_second
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#EXTM3U
#EXT-X-MEDIA-SEQUENCE:4
#EXT-X-INDEPENDENT-SEGMENTS
#EXT-X-PROGRAM-DATE-TIME:2019-02-14T02:00:08.106Z
#EXT-X-MAP:URI="init.mp4"
#EXTINF:4,
4.mp4
#EXTINF:4,
5.mp4
#EXTINF:4,
6.mp4
#EXTINF:4,
7.mp4
#EXTINF:4,
8.mp4

0 comments on commit ef93727

Please sign in to comment.