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

Dashjs v3 - New Segments Management Model #3003

Merged
merged 29 commits into from
Jun 21, 2019

Conversation

epiclabsDASH
Copy link
Contributor

@epiclabsDASH epiclabsDASH commented Jun 18, 2019

Description

It took long time and lot of back and forth but finally, with this PR, I am proposing the first set of big changes for dash.js v3.

As promised during the last face to face dash.js meeting in Berlin, this PR is coming with a re-architecture of the classes that are in charge of segments management so dash.js is less sensible to segment timing and/or alignment issues. We have been working in a much bigger change (fully rearchitecture dash.js core) during the last months but, given those are requiring more time than expected, what we have done is taking that big refactor and replacing just the parts of dash.js code base that we consider more sensible. That allows us to release faster and start getting feedback from you as soon as possible. We will incorporate all the other changes with a 1 month cadence, including all the fixes for the detected issues.

Most important changes coming in this PR:

  • NextFragmentRequestRule. Code slightly changed, logic changed a lot. With the new approach NextFragmentRequestRule is less time dependant. Instead of starting to find the right segment to load by time, current approach leverages the fact that chunks are usually played sequentially. In other words, if the last segment we requested was Massive rearchitecture. #1, the next one should be Add feature detection for MediaSource support in reference player #2, the next one should be Initial commit of Media Source Extension tests. #3 and so on, (at least there was a seek operation in the middle).
  • SegmentsGetter. These are not needed anymore. All segments requests are built "on the fly" taking into account the index of the segment requested or its time.
  • DashHandler. Because its relation with SegmentsGetter and the mix of responsibilities that it has (using segmentsGetter, implement SegmentBase "type of segments" by itself, etc). We have simplify its code and its responsibilities.
  • SegmentsController. We have added a new component, SegmentsController, whose responsibility is build segment objects given an index or time and that abstract the underlying segment method.
  • SegmentBaseGetter. Added a new segment getter that implements the functionality behind managing segments that are defined in the manifest using SegmentBase method.

Please, I would like to ask you to test this PR as much as possible. I have uploaded it to http://dashjsv3.surge.sh/samples/index.html to make easier the testing phase. We are 200% dedicated to this so, please, feel free to report any issue you find. It is really important for me to get a fully stable version so I will be happy to work immediately on fixing any of your reported issues.

Pending work

  • Working on unit tests for the new classes

Known issues:

  • Under certain circumstances, for live streams using SegmentTimeline, we are not correctly calculating the DVRWindow of the stream. We are working on a fix for this.
  • SegmentBase. We have detected a sporadic issue in SegmentBase streams when playing for a whil and after detecting the end of the stream. It is not easy to reproduce but we are working on it.
  • Sometimes in multiperiod streams using SegmentTimeline, representation metrics are not correctly reported and top level application could get erroneous "current bitrate" value from dash.js. We are working on this.

@epiclabsDASH epiclabsDASH added this to the v3.0 milestone Jun 18, 2019
@epiclabsDASH epiclabsDASH requested review from nicosang and bbert June 18, 2019 01:06
src/dash/controllers/SegmentsController.js Show resolved Hide resolved
src/dash/controllers/SegmentsController.js Show resolved Hide resolved
src/dash/controllers/SegmentsController.js Show resolved Hide resolved
@@ -40,48 +40,45 @@ function ListSegmentsGetter(config, isDynamic) {

let instance;

function getSegmentsFromList(representation, requestedTime, index, availabilityUpperLimit) {
function getSegmentByIndex(representation, index) {
const list = representation.adaptation.period.mpd.manifest.Period_asArray[representation.adaptation.period.index].
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add tests of representation and index parameters

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that's planned for this PR.

seg.mediaRange = s.mediaRange;
seg.index = s.index;
seg.indexRange = s.indexRange;
function getSegmentByTime(representation, requestedTime) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add tests of representation and requestedTime parameters

src/dash/utils/TimelineSegmentsGetter.js Show resolved Hide resolved
src/streaming/SourceBufferSink.js Outdated Show resolved Hide resolved
src/streaming/controllers/ScheduleController.js Outdated Show resolved Hide resolved
src/streaming/controllers/ScheduleController.js Outdated Show resolved Hide resolved
src/streaming/controllers/ScheduleController.js Outdated Show resolved Hide resolved
jeoliva and others added 2 commits June 19, 2019 17:37
…mbnail_urls

Fix thumbnail sample hostnames to point to a hostname with a valid cert
@epiclabsDASH epiclabsDASH merged commit 486974a into Dash-Industry-Forum:development Jun 21, 2019
@epiclabsDASH epiclabsDASH deleted the dashjsv3 branch June 23, 2019 19:32
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.

4 participants