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

Implement low latency mode for live streams #2427

Merged
merged 14 commits into from Apr 23, 2018

Conversation

mmarciel
Copy link
Contributor

This PR implements a low latency mode implementing points 1 and 2 of @wilaw comment in #1474 and it is based on the implementation of @TobbeEdgeware team.

There is a new parameter lowLatencyMode to control the mode in live streams.

Main changes:

  1. Using fetch and reading output as stream (feature only available in Chrome). There is a new layer for requests (HTTPLoader.js) that decides whether to use xhr or fetch. Fetch is only used when parameter lowLatencyMode is true.
  2. When using fetch, bytes are appended to the buffer (in the event FRAGMENT_LOADING_PROGRESS). Only when the first segment has been downloaded, it starts playing (event BYTES_APPENDED_END_FRAGMENT).
  3. Setting live edge more aggressive. The live edge is computed with the formula: startTime + duration - liveDelay. liveDelay is set to 3s by default.

…ing the start of the stream at the current time
Copy link
Contributor

@davemevans davemevans left a comment

Choose a reason for hiding this comment

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

Just some initial comments from a first quick look. Great first effort! 👍

@@ -370,7 +370,8 @@ app.controller('DashController', function ($scope, sources, contributors, dashif
liveDelay: $scope.defaultLiveDelay,
stableBufferTime: $scope.defaultStableBufferDelay,
bufferTimeAtTopQuality: $scope.defaultBufferTimeAtTopQuality,
bufferTimeAtTopQualityLongForm: $scope.defaultBufferTimeAtTopQualityLongForm
bufferTimeAtTopQualityLongForm: $scope.defaultBufferTimeAtTopQualityLongForm,
lowLatencyMode: $scope.defaultLowLatencyMode
Copy link
Contributor

Choose a reason for hiding this comment

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

defaultLowLatencyMode doesn't appear to be defined anywhere?

@@ -150,7 +150,11 @@ function TimelineConverter() {
const now = calcPresentationTimeFromWallTime(new Date(), voPeriod);
const periodEnd = voPeriod.start + voPeriod.duration;
range.start = Math.max((now - voPeriod.mpd.timeShiftBufferDepth), voPeriod.start);
range.end = now >= periodEnd && now - d < periodEnd ? periodEnd - d : now - d;
if (lowLatencyMode) {
range.end = now >= periodEnd && now < periodEnd ? periodEnd : now;
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look right - the first chunk isn't available until after the chunk duration. Just subtracting the segment duration could lead to segments being requested early.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bbcrddave, that seems to be true for regular streams but for low latency ones a chunk should start being available as soon as the duration of its first sub-chunk. Furthermore, if HTTP server supports chunked transfer encoding, and the process in charge of generating chunks is flushing each sub-chunk before they are fully completed, we could start downloading the chunk as soon as the server has something ready to be served (first frames encoded?).

Our above suggestion looks a bit aggressive (we assume "now" is covered by the latest segment), but this is just to inform about segments availability. We are assuming segment for live time is available (it should almost be), although later on, when calculating edge live position, we apply a safety margin that protects player for requesting a time position that still is not covered in the latest segment.

Unfortunately there is nothing in the mpd (or at least nothing that I know) that indicates the duration of sub-chunks so the value of the safety margin used for calculating edge live position is something we should expose though Dash.js api. When trying to reduce latency as much as possible, it is not the same having sub-chunks of 1 second, than sub-chunks of 0.5 seconds.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless the mdat is being sent before the moof is written (in which case the player would need to be aware of this and reorder the boxes before they are appended to be compliant with MSE, causing latency equal to what it would have been anyway), a CMAF chunk should only be available once the moof has been written. By definition this cannot occur until the mdat for that chunk has been completed, so that would be the earliest point at which anything would be available, regardless of the transfer mode,

We shouldn't be assuming anything regarding availability - the client should know what is available at all times. The lowest latency would be achieved by requesting chunks/segments as soon as they are available.

In terms of manifest signalling, whilst I am not sure that it was the original intention, it seems that some streams intend to use @availabilityTimeOffset in order to signal the offset to the availabilityStartTime at which the first chunk becomes available. I think @availabilityTimeComplete should probably be set to false to indicate the segment is not complete at the offset time, but I have not seen that. At least if these or some similar signalling were used, clients not able to understand the signalling would not be affected.

It seems to me that there really needs to be some standardisation around this topic before much more work is done, as currently there seems to be no authoritative test streams or player requirements, or content production guidelines for that matter. Some work is due to kick off in DVB this week. Not sure if DASH-IF has anything planned (or has already completed something and I have not seen it).

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right, the minimum unit would be a CMAF chunk (as you said, moof atom can only be built, and written, once we have built the mdat one), independently of using chunked transfer encoding or not.

@bbcrddave, diving into some of the latest published information regarding DASH and low latency, seems people agree on using @availabilityTimeOffset to signal availability of the first chunk within a segment and @availabilityTimeComplete sets to false. Looking at DASH-IF IOP 4.1, its description for @availabilityTimeOffset confirms that:

If files are available in chunks on the origin, for example due to specific encoding or delivery matters, chunked delivery may be supported. If this feature is offered, then the @availabilityTimeOffset attribute may be provided to announce how much earlier than the nominal segment availability the segment can be accessed.

That also matches with your comment.

According to this, we are going to modify timing behavior so we take into account availabilityTimeOffset instead of using an arbitrary value.

Copy link
Member

Choose a reason for hiding this comment

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

@epiclabsDASH - this interpretation and use of @availabilitytimeoffset with chunked low latency encoding is correct. If a stream has 1s segments with 100ms chunks, then @availabilitytimeoffset should be set to 0.9. This instructs the player that it can make a request for that segment 0.9s earlier than it would if the content were non-chunked.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great. I am going to push a commit in the next few minutes with that interpretation implemented. It is now under testing. Thanks to both


fetch(httpRequest.url, reqOptions).then(function (response) {
if (!response.ok) {
httpRequest.onend();
Copy link
Contributor

Choose a reason for hiding this comment

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

onend expects to read statusText from the response, but that doesn't appear to be set until below.

);

if (config.error) {
config.error(request, 'error', httpRequest.http.statusText);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be httpRequest.response.statusText? (although that probably won't be set - see above)

if (x.response && x.response.abort) {
x.response.abort();
} else if (x.reader) {
x.reader.cancel();
Copy link
Contributor

Choose a reason for hiding this comment

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

It only works on Firefox, but the correct way to cancel fetch is using AbortController: https://developer.mozilla.org/en-US/docs/Web/API/AbortController/abort.

request.requestStartDate = requestStartTime;
}

xhr = requestModifier.modifyRequestHeader(xhr);
Copy link
Contributor

Choose a reason for hiding this comment

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

This functionality seems to have been removed completely? This would break anyone relying on header authentication etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right. This was removed when the loader abstraction layer was added. Needs to be restored.

}
httpRequest.response.responseHeaders = responseHeaders;

const totalBytes = parseInt(response.headers.get('Content-Length'), 10);
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't really make sense. When using chunked transfer encoding, the content length is omitted as it is unknown, so totalBytes will always be NaN.

Copy link
Contributor

Choose a reason for hiding this comment

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

I will take a look at this part. There is just one case in which totalBytes could be higher than 0, that is when a user enables Low Latency Mode but the HTTP server doesn't support chunked transfer encoding. In those cases we have two options:

  • Detect that situation and disable low latency mode after the first request. More complex to deal with (what we do in situations in which segments could be served from more than one server/cdn? one could support chunked transfer encoding while the other no)
  • Continue using fetch and mimic the behavior of xhr loader.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think you should disable low latency mode if the application has set it. That leads to awkward loss of state. If the server doesn't support chunked transfer of the chunked-encoded content, then there is no harm in continuing to use the exact same fetch based routine to retrieve the data. The difference will be that instead of the data streaming in at a relatively constant rate, there will be nothing and then a sudden dump of data at the completion of the segment (analogous to what we get with XHR today).

Copy link
Contributor

Choose a reason for hiding this comment

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

analogous to what we get with XHR today

Agreed, and in which case, why bother switching out XHRLoader and FetchLoader at all? On supported platforms, it seems we should just use Fetch all the time (assuming the Loaders have feature parity)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see this as a temporary solution. Low Latency support (fetch included) is going to be released in experimental mode (need more tests, feedback from community, etc). It is a change that affects other parts of dash.js (for example, required us to modify the way we interact with SourceBuffers, it will affect the ABR algorithm) and don't want to apply it without being sure it is not breaking anything and it is not impacting support for any browser/device.

Once we feel confortable, I agree with you. We could use Fetch as the primary way for doing requests and use XHR just as a fallback mechanism in case the device/browser doesn't support fetch.


httpRequest.reader = response.body.getReader();

const processResult = function ({ value, done }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is all of this strictly necessary? As long as you ensure that bytes are appended to the source buffer in the order in which they are received, MSE is able to do all of this hard work without the need for expensive JS memory manipulation.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am working in some changes related with this part. @bbcrddave, I will let you know once modifications are applied.

@epiclabsDASH
Copy link
Contributor

Agree with you. I put that there because I have some streams defining availabilityTimeOffset in baseUrl and wanted to test as much streams as possible. Anyway, that part of the code will be removed before merging.

@@ -184,13 +186,17 @@ function BufferController(config) {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I've just noticed that the event stream stuff above no longer works in low latency mode and an exception is throw in handleInbandEvents if a stream signals it contains inband events. This is because getRequests for state FRAGMENT_MODEL_EXECUTED will return nothing until the request has actually completed (ie the whole segment has arrived), and therefore request is undefined. The problem seems to arise from onMediaFragmentLoaded is called for incomplete chunks of data.

Perhaps there will need to be some box handling in the downloader/elsewhere after all, since the UA is not capable of handling this box - it must be done in JS.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, @bbcrddave. @mmarciel, did in that way (box parsing using codem-isoboxer) in her first implementation because she detected some stability issues when working with not completed boxes. This was later changed to reduce the impact on performance.

I am now reviewing this part. In the worst case, in case there is no way of avoiding parsing, I think makes sense using a new and simplified "parser" focused on speed/perf for this, instead of using codem-isoboxer. We really don't need to parse boxes. Just reading their lengths, and probably their types so we just append when finding specific box types, would be enough. It will be just parsing a few numbers and move though the data array which should be quite fast.

I will share my results.

Copy link
Contributor

Choose a reason for hiding this comment

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

Apart from issues with events, I have been testing a lot (quite a lot) this in Chrome/Firefox and not inserting complete boxes also causes stability issues from time to time. Although most of the time works, there are situations in which playback is interrupted because a decoding issue.

As a solution I have added a new method to BoxParser.js which finds specific and completed top ISO boxes in a data array. It just iterates over top boxes and didn't parse anything so it is quite fast and I don't expect perceivable perf degradation because it. It is around 50-100x faster than parsing same chunks with ISOBox

Not a fair comparison because ISOBox is not just iterating over top boxes, it iterates over all boxes and it is also parsing payloads. However, for what we need here, it is enough with ensuring moov/mdat boxes are received so we don't put anything that breaks browser decoding process.

I am going to do further testing using this new approach.

Copy link
Member

Choose a reason for hiding this comment

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

@epiclabsDASH - if we mainly use ISOBox for retrieving EMSG messages, then would your new parser be a replacement for that, assuming we added a component to it to parse only the EMSG payload?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, with the addition of the new parser I was not thinking on replacing ISOBox completely, at least for this PR. ISOBox is also used by MSS module, and not just as a box parser, it is also used to create boxes to ensure fragments have the structure required by the browser. So, replacing it completely will take time.

What I think makes sense is start replacing it in the cases in which we can get better performance doing something focused just on our requirements. ISOBox is a great library correctly designed for a generic purpose but, for example, for retrieving EMSG payload I think using it is a sledgehammer to crack a nut. We really don't need parsing all the boxes of a chunk to get payload of one (or more) of them. We can do much, much faster if we implement a parser that does just what we need.

So, as a summary, in the short term we won't replace ISOBox completely. In the short term we will move parsing stuff that affects performance (EMSG boxes) to this new parser.

x.controller.abort();
} else if (x.reader) {
// For Chrome
x.reader.cancel();
Copy link
Contributor

Choose a reason for hiding this comment

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

This can throw a TypeError which ought to be caught. For example, calling this after a network error will cause it to throw.

Copy link
Contributor

Choose a reason for hiding this comment

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

Further to the above, this whole method seems to break the abstraction. Surely each request should just have an abort method or similar and the underlying functionality should be encapsulated there?

@yogevNisim
Copy link

Hey, can you share your insights about what is the lowest latency possible reaching with this change?

@epiclabsDASH
Copy link
Contributor

@yogevNisim, that depends on how you are packaging your content (length of sub-segments) and on how aggressive you would like to be with the live buffer (how close to the live edge you would like to be). In my tests, I could reach latencies around 2-4 seconds with smooth playback experience. Note: This is the latency as it is measured by dash.js, which doesn't take into account latency introduced in the encoding phase.

@yogevNisim
Copy link

@epiclabsDASH just for confirm, the 2-4 is the latency regardless to the encoding? i.e if my encoder is encoding in 800ms latency, i will experience a 2.8 - 4.8 seconds?

@epiclabsDASH
Copy link
Contributor

epiclabsDASH commented May 3, 2018

Yes, you are right. It is the latency regardless to the encoding.

@yogevNisim
Copy link

Minimizing the buffer wouldn't help?

@epiclabsDASH
Copy link
Contributor

epiclabsDASH commented May 3, 2018

Sure, you can use setLiveDelay method for that. With it, you are defining an extra delay compared with what dash.js considers the bleeding live edge (calculates using suggested presentation delay, availabilityTimeOffset or minBufferTime, depending on the mpd and on how you configure dash.js). I am preparing a wiki page in which I explain how dash.js uses these parameters to calculate the playback starting position (and then the starting delay) for live streams. I will let you know once it is ready.

By default, for low latency streams, live latency is set to 3 seconds. If you set it with a lower value you will reduce the latency although will increase the possibilities of impacting negatively playback experience. The lower it is, higher are the possibilities of suffering rebuffering events.

@haudiobe
Copy link

haudiobe commented May 4, 2018 via email

@epiclabsDASH
Copy link
Contributor

Hi @haudiobe. Sorry, I think I explained myself wrong. What I am preparing is information related with how dash.js calculates live stream position (with low latency or not) and what dash.js methods you can use to modify that behaviour. There are recurrent questions regarding that (with low latency or not) and I think make sense clarifying how dash.js works. It would be a similar approach we are following when we are documenting how buffering or ABR algorithm work in dash.js.

It is not the intention explaining how low latency feature is defined (and should be implemented) from the point of view of the spec/IOP. Anyway, good to know you are attending to f2f meeting, we will have the opportunity to discuss this topic there.

Thanks!

@wilaw
Copy link
Member

wilaw commented May 5, 2018 via email

epiclabsDASH added a commit that referenced this pull request May 8, 2018
Fix merge problems in persistent licenses functionality caused after merging #2427
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants