1464268 - Media Recorder - RequestData does not provide data each time when ondataavailable is fired

Status: RESOLVED FIXED

(Core :: Audio/Video: Recording, defect, P2)

Description

[sachin]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.181 Safari/537.36

Steps to reproduce:

Expected behavior as per documentation of requestData is 
The MediaRecorder.requestData() method (part of the MediaRecorder API) is used to raise a dataavailable event containing a Blob object of the captured media as it was when the method was called

Called a requestData every 100 ms


Actual results:

dataavailable does not fire immediately and blob is not available


Expected results:

dataavailable should fired immediately

Comment 1

[Jean-Yves Avenard [:jya]]

Can you please define immediately VS non immediately?
dataavailable will only be fired once at least 1 blob is ready, if your blobs are 100ms long, then there will be at least a 100ms delay for the first blob to be produced.

Additionally, if your frame rate is 10fps, a frame is 100ms long, the vp8 encoder has a small latency which depends on your CPU configuration. So if 2 frames, it will be at best 200ms before the event is queued. Add another few milliseconds for the event to actually be fired (time impacted by how busy the main thread is)
So does 200ms falls in your definition of immediate?

Comment 2

[sachin]

I checked again. ondataavailable  is fired immediately but it does not provide data. I got data after 30 sec.

Does it mean there is no way to retrieve encoded stream from MediaRecorder API? 
Both ways i.e. requestData and start with timeslice does not provide configured WebM stream.  
requestData  provided encoded stream at some large interval.
start with timeslice provide the encoded stream but each blob contains key frames which causes high bitrate

[Reference - bug 1464267]

Comment 3

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Are you able to provide JS example to help reproduce this? In the referenced bug you indicate that you're getting data at the requested intervals (100ms), but that the data is all keyframes. Here you indicate that it takes 30 seconds to get the data event, which is indeed quite a wait.

If you need to pull data quickly from a recorder to provide real time communication you may be better served by the WebRTC API[0].

An explanation to some of the behaviour you're seeing:
- If you have a 10 FPS camera you will get a frame every 100ms.
- If you request a frame every 100ms the MediaRecorder will have been fed 1 frame in that time.
- The MediaRecorder will emit a cluster with 1 frame in it. Per the WebM spec this 1 frame should be a keyframe.
- This process repeats.

If the MediaRecorder is polled less often for data then it will have more flexibility and can encode non-key frames. Because emitting the blob ends up flushing the encoding pipeline, if you poll at your camera frequency you will get keyframes.

[0]: https://developer.mozilla.org/en-US/docs/Web/API/WebRTC_API

Comment 4

[sachin]

Attachment: MediaRecord.zip

Sample example to check Media recorder

Comment 5

[sachin]

Please find attached MediaRecord.zip which includes
1. index.html - test example html
2. main.js    - Media record js 

Main.js has two ways to record
1. mediaRecorder.start(50); // collect 50ms of data
   Request frame at 50 ms interval
   
   Observation - Here each frame is key frame. Above sample does not include WebM parser. But I verified it with WebM parser. 

2. mediaRecorder.start();
   mediaTimer = setInterval(function () {
       mediaRecorder.requestData();
       console.log("RequestData");
   }, 50);

   Observation -  Here frame is received after 20 sec on my PC.

Comment 6

[sachin]

I understood following from your comments.

1. If media recorder client asks data at the frame rate interval i.e. start(timeslice), it will get one encoded frame but each frame is key frame. 
2. If media recorder client asks data with requestData, it will get data at default key frame interval. Default key frame interval as per code seems to be 600 frames. (For 30 fps, it will take 20 sec)

Both ways to obtain encoded frames are not desirable from real time perspective. 

Is there any plan for supporting real time encoded frame retrieval similar to Chrome?  It will make Media Recorder API more useful. 

I also gone through comments in the issue 1411857 where you talked about rewriting the the logic. But due to lack of time, workaround has been done to fix the issue 1411857. 

Regarding our application, we can not use WebRTC as we want to reduce server complexity. Media recorder API makes solution simple. Also we don't have hard requirement for latency etc (few seconds delay is acceptable).

Comment 7

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

