feat: add support for dash manifests describing sidx boxes #303

ldayananda · 2018-11-30T23:42:11Z

Description

Attempt to fix #162.

Specific Changes proposed

Dash playlist loader requests the sidx after parsing the master manifest once for the sidx byte range information. Then, the master playlist is mutated.

Current Status

Need to investigate ended issue (known issue 1)

Sources:

{ 
  src: "https://dash.akamaized.net/dash264/TestCases/10a/1/iis_forest_short_poem_multi_lang_480p_single_adapt_aaclc_sidx.mpd",
  type: "application/dash+xml"
}

Known Issues

there appears to be a bug where the ended event is not fired sometimes. This may be an existing issues with DASH.
Repro by using https://dash.akamaized.net/dash264/TestCases/10a/1/iis_forest_short_poem_multi_lang_480p_single_adapt_aaclc_sidx.mpd, seeking to > 25s, changing audio selection, seeking near the end.
The skipped playback test DASH sidx with alt audio should end should also reproduce the issue
if sidx ranges change in between Periods in a live stream, we do not clean up sidxMapping.
This will not be handled at this point as there is further multiperiod work that is required to be able to detect a period change. There is a TODO in the code for this, but no Github issues yet

Requirements Checklist

Feature implemented / Bug fixed
Unit Tests updated or fixed
Reviewed by Core Contributor(s)

ldayananda · 2019-01-03T22:35:48Z

Things to do still:

All representations(specifically audio) can also have a SegmentBase and also need their sidx to be requested via DashPlaylistLoader. The current idea is to modify this to include audio groups as well.

Currently, the video representation is being returned correctly and the resulting video playlist is being requested correctly.

ldayananda · 2019-01-05T01:43:52Z

Appears to be working on at least one source. More testing next week.

src/dash-playlist-loader.js

ldayananda · 2019-01-11T22:08:25Z

One last bug to figure out: looks like both audio playlists are being requested and overwriting each other, potentially due to the state of the master playlist changing while sidx boxes for each playlist are being requested and parsed. Still debugging

ldayananda · 2019-01-24T23:11:57Z

As an update: I discovered some issues with DashSegmentLoader that needs to be addressed before this PR can proceed.

ldayananda · 2019-02-14T01:33:39Z

Now depends on #386 being merged first

ldayananda · 2019-02-22T22:40:27Z

Here are the things that are left to do:

unit tests for sidx cases, and probably a playback test with this source
handle cases where saved sidx doesn't match updatedMaster
handle period changes
- this is deferred for now
Investigating a final bug, mentioned in known issues in the description

- start DashPlaylistLoader in HAVE_NOTHING state - enter HAVE_METADATA in media() method - set media automatically only for child playlist loader as masterPlaylistLoader is set from masterPlaylistController update tests with desired behavior ensure loadedmetadata is triggered after loadedplaylist remove unnecessary media playlist selection for master loader

…urns and once when the initial media is selected

- delay triggering selectedinitialmedia event until the active audio playlist loader is finished setting media - ensure that HLS playlists without alternate audio are not affected

- don't autoselect media for the masterPlaylistLoader as that is done by MasterPlaylistController - ensure updateMaster returns null if there are no changes in the playlist

- trigger loadedmetadata as part of haveMetadata - add some comments to explain why we trigger certain events and set media at points

- moving haveMetadata to a class-level method, more comments. - remove setTimeouts as they were not performing a necessary function. XHR requests can return in any amount of time and HLS PlaylistLoader handles any requests within a callback, or returns if a request need not be made. DASH should do the same - cleanup the minimumUpdatePeriod setup - remove unnecessary clock.ticks from tests

… playlist

…and media

- fix up a comment - add idea for media groups refactor

- correcting merge mistake - moving check for sidx to correct place

- have parseMasterXml use previously downloaded sidxes when refreshing media

- haveMaster - parseMasterXml - handleSidxResponse_ - requestSidx_

- add unit tests for setupChildLoader, hasPendingRequest - add functional test to check if requests are made for sidx boxes

…error case

gkatsev · 2019-03-22T19:39:41Z

Tests are failing because no mpd-parser update. We should review and test and get it in.

gkatsev · 2019-04-05T21:16:38Z

Replaced by #455

ldayananda changed the title ~~[WIP] fix: Request sidx~~ [WIP] fix: dash manifests using SegmentBase would not request sidx Nov 30, 2018

ldayananda self-assigned this Nov 30, 2018

omarroth mentioned this pull request Dec 3, 2018

Add support for sidx segments #207

Closed

6 tasks

forbesjo added the needs updates label Dec 11, 2018

mjneil reviewed Jan 11, 2019

View reviewed changes

src/dash-playlist-loader.js Outdated Show resolved Hide resolved

src/dash-playlist-loader.js Outdated Show resolved Hide resolved

omarroth mentioned this pull request Jan 13, 2019

