Closed Bug 1464268 Opened 6 years ago Closed 4 years ago

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

Categories

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

Product:
Core
Component:
Audio/Video: Recording
Version:
61 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
87 Branch
Tracking Flags:
Tracking Status
firefox-esr78 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox85 --- wontfix
firefox86 --- wontfix
firefox87 --- fixed

People

(Reporter: sachinhambar, Assigned: pehrsons)

References

(Regression)

Details

(Keywords: regression)

Attachments

(23 files)

MediaRecord.zip
(deleted), application/x-zip-compressed
Details
Bug 1464268 - Fire "start" when all track encoders have received some data. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Remove ref-counting from TrackEncoder. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Constify fixed MediaEncoder members. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Rename Muxer::mEncoded{Audio|Video}Frames to mEncoded{Audio|Video}Queue. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Let encoders encode automatically and push directly to MediaQueues. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Allow for coalesced VideoTrackEncoder::AdvanceCurrentTime calls. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Rename MediaEncoder::Stop to DisconnectTracks to better reflect what it does. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - MozPromisify MediaRecorder::Session::Extract and let GatherBlob paths extract remaining data first. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Reduce the MediaEncoder::Shutdown MozPromise return type. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Add MediaEncoder::Stop that stops gracefully and returns a MozPromise. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Shut down the MediaEncoder automatically when all track encoders have finished. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Allow transferring metadata after finishing, and getting muxed data before metadata gets set. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Update MediaEncoder internal state sync on callbacks, and notify MediaRecorder async. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Move MutableBlobStorage and the timeslice to MediaEncoder. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Merge MediaEncoder's Extract and GatherBlob into RequestData. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Use timeslice to gather blobs in MediaEncoder. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Change MediaEncoder::Shutdown to not remove TrackEncoder listeners until all data has been processed. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Clear VP8TrackEncoder::mMuteFrame on EOS to avoid gfx assertions. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Move remaining MediaEncoder events to MediaEvent. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Fix MediaEncoder class comments. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Fix MediaRecorder comments. r?bryce!
(deleted), text/x-phabricator-request
Details
Bug 1464268 - Convert static vars to constexpr in MediaRecorder and friends. r?bryce!
(deleted), text/x-phabricator-request
Details
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
Component: Untriaged → Audio/Video
Product: Firefox → Core
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?
Flags: needinfo?(sachinhambar)
Flags: needinfo?(sachinhambar)
Summary: Media Recorder - RequestData does not fire ondataavailable immediately → Media Recorder - RequestData does not provide data each time when ondataavailable is fired
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]
Component: Audio/Video → Audio/Video: Recording
Flags: needinfo?(bvandyk)
Whiteboard: [need info bryce 2018-05-25]
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
Flags: needinfo?(bvandyk) → needinfo?(sachinhambar)
Attached file MediaRecord.zip (deleted) —
Sample example to check Media recorder
Flags: needinfo?(sachinhambar)
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.
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).
(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/
Assignee: nobody → bvandyk
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.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Blocks: 1411857
Keywords: regression
Rank: 29
Priority: -- → P3
@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?
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.
status-firefox64: --- → wontfix
status-firefox65: --- → fix-optional
status-firefox66: --- → fix-optional
Whiteboard: [need info bryce 2018-05-25]
Blocks: 1577198
Assignee: bvandyk → apehrson
Priority: P3 → P2

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.

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

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.

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.

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".

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.

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

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.

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

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.

Pushed by pehrsons@gmail.com: 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
Regressions: 1693045
Regressions: 1697537
Regressions: 1697521
Regressions: 1696794
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: