refactor: align DashPlaylistLoader closer to PlaylistLoader states #386
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mjneil
previously requested changes
Jan 30, 2019
added 5 commits
February 7, 2019 17:18
…urns and once when the initial media is selected
- 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
- delay triggering selectedinitialmedia event until the active audio playlist loader is finished setting media - ensure that HLS playlists without alternate audio are not affected
ldayananda
force-pushed
the
dash-loader-fix
branch
from
70ce772 to
cf60c4f
Compare
ldayananda
commented
Feb 7, 2019
| @@ -2915,6 +2915,50 @@ QUnit.test('configures eme if present on selectedinitialmedia', function(assert) | |||
| }, 'set source eme options'); | |||
| }); | |||
|
|
|||
| QUnit.test('integration: configures eme if present on selectedinitialmedia', function(assert) { | |||
There was a problem hiding this comment.
The test above this tests the same thing, but with all the playlist loaders mocked. Should we keep both?
added 3 commits
February 11, 2019 17:49
- 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
ldayananda
force-pushed
the
dash-loader-fix
branch
from
7750428 to
52a1f18
Compare
added 2 commits
February 11, 2019 21:35
mjneil
reviewed
Feb 12, 2019
added 7 commits
February 12, 2019 14:41
3 tasks
mjneil
reviewed
Feb 14, 2019
ldayananda
changed the title
…mention hasPendingRequest for fallback media selection
forbesjo
approved these changes
Feb 27, 2019
ldayananda
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is a change to ensure that DashPlaylistLoader and PlaylistLoader follow a similar setup path. This should make it easier for
sidxboxes received to result in further XHR requests in particular DASH Media Presentations, e.gSegmentBasewithindexRangeSpecific Changes proposed
HAVE_NOTHING -> HAVE_MASTER -> HAVE_METADATAloadedplaylistwhen the master playlist is setup after requesting or refreshing the mpdloadedplaylistwhen a video playlist has finished being selected, or a child playlist(for media groups) has finished being selectedloadedmetadatathe first time a media playlist has been loaded for a specific loaderupdateMastershould return null if there are no manifest changesWait until audioPlaylistLoader has set media to trigger
selectedinitalmediaWe now use
hasPendingRequestto automatically select a media playlist for the masterPlaylistLoader as a fallback in case one is not selected by MPC. The child loaders are created with a media playlist so this fallback is not necessary for them.Sources to use for testing:
Requirements Checklist