shaka-player icon indicating copy to clipboard operation
shaka-player copied to clipboard

feat(DASH): Add MPD Patch support

Open dave-nicholas opened this issue 1 year ago • 12 comments

Closes https://github.com/shaka-project/shaka-player/issues/2228

dave-nicholas avatar May 25 '23 14:05 dave-nicholas

Incremental code coverage: 87.44%

github-actions[bot] avatar May 25 '23 15:05 github-actions[bot]

Thank you for contributing!

I don't understand the patch adapter, though. I worry that it will not be representative of actual patch streams. I also see it being applied in the demo to a VOD manifest, which should have no need of patching. Patching is only useful for updates to live streams, correct?

joeyparrish avatar Jun 02 '23 15:06 joeyparrish

Thank you for contributing!

I don't understand the patch adapter, though. I worry that it will not be representative of actual patch streams. I also see it being applied in the demo to a VOD manifest, which should have no need of patching. Patching is only useful for updates to live streams, correct?

Hi @joeyparrish

I agree with you that patch manifest adapter is not ideal, but I wanted to give the ability to test the feature in the demo app as open patch compliant streams are not easy to come by (at least I haven't seen any 😆 ). The reason I chose the ad enabled VOD is because of its behaviour of presenting new segments and periods regularly like a live stream.

I can remove remove the manifest adapter if you wish.

dave-nicholas avatar Jun 06 '23 08:06 dave-nicholas

Hi @joeyparrish I have removed the patch adapter. Please let me know if there any concerns or changes in approach that you would like me to make. We will be taking this change into production shortly.

dave-nicholas avatar Jun 20 '23 17:06 dave-nicholas

Happy to support in this kink of new feature. Let me know if you need help

Iragne avatar Sep 29 '23 12:09 Iragne

@dave-nicholas @tykus160 Please add a Demo streams. You can use https://github.com/Dash-Industry-Forum/dash.js/issues/4452 . With a reference stream in the Demo I will be able to review the change and test it for myself. Thanks!

avelad avatar Apr 15 '24 15:04 avelad

When running towards dash.js, some issues were fond in the ongoing development of MPD patch in livesim2. I think things should be good now, but if you find any issues, please report them to https://github.com/Dash-Industry-Forum/livesim2/pull/174 or open a new issue if the PR is closed.

tobbee avatar Apr 16 '24 11:04 tobbee

The MPD Patch support is now on the public DASH-IF livesim2 server. Here is a possible test URL:

https://livesim2.dashif.org/livesim2/patch_60/segtimeline_1/testpic_2s/Manifest.mpd

tobbee avatar Apr 23 '24 12:04 tobbee

The MPD Patch support is now on the public DASH-IF livesim2 server. Here is a possible test URL:

https://livesim2.dashif.org/livesim2/patch_60/segtimeline_1/testpic_2s/Manifest.mpd

@tobbee Your URL uses SegmentTimeline, is there any URL that uses SegmentList or SegmentBase?

avelad avatar Apr 24 '24 09:04 avelad

Added demo asset, thanks @tobbee. This PR still needs some love though, as initial scope of work is not sufficient to properly play DASH-IF asset. Me and @dave-nicholas will continue work on that.

tykus160 avatar Apr 24 '24 09:04 tykus160

@avelad The output is "live" streams and available in three different formats:

  • SegmentTemplate with $Number$: https://livesim2.dashif.org/livesim2/patch_60/testpic_2s/Manifest.mpd
  • SegmentTimeline with $Number$: https://livesim2.dashif.org/livesim2/patch_60/segtimelinenr_1/testpic_2s/Manifest.mpd
  • SegmentTimeline with $Time$: https://livesim2.dashif.org/livesim2/patch_60/segtimeline_1/testpic_2s/Manifest.mpd

SegmentList is not a format recommended by DASH-IF so it is not supported. SegmentBase is only useful for VoD content in DASH OnDemand profile as far as I know, so it is irrelevant to live output.

I hope you can make these variants work (the first one has very little updates since no Segments are mentioned).

Once they work, one can also test with multiperiod variants by adding /periods_60 before testpic_2s in any of the above URLs like

https://livesim2.dashif.org/livesim2/patch_60/segtimeline_1/periods_60/testpic_2s/Manifest.mpd

tobbee avatar Apr 24 '24 12:04 tobbee

Hi! This PR now supports DASH IF test streams, though there are few gotchas @tobbee

  • SegmentTemplate with $Number$ has publishTime set to 1970, so we do regular updates there instead of patches
  • on multiperiod, whenever new period arrives, following patch requests will return HTTP 500, something similar has been reported on dash.js board as well https://github.com/Dash-Industry-Forum/dash.js/issues/4487

I'm gonna focus on adjusting unit tests now.

tykus160 avatar May 23 '24 10:05 tykus160

@shaka-bot test

avelad avatar May 27 '24 11:05 avelad

@avelad: Lab tests started with arguments:

  • pr=5247

shaka-bot avatar May 27 '24 11:05 shaka-bot

@tykus160 The patch issues in livesim2 should be fixed now including the multiperiod case for SegmentTimeline.

Regarding the publishTime for SegmentTemplate with $Number$ it changes every time the MPD changes, which is every minute for one-minute period, but never if there are no periods. The reason behind that is that it the publishTime is an "id" of the manifest so it shouldn't change every segment if there is no change.

In general, I think that MPD Patch and SegmentTemplate with Number is a bad match, so one should rather focus on SegmentTimeline, which is also the case with long MPDs. I think that your choice of regular updates is the most reasonable in that case.

As far as I understood from Daniel Silhavy, dash.js fallbacks to normal MPD requests if there is no update of MPD patch.

tobbee avatar May 28 '24 12:05 tobbee