fix: support timeshift dash manifests without startNumber attribute

[ziddey]

Description

This should bring support for live DASH assets that utilize timeShift (sliding window) and $Time$ as opposed to $Number$ (no startNumber attribute). Would fix https://github.com/videojs/http-streaming/issues/256

For such manifests, mpd-parser will always set mediaSequence to 1. Consequently, SegmentLoader will be unaware of any live window shifts.

Specific Changes proposed

Instead, we need to derive an appropriate mediaSequence on each playlist update so the window can shift accordingly. We can take the first segment of the new playlist and find its index in the old playlist (we can search by the segment's uri). This is the number of shifts, or change in mediaSequence.

In the event the segment isn't found (e.g. network issues preventing manifest update), we can instead advance the entire length of existing segments, which should then simulate falling behind the live window and trigger a reseek.

This derivation only needs to occur for this type of DASH asset, but it should not cause issues otherwise since it should end up deriving the same value. It should generally be sufficient to check that the new playlist has a mediaSequence of 1, but maybe there's a more definitive way? We could then also check that the new sequence <= the old one (to exclude a playlist naturally going from 0 to 1).

Requirements Checklist

• [X] Feature implemented / Bug fixed
• [ ] If necessary, more likely in a feature request than a bug fix
  • [ ] Unit Tests updated or fixed
  • [ ] Docs/guides updated
  • [ ] Example created (starter template on JSBin)
• [ ] Reviewed by Two Core Contributors

[welcome[bot]]

💖 Thanks for opening this pull request! 💖

Things that will help get your PR across the finish line:

• Run npm run lint -- --errors locally to catch formatting errors earlier.
• Include tests when adding/changing behavior.
• Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

[ziddey]

Can segments ever evaluate false? If it's always an array, even if empty, we could just check b.mediaSequence === 1 && b.segments.length.

In general, it may be worth considering scenarios where a playlist goes from not having segments to having segments and vice versa and how to handle setting the mediaSequence

Logically, the dashPlaylistUnchanged function may not the most ideal place for this fix, but it was easiest to implement here. Maybe someone more familiar with the codebase could move it somewhere more appropriate?

I've setup a temporary demo which now plays the Unified Streaming Live DASH stream here

Note this fix may not completely solve issues with playing these types of assets. While I've now had the above stream playing continuously for almost a day without issue, I am seeing similar streams stop randomly. It seems to stop fetching the manifest entirely, and I'm not seeing anything that stands out in the debug log. When this happens, vhs.playlists.minimumUpdatePeriodTimeout_ becomes null, while vhs.playlists.master.minimumUpdatePeriod still has the proper value. It can happen almost right away or sometimes up to an hour into the (same) stream.

[gkatsev]

PRs are also built automatically via netlify, so, your changes are lives here https://deploy-preview-1092--videojs-http-streaming.netlify.app/ (see the netlify checks)

[ziddey]

I believe I've identified the next issue. For an HLS stream, when changing quality, the media manifest for the old quality is no longer refreshed. Unfortunately, this is also being applied for DASH streams.

Note this isn't an issue with the Unified Streaming Live DASH stream since it only has one quality.

[ziddey]

Added a sourceType check to onGroupChanged to not stop the playlist loader for dash sources.

What ramifications does this have on switching from demuxed to muxed audio? As it was, I'm not sure those conditions would have evaluated true.

[codecov[bot]]

Codecov Report

Merging #1092 (b848006) into main (0964cb4) will decrease coverage by 0.00%. The diff coverage is 100.00%.

:exclamation: Current head b848006 differs from pull request most recent head e5beaea. Consider uploading reports for the commit e5beaea to get more accurate results

@@            Coverage Diff             @@
##             main    #1092      +/-   ##
==========================================
- Coverage   86.31%   86.30%   -0.01%     
==========================================
  Files          39       38       -1     
  Lines        9807     9079     -728     
  Branches     2279     2061     -218     
==========================================
- Hits         8465     7836     -629     
+ Misses       1342     1243      -99     

Impacted Files	Coverage Δ
src/dash-playlist-loader.js	89.66% <100.00%> (-0.43%)	:arrow_down:
src/media-groups.js	98.91% <100.00%> (+0.24%)	:arrow_up:
src/playlist-loader.js	91.40% <0.00%> (-3.64%)	:arrow_down:
src/playlist-selectors.js	85.23% <0.00%> (-1.83%)	:arrow_down:
src/util/text-tracks.js	88.29% <0.00%> (-1.33%)	:arrow_down:
src/segment-loader.js	95.32% <0.00%> (-1.02%)	:arrow_down:
src/vtt-segment-loader.js	80.00% <0.00%> (-0.90%)	:arrow_down:
src/sync-controller.js	96.72% <0.00%> (-0.73%)	:arrow_down:
src/videojs-http-streaming.js	90.56% <0.00%> (-0.47%)	:arrow_down:
... and 13 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0964cb4...e5beaea. Read the comment docs.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.