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

Fix repeated segments #1489

Merged
merged 22 commits into from
Feb 28, 2024
Merged

Fix repeated segments #1489

merged 22 commits into from
Feb 28, 2024

Conversation

dzianis-dashkevich
Copy link
Contributor

@dzianis-dashkevich dzianis-dashkevich commented Feb 22, 2024

The Problem

The player repeats segments sometimes.

To understand better the root cause of the problem, we should understand the following concepts:

A transport stream needs timing information to ensure Video and Audio are shown at the right time and in sync.
The most important timecodes are:

The basic unit of a timecode is the Tick. There are 90,000 Ticks in a second (90kHz).

  • Presentation Time Stamp (PTS)
    The PTS tells the decoder when to display or present a frame in the stream.
    The PTS is present for elementary streams such as Video, Audio, & Subtitles.
  • Decode Time Stamp (DTS)
    The DTS tells the decoder when to decode a frame in the stream.
    The DTS is present for elementary streams such as Video, Audio, & Subtitles.
  • Programme Clock Reference (PCR)
    The Programme Clock Reference (PCR) is a reference clock the decoder can use to keep track of all timing information and keep streams in sync.
    If the PTS or DTS timestamps are corrupted, the decoder can use the PCR to recreate the corrupted timestamps.
    The PCR always increments linearly, so it can also be used to seek video.
    For greater accuracy, the PCR has an extension which increases the resolution to 27MHz (instead of 90KHz for PTS and DTS)
    There can be a PCR present for each Programme (PMT) or one PCR for the entire Transport stream. In this case, the PCR is usually carried on a separate PID, commonly 0x1FFE or 0x1FE.

mux.js reads, parses PES packets, and extracts DTS and PTS values. Both values are 33-bit.
These timestamps monotonically increase until they hit a 33-bit boundary, which is 8589934592 (2^33). Once they hit 2^33, they roll over to 0, and this process repeats. This "feature" is called timestamp rollover.

Timestamp rollover happens every ~26.5 hours:

  • Adjust frequency to seconds: 8589934592 / 90000 = ~95443 sec
  • Convert to hours: 95443 / (60 * 60) = ~26.5 h

mux.js stores the first-encountered DTS value within one timeline and uses it as a reference value to handle rollovers for upcoming timestamps.
It adjusts timestamp values based on the direction (backward/forward).
Forward adjustment: PTS/DTS value + 2^33.
Backward adjustment: PTS/DTS value - 2^33.

Now that we know about PTS, DTS, PCR, and timestampRollover, we need to understand two more concepts: discontinuities(or timelines) and timestamp offsets.

Discontinuities signal that we should reset the decoder since we are switching to different content (e.g., it was encoded Differently, it has different timestamps, etc...). The following examples are common discontinuities cases:

  • We switch from the main content to the commercial.
  • We switch from one commercial to another commercial.
  • We switch from a commercial to the main content.

vhs uses the internal term "timeline" since when we hit discontinuity, we switch to a different timeline because PTS/DTS timestamps are completely different from the content before discontinuity.

Now we should understand timestampOffset concept:

The player has a progress bar with a predictable timeline for a user.
Segment's PTS and DTS values do not align with the player's time, and they may "jump" during discontinuities. That is why we use timestampOffset to map the segment's timestamps to the time in our player.

So, Segments PTS/DTS + timestampOffset = player's time for a segment.

Ok, now let's put everything together:

  • The player loads a segment and passes it to mux.js
  • mux.js parses it
  • mux.js (possibly) adjusts forward/backward rollover.
  • mux.js passes back to the player data with timing info.
  • If discontinuity - the player updates timestampOffset.
  • The player appends data to a source buffer.

Now, the same stream may contain different renditions. Some renditions may have timestamp rollover, while others may not. This means that even if we switch from one rendition to another within the same timeline (no discontinuity), we may still have completely different timestamps. Since we already use timestampOffset adjustment, the easiest and most obvious fix would be re-calculating timestampOffset after each rendition switch.

VHS uses SyncController in order to understand which segment to load after the rendition switch.
Sometimes, we don't have enough information to make a proper decision because of the many factors, such as:

  • Misalignment between segment duration info from a playlist and the actual segment duration
  • Gaps between playlists update
  • Some other issues (cpu/network, etc) that may lead us to drop out-of-sync

Sometimes, the player may select segments already appended to the source buffer.
Previously - such appends did not affect the source buffer. Since timestampOffset stays the same, and PTS/DTS values are the same - a browser simply "replaces" already appended data with new data.

Unfortunatelly, re-calculating timestamp offset on every rendition switch makes us less resilient to such appends since now the browser places such segments on top of the buffer (instead of the same place as it was).

This PR is another try to improve the syncing mechanism to avoid selecting already appended data between rendition switches.

Specific Changes proposed

  • Enhance MediaSequence Sync Strategy
  • Add markAppended for each segment for the mediaSequence sync strategy
  • Use MediaSequence segment-selection algorithm by default if a playlist is reliable (has a stable media sequence)
  • Disable re-calculation for timestampOffset for MPEG-DASH since we should not have issues with timestampRollover for MPEG-DASH

Possible Alternative to avoid using timestampOffsets

  • since we have control over mux.js we can simply override "real" PTS/DTS values to align it with the player's timeline. In this case, both the segment's time and the player's time will be aligned by default, and the offset will always be 0. (Hls.js uses this approach).

Testing Matrix

Type Fast Quality Switch Bandwidth Update Seeking
HLS VOD CMAF OK OK OK
HLS VOD TS OK OK OK
HLS LIVE CMAF OK OK OK
HLS LIVE TS OK OK OK
DASH VOD OK OK OK
DASH LIVE OK OK OK
HLS LL OK OK OK
HLS ByteRange OK OK OK

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
  • Reviewed by Two Core Contributors

@dzianis-dashkevich dzianis-dashkevich marked this pull request as ready for review February 27, 2024 20:40
Copy link

codecov bot commented Feb 27, 2024

Codecov Report

Attention: Patch coverage is 96.72131% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 86.26%. Comparing base (75f7b1a) to head (bad97ec).
Report is 1 commits behind head on main.

Files Patch % Lines
src/util/media-sequence-sync.js 96.29% 4 Missing ⚠️
src/segment-loader.js 95.91% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1489      +/-   ##
==========================================
+ Coverage   86.06%   86.26%   +0.19%     
==========================================
  Files          42       43       +1     
  Lines       10752    10877     +125     
  Branches     2474     2501      +27     
==========================================
+ Hits         9254     9383     +129     
+ Misses       1498     1494       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@adrums86 adrums86 left a comment

Choose a reason for hiding this comment

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

Great refactor. The use of the MediaSequenceSync made this very legible. May there never be another timing issue in VHS! 🍻 😁

src/source-updater.js Show resolved Hide resolved
src/util/media-sequence-sync.js Outdated Show resolved Hide resolved
src/util/media-sequence-sync.js Outdated Show resolved Hide resolved
src/util/media-sequence-sync.js Outdated Show resolved Hide resolved
test/playlist-controller.test.js Show resolved Hide resolved
Copy link
Contributor

@wseymour15 wseymour15 left a comment

Choose a reason for hiding this comment

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

Great work! +1 after spelling fix

src/util/media-sequence-sync.js Outdated Show resolved Hide resolved
@dzianis-dashkevich dzianis-dashkevich merged commit ed8f6bd into main Feb 28, 2024
15 checks passed
@dzianis-dashkevich dzianis-dashkevich deleted the fix-repeated-segments branch February 28, 2024 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants