Dash-Industry-Forum / dash.js

A reference client implementation for the playback of MPEG DASH via Javascript and compliant browsers.
http://reference.dashif.org/dash.js/nightly/samples/dash-if-reference-player/index.html
Other
5.15k stars 1.68k forks source link

Segmenduration wrong #2940

Closed PolynomialDivision closed 4 years ago

PolynomialDivision commented 5 years ago

The getQualityForBitrate function has this if statement: https://github.com/Dash-Industry-Forum/dash.js/blob/4ee40ce616c37da9f8c17b873362cc50a52e525c/src/streaming/controllers/AbrController.js#L499

Here the Fragmentduartion is fetched: https://github.com/Dash-Industry-Forum/dash.js/blob/4ee40ce616c37da9f8c17b873362cc50a52e525c/src/streaming/controllers/AbrController.js#L498

At this line: https://github.com/Dash-Industry-Forum/dash.js/blob/4ee40ce616c37da9f8c17b873362cc50a52e525c/src/dash/DashAdapter.js#L95

What is this line doing? I have a MPD file specifying the durations by d='...'. I think voRepresentation.segmentDuration is always NaN. And so voRepresentation.segments[0].duration is always taken. Is this always the first segement? What if my durations varies? What is exactly this list voRepresentation.segments? As I understand it, it contains all elements for one Representation? Why can't I use the element here that I want to fetch? Is there some way to get the index of the element I want to fetch?


Here is exactly what I mean: https://github.com/Dash-Industry-Forum/dash.js/blob/4ee40ce616c37da9f8c17b873362cc50a52e525c/src/streaming/thumbnail/ThumbnailTracks.js#L169

I just need to get the duration for the next segment in the list that should be downloaded or checked next... Is there a way to get this segment number?


Furthermore, the segment duration is wrong for the insufficient buffer rule leading to weak download behaviour and stalling.

https://github.com/Dash-Industry-Forum/dash.js/blob/96898aea423ece66fc7749d8573387189d94ac83/src/streaming/rules/abr/InsufficientBufferRule.js#L88

epiclabsDASH commented 5 years ago

@PolynomialDivision, thanks for reporting. The way we manage some of the objects you reference here (voRepresentation.segments, that keeps the next 10 segments to be played for a given representation) is going to suffer a big change in dash.js v3.0 (to be released shortly). So, the issue you mention that is affecting to ABR is going to be fixed "collaterally". I will double check to be sure of this.

PolynomialDivision commented 5 years ago

Thanks for your answer. Can I just checkout the development branch and test it myself? :)

epiclabsDASH commented 5 years ago

Not yet. Still is not published but it will shortly. I will keep you updated.

PolynomialDivision commented 5 years ago

On the dash.js meeting it was said that there is only getNextSegment() left for the scheduler. It was not sounding as the segment time regarding the rules will be fixed in dash.js v3. Further, it sounded that just the getNextSegmentByTime(...) will be fixed.

PolynomialDivision commented 5 years ago

There is some interesting Pull Request https://github.com/Dash-Industry-Forum/dash.js/pull/3003. It's written that using SegmentTimeline representation metrics are not correctly reported and top level application could get erroneous "current bitrate" value. I'm not entirely sure, what that means. is that maybe related to my issue? I'm using SegmentTimeline as well but not for live-streaming. So representation metrics are not correctly reported as well?

epiclabsDASH commented 5 years ago

Not really, it is related with the index of the current level as it is retrieved by the reference player, when playing multiperiod streams.

epiclabsDASH commented 5 years ago

I will send a PR in the next few hours fixing this + extra improvements.

epiclabsDASH commented 5 years ago

@PolynomialDivision, already fixed in the PR I am preparing for v3.0.1. Still not published because I am including on it other fixes related with timing. I will let you know once finished.

No later than early next week so it is ready for the code freeze for v3.0.1.

PolynomialDivision commented 4 years ago

Actually, is this now fixed?