1080p and other video support iv-org/invidious#34

Closed

mjneil mentioned this pull request Jan 22, 2019

Issues playing dash and hls streams #384

Closed

ldayananda force-pushed the request-sidx branch from e1fddfe to 093fbac Compare February 14, 2019 01:32

ldayananda force-pushed the request-sidx branch 2 times, most recently from 797006d to c622fd3 Compare February 27, 2019 17:33

ldayananda added 14 commits March 11, 2019 16:52

Adding initial tests to ensure coverage of states and events

3a9a95d

fixes live manifests

5aa5d16

loadedplaylist should happen twice, once when the master manifest ret…

0fd7f87

…urns and once when the initial media is selected

Fix eme setup:

85049e8

- delay triggering selectedinitialmedia event until the active audio playlist loader is finished setting media - ensure that HLS playlists without alternate audio are not affected

Fixes:

ff40727

- don't autoselect media for the masterPlaylistLoader as that is done by MasterPlaylistController - ensure updateMaster returns null if there are no changes in the playlist

Cleanup:

67c92cc

- trigger loadedmetadata as part of haveMetadata - add some comments to explain why we trigger certain events and set media at points

only trigger loadedplaylist and loadedmetadata when loading a new VOD…

1bb3dd4

… playlist

Fix: return to using zero delays to mimic asynchronous code in start …

9aab16d

…and media

docs:

65b9d0e

- fix up a comment - add idea for media groups refactor

remove comment

02c8595

fix: media selection fallback

6723894

fix: mock loader.hasPendingRequest for functional tests

6ebf1d2

ldayananda added 23 commits March 11, 2019 16:53

move sidx requesting into the dash playlist loader

f7195dc

first master playlist load complete

ecd9699

make parseMasterXml async

e3109dc

fix test

55e02c2

slight change to make things clearer

38c11a3

request segment base sidxes for media groups also

e320129

clean up code a bit

c3d39f1

CR comments

97e37d6

fix:

31c8ad8

- correcting merge mistake - moving check for sidx to correct place

- update to request sidx on a playlist basis

420fe73

- have parseMasterXml use previously downloaded sidxes when refreshing media

fix: store sidxInfo reference before making request for sidx

77a8e9f

clean up request code

7c74d4b

add integration tests for sidx

e80ad0a

tests: add unit tests for:

4dd41fd

- haveMaster - parseMasterXml - handleSidxResponse_ - requestSidx_

tests:

3d101f8

- add unit tests for setupChildLoader, hasPendingRequest - add functional test to check if requests are made for sidx boxes

tests: added test for asynch haveMaster handling

795f420

fixing rebase mistake

089d874

tests: update handleSidxResponse tests to use class method and check …

94fb586

…error case

add support for sidx info changing in a live playlist

859d111

tests: refreshXml_

ca459b4

fixes for filterSidxMapping and utilities and tests

d42cf8a

linting updates

870c8cc

add redirect support for sidx requests

e2b8754

ldayananda force-pushed the request-sidx branch from cdfd948 to e2b8754 Compare March 11, 2019 21:04

ldayananda changed the title ~~[WIP] fix: dash manifests using SegmentBase would not request sidx~~ feat: add support for dash manifests describing sidx boxes Mar 11, 2019

ldayananda removed the needs updates label Apr 5, 2019

gkatsev closed this Apr 5, 2019

ldayananda deleted the request-sidx branch April 5, 2019 21:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for dash manifests describing sidx boxes #303

feat: add support for dash manifests describing sidx boxes #303

ldayananda commented Nov 30, 2018 •

edited

Loading

ldayananda commented Jan 3, 2019 •

edited

Loading

ldayananda commented Jan 5, 2019

ldayananda commented Jan 11, 2019 •

edited

Loading

ldayananda commented Jan 24, 2019

ldayananda commented Feb 14, 2019

ldayananda commented Feb 22, 2019 •

edited

Loading

gkatsev commented Mar 22, 2019

gkatsev commented Apr 5, 2019

feat: add support for dash manifests describing sidx boxes #303

feat: add support for dash manifests describing sidx boxes #303

Conversation

ldayananda commented Nov 30, 2018 • edited Loading

Description

Specific Changes proposed

Current Status

Sources:

Known Issues

Requirements Checklist

ldayananda commented Jan 3, 2019 • edited Loading

ldayananda commented Jan 5, 2019

ldayananda commented Jan 11, 2019 • edited Loading

ldayananda commented Jan 24, 2019

ldayananda commented Feb 14, 2019

ldayananda commented Feb 22, 2019 • edited Loading

gkatsev commented Mar 22, 2019

gkatsev commented Apr 5, 2019

ldayananda commented Nov 30, 2018 •

edited

Loading

ldayananda commented Jan 3, 2019 •

edited

Loading

ldayananda commented Jan 11, 2019 •

edited

Loading

ldayananda commented Feb 22, 2019 •

edited

Loading