(In reply to sachin from comment #6)
> I understood following from your comments.
> 
> 1. If media recorder client asks data at the frame rate interval i.e.
> start(timeslice), it will get one encoded frame but each frame is key frame. 
> 2. If media recorder client asks data with requestData, it will get data at
> default key frame interval. Default key frame interval as per code seems to
> be 600 frames. (For 30 fps, it will take 20 sec)
> 
> Both ways to obtain encoded frames are not desirable from real time
> perspective. 

Since this bug is about the second point I will focus on that. My reading of the spec[0] is that the blob should be fired but may be empty. In your example only non-empty blobs are reported, and the issue is that the time between these blobs is large. I agree that 20 seconds is a long delay here.

If you're able to create a minimal example somewhere like jsfiddle to hightlight this specific issue, that would be helpful. Otherwise I can do so when I find a moment.

> Is there any plan for supporting real time encoded frame retrieval similar
> to Chrome?  It will make Media Recorder API more useful. 

There have been plans to rewrite some of the code around our blob construction, which may allow for more flexibility in terms of the behaviour we're discussing. However, at this time other projects have superseded that work.

[0]: https://www.w3.org/TR/mediastream-recording/ or (draft) https://w3c.github.io/mediacapture-record/

Comment 8

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Minimal code pen for testing: https://codepen.io/SingingTree/pen/qKdPJz

It looks like this has been regressed by bug 1411857. Specifically, if a timeslice is not specified and requestData is called we see a significant increase in the time between when non-empty blobs are surfaced. I imagine that different environmental factors will affect this, but I see a 10 fold increase in the times in my testing. This is permissible given the spec, but is not desirable for users.

Bug 1014393 would bring us closer to better support here (there's follow up work that would see us use a more flexible back-end for writing the recorded data), but work there is stalled.

Comment 9

[sachin]

@Bryce, one question on expected behavior. What behavior you are planning to implement in following cases?  
1. Media recorder client asks data with requestData
   Will it provide cluster or block?

2. Media recorder client asks data using start(timeslice)
   Will it provide cluster or block?

Comment 10

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Right now we surface clusters. In order to provide more flexibility to our timing I would like to see us be able to do blocks (and this is permissable given the spec). The two different cases should be the same in terms of both being able to surface data without needed to write complete clusters each time. However, I don't have a time frame for the work that would change this. The bug mentioned in my last comment is the first step I think we should take towards this, but there would be further work needed on top of that too.

Comment 12

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Fire "start" when all track encoders have received some data. r?bryce!

This no longer waits for the encoders to initialize. We "fire" start when
data has reached the encoders instead. The spec is a bit vague on this:

Start recording all tracks in tracks using the recorder’s current
configuration and gather the data into a Blob blob. Queue a task, using the
DOM manipulation task source, to run the following steps:
(...)
4. Fire an event named start at recorder.

We follow this in the sense of having started "gathering the data into a Blob".
The data is just not there yet, but we know it is coming.

Comment 13

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Remove ref-counting from TrackEncoder. r?bryce!

This would help control its lifetime, needed as it will receive an lref member
in a future patch.

Comment 14

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Constify fixed MediaEncoder members. r?bryce!

Comment 15

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Rename Muxer::mEncoded{Audio|Video}Frames to mEncoded{Audio|Video}Queue. r?bryce!

Comment 16

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Let encoders encode automatically and push directly to MediaQueues. r?bryce!

Prior to this patch, DataAvailable notifications went from each encoder, up to
MediaRecorder::Session, which decided whether to pull data from the encoders for
writing to a container.

Now, data is encoded by the encoders automatically as it reaches them and gets
added to MediaQueues. The Muxer is notified immediately through the MediaQueues
and muxes data into its ContainerWriter continuously.

MediaRecorder::Session still has to manually pull data from ContainerWriter.

Note that this breaks VP8TrackEncoder's frame-dropping policy, because calls
to encode data are no longer coalesced. This gets fixed in a later patch in the
stack.

Comment 17

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Allow for coalesced VideoTrackEncoder::AdvanceCurrentTime calls. r?bryce!

The frame-skipping logic in VP8TrackEncoder works when it gets to encode more
than one frame in a single Encode(). Coalescing the AdvanceCurrentTime like this
will allow for that because if VP8TrackEncoder lags behind, an
AdvanceCurrentTime runnable will take long enough to run (and allow the next
AdvanceCurrentTime dispatch) that multiple frames will be appended in the
meantime.

Comment 18

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Rename MediaEncoder::Stop to DisconnectTracks to better reflect what it does. r?bryce!

Comment 19

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - MozPromisify MediaRecorder::Session::Extract and let GatherBlob paths extract remaining data first. r?bryce!

This ensures that DoSessionEndTask will stop the MediaEncoder first, or cancel
it if data is to be discarded. We also wait for all the data still buffered to
be encoded, muxed and extracted into the blob before proceeding with
"dataavailable".

Comment 20

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Reduce the MediaEncoder::Shutdown MozPromise return type. r?bryce!

Comment 21

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Add MediaEncoder::Stop that stops gracefully and returns a MozPromise. r?bryce!

Comment 22

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Shut down the MediaEncoder automatically when all track encoders have finished. r?bryce!

Comment 23

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Allow transferring metadata after finishing, and getting muxed data before metadata gets set. r?bryce!

Comment 24

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Update MediaEncoder internal state sync on callbacks, and notify MediaRecorder async. r?bryce!

This allows doing internal work synchronously in these callbacks, such as
transferring metadata to the muxer. All callbacks must behave the same to
handle them in the order they were called.

Comment 25

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Move MutableBlobStorage and the timeslice to MediaEncoder. r?bryce!

This will let MediaEncoder extract blobs when a certain duration of
encoded data has been gathered into the blob, in a future patch.

Comment 26

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Merge MediaEncoder's Extract and GatherBlob into RequestData. r?bryce!

Comment 27

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Use timeslice to gather blobs in MediaEncoder. r?bryce!

This brings back dataavailable events that are based on the timeslice interval
to MediaRecorder.

Prior to this stack we used wall-clock time to decide whether to fire
dataavailable events. The spec says "once a minimum of timeslice
milliseconds of data have been collected", meaning the old behavior was wrong
as no guarantee could be given to how much data had been collected, and
especially muxed, for a given duration of wall-clock time.

With this patch the responsibility of triggering dataavailable events lies with
MediaEncoder, which also knows the timeslice. This means it can issue blobs
precisely when they contain enough data to fill the timeslice.

Buffering in the ContainerWriter is a problem that can result in, in the worst
case, empty blobs, as the logic is based on the input to the muxer where
timestamps are still known.

Comment 28

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Change MediaEncoder::Shutdown to not remove TrackEncoder listeners until all data has been processed. r?bryce!

Without this we get an unexpected empty blob in test_mediarecorder_onerror_pause.html.

Comment 29

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Clear VP8TrackEncoder::mMuteFrame on EOS to avoid gfx assertions. r?bryce!

The refactoring has led to the VP8TrackEncoder being cleaned up later on
shutdown. This is fine but is not guaranteed to happen before threads are shut
down (MediaEncoder is ref counted!).

Gfx doesn't like it when a resource is cleaned up this late, so this patch
adds a guarantee that mMuteFrame gets cleaned up before thread shutdown.

Comment 30

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Move remaining MediaEncoder events to MediaEvent. r?bryce!

Comment 31

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Fix MediaEncoder class comments. r?bryce!

Comment 32

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Fix MediaRecorder comments. r?bryce!

Comment 33

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1464268 - Convert static vars to constexpr in MediaRecorder and friends. r?bryce!

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d1768aade7f3
Fire "start" when all track encoders have received some data. r=bryce
https://hg.mozilla.org/integration/autoland/rev/48815139e52e
Remove ref-counting from TrackEncoder. r=bryce
https://hg.mozilla.org/integration/autoland/rev/41d12b06c732
Constify fixed MediaEncoder members. r=bryce
https://hg.mozilla.org/integration/autoland/rev/22ec0122e2b0
Rename Muxer::mEncoded{Audio|Video}Frames to mEncoded{Audio|Video}Queue. r=bryce
https://hg.mozilla.org/integration/autoland/rev/4a187d94c6a2
Let encoders encode automatically and push directly to MediaQueues. r=bryce
https://hg.mozilla.org/integration/autoland/rev/e5b53ed048f4
Allow for coalesced VideoTrackEncoder::AdvanceCurrentTime calls. r=bryce
https://hg.mozilla.org/integration/autoland/rev/bb840d443ce5
Rename MediaEncoder::Stop to DisconnectTracks to better reflect what it does. r=bryce
https://hg.mozilla.org/integration/autoland/rev/e43d3445f0d4
MozPromisify MediaRecorder::Session::Extract and let GatherBlob paths extract remaining data first. r=bryce
https://hg.mozilla.org/integration/autoland/rev/87a8cf23da93
Reduce the MediaEncoder::Shutdown MozPromise return type. r=bryce
https://hg.mozilla.org/integration/autoland/rev/7ef0811b3e59
Add MediaEncoder::Stop that stops gracefully and returns a MozPromise. r=bryce
https://hg.mozilla.org/integration/autoland/rev/267b361271e2
Shut down the MediaEncoder automatically when all track encoders have finished. r=bryce
https://hg.mozilla.org/integration/autoland/rev/d459b05e1959
Allow transferring metadata after finishing, and getting muxed data before metadata gets set. r=bryce
https://hg.mozilla.org/integration/autoland/rev/334c6165b8a0
Update MediaEncoder internal state sync on callbacks, and notify MediaRecorder async. r=bryce
https://hg.mozilla.org/integration/autoland/rev/28563731435b
Move MutableBlobStorage and the timeslice to MediaEncoder. r=bryce
https://hg.mozilla.org/integration/autoland/rev/7ef85a640ab2
Merge MediaEncoder's Extract and GatherBlob into RequestData. r=bryce
https://hg.mozilla.org/integration/autoland/rev/bb3690c52cb9
Use timeslice to gather blobs in MediaEncoder. r=bryce
https://hg.mozilla.org/integration/autoland/rev/4bf128c1bebc
Change MediaEncoder::Shutdown to not remove TrackEncoder listeners until all data has been processed. r=bryce
https://hg.mozilla.org/integration/autoland/rev/a6e0db017654
Clear VP8TrackEncoder::mMuteFrame on EOS to avoid gfx assertions. r=bryce
https://hg.mozilla.org/integration/autoland/rev/7464b3036653
Move remaining MediaEncoder events to MediaEvent. r=bryce
https://hg.mozilla.org/integration/autoland/rev/8a26c0496e18
Fix MediaEncoder class comments. r=bryce
https://hg.mozilla.org/integration/autoland/rev/925b22cbd3b6
Fix MediaRecorder comments. r=bryce
https://hg.mozilla.org/integration/autoland/rev/e210b5200da9
Convert static vars to constexpr in MediaRecorder and friends. r=bryce

Comment 35

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/d1768aade7f3
https://hg.mozilla.org/mozilla-central/rev/48815139e52e
https://hg.mozilla.org/mozilla-central/rev/41d12b06c732
https://hg.mozilla.org/mozilla-central/rev/22ec0122e2b0
https://hg.mozilla.org/mozilla-central/rev/4a187d94c6a2
https://hg.mozilla.org/mozilla-central/rev/e5b53ed048f4
https://hg.mozilla.org/mozilla-central/rev/bb840d443ce5
https://hg.mozilla.org/mozilla-central/rev/e43d3445f0d4
https://hg.mozilla.org/mozilla-central/rev/87a8cf23da93
https://hg.mozilla.org/mozilla-central/rev/7ef0811b3e59
https://hg.mozilla.org/mozilla-central/rev/267b361271e2
https://hg.mozilla.org/mozilla-central/rev/d459b05e1959
https://hg.mozilla.org/mozilla-central/rev/334c6165b8a0
https://hg.mozilla.org/mozilla-central/rev/28563731435b
https://hg.mozilla.org/mozilla-central/rev/7ef85a640ab2
https://hg.mozilla.org/mozilla-central/rev/bb3690c52cb9
https://hg.mozilla.org/mozilla-central/rev/4bf128c1bebc
https://hg.mozilla.org/mozilla-central/rev/a6e0db017654
https://hg.mozilla.org/mozilla-central/rev/7464b3036653
https://hg.mozilla.org/mozilla-central/rev/8a26c0496e18
https://hg.mozilla.org/mozilla-central/rev/925b22cbd3b6
https://hg.mozilla.org/mozilla-central/rev/e210b5200da9