I have tried the following 1. create a canvas not attached to the DOM 2. capture canvas stream 3. use setTimeout to draw images onto the canvas at variable intervals i did that in two variants: a) canvas.captureStream() b) canvas.captureStream(0) + triggering stream.requestFrame() after each draw In both cases the result was a 30fps constant framerate webm instead of the expected VFR output. MDN docs[1] suggest that not explicitly setting a framerate should only capture frames when something is drawn on the canvas. reading the spec[2], specifically this part: > If new content has been drawn to the canvas since it was last painted, and if the frameCaptureRequested internal property of track is set, add a new frame to track containing what was painted to the canvas. also suggests that setting 0fps on captureStream() should not add any frames unless explicitly requested. [1] https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement/captureStream [2] https://w3c.github.io/mediacapture-fromelement/#methods-1

I want to fix this to be able to get >30fps videos.

Comment on attachment 8826164 [details] Bug 1231848 - Encode every frame as it comes in to VP8TrackEncoder. https://reviewboard.mozilla.org/r/104186/#review104912 This patch does what it says basically, and simplifies the code, but I have one big problem with this patch - it assumes the input durations on the videochunks are correct. There is no way to know the correct duration *until* the next frame arrives somewhere. Since we don't want to delay inserting frames into the graph, we choose the mimimum duration requested by MSG in the pull when we initially insert data. When we get the next NotifyPull(), if a new frame hasn't arrived, we will extend the duration of the current frame (by calling AppendToTrack with the same image, and the additional duration the graph is running for). Note that this is *never* a very accurate representation of the total duration of the frame, which would be based on frame capture time of the *next* frame minus the frame capture time of this frame - and of course that can't be known until the next frame arrives. A reasonable approximation of this would be final duration of a segment when the next frame gets added.... We do notify consumers of each extension of the frame time, by passing the new Chunk with the same image, and the additional delta duration. If you set up a 1-frame queue before the encoder, you can approximate the duration by accumulating the durations. That would add it's own complications (and it's closer to the old code). Also: the old code implicitly handled this "Notify with same image" behavior by grouping into target-encode-duration chunks (though perhaps not optimally). With the new code, I'm concerned we might encode the same image more than once, with very short durations. Perhaps we don't (I didn't dig deep enough to be sure, given the other issues), but it's a question we need to answer (and document in comments) before landing this.

Comment on attachment 8826163 [details] Bug 1231848 - Do not write framerate metadata to webm container in MediaRecorder. https://reviewboard.mozilla.org/r/104184/#review104928 r? rillian since he knows more about WebM than I do Patch seems ok, but I'm unsure as to the impact of ommitting the framerate from the WebM stream on players.

Comment on attachment 8826163 [details] Bug 1231848 - Do not write framerate metadata to webm container in MediaRecorder. Adding rillian from bugzilla interface

(In reply to Randell Jesup [:jesup] from comment #4) > Comment on attachment 8826164 [details] > Bug 1231848 - Encode every frame as it comes in to VP8TrackEncoder. > > https://reviewboard.mozilla.org/r/104186/#review104912 > > This patch does what it says basically, and simplifies the code, but I have > one big problem with this patch - it assumes the input durations on the > videochunks are correct. > > There is no way to know the correct duration *until* the next frame arrives > somewhere. You are right, but you are also wrong :-) VideoTrackEncoder makes sure the frames are unique once they get to VP8TrackEncoder: http://searchfox.org/mozilla-central/source/dom/media/encoder/TrackEncoder.cpp#276 If you want I can add an assert to that effect. (In reply to Randell Jesup [:jesup] from comment #5) > Comment on attachment 8826163 [details] > Bug 1231848 - Do not write framerate metadata to webm container in > MediaRecorder. > > https://reviewboard.mozilla.org/r/104184/#review104928 > > r? rillian since he knows more about WebM than I do > > Patch seems ok, but I'm unsure as to the impact of ommitting the framerate > from the WebM stream on players. kinetik assured me in Hawaii that it was ok to omit the framerate metadata if you have a variable framerate.

Comment on attachment 8826164 [details] Bug 1231848 - Encode every frame as it comes in to VP8TrackEncoder. See comment 7. Actually my biggest concern with this patch is that it sets the maximum key frame interval to 60 frames (1 second at 60fps, which is what we can get when driven by the compositor generally), but we have no idea what the frame rate we'll pass in to the encoder is.

This is probably outside the spec, but would it also be possible to pass the timing information directly for each frame, e.g. if the code calling .requestFrame() knows the difference in time relative to the previous frame, even if processing delay makes it submit it a few milliseconds later than intended?

(In reply to The 8472 from comment #9) > This is probably outside the spec, but would it also be possible to pass the > timing information directly for each frame, e.g. if the code calling > .requestFrame() knows the difference in time relative to the previous frame, > even if processing delay makes it submit it a few milliseconds later than > intended? requestFrame() is async by design. Frame time depends on first render after drawing something. And if two captureStreams requestFrame() at different times to the same canvas (and get the same frame), what do you do?

Hrm, I guess it provides too little control for my use-case then (synthesizing VFR video with accurate timing for individual frames). I'll have to look into some emscripten ports of ffmpeg and mkvmerge.

Comment on attachment 8826164 [details] Bug 1231848 - Encode every frame as it comes in to VP8TrackEncoder. https://reviewboard.mozilla.org/r/104186/#review104960 we can get duplicate frames due to bug 1261007, but only once per second. One issue is that the AppendVideoSegment code above us does NOT truly generate frame durations, it approximates them by taking time-since-last-frame and adding the current chunk's duration (typically ~10ms due to how frames are inserted into MSG). For real variable-frame-rate sources, this could be mildly wrong to Very wrong. (For example, a canvas.captureStream source, with say frame ... 100ms ... frame ... 100ms ... frame ... 900ms ... frame ... 100ms ... frame will yield durations of probably around 10ms (should be 100), 100ms, 100ms (should be 900), 900ms (should be 100) etc I'll r+, but please file a followup to fix the durations by waiting for the next frame (which may mean delaying muxing too, not sure).

Comment on attachment 8826163 [details] Bug 1231848 - Do not write framerate metadata to webm container in MediaRecorder. https://reviewboard.mozilla.org/r/104184/#review104982 ::: media/libmkv/WebMElement.c:85 (Diff revision 1) > if (pixelHeight != displayHeight) { > Ebml_SerializeUnsigned(glob, DisplayHeight, displayHeight); > } > + if (frameRate > 0.0) { > - Ebml_SerializeFloat(glob, FrameRate, frameRate); > + Ebml_SerializeFloat(glob, FrameRate, frameRate); > + } `FrameRate` is deprecated, so removing this entirely should be fine. I'd suggest removing the argument from `writeVideoTrack()` entirely. The only way to declare a fixed frame rate in WebM is to use the `DefaultDuration` element and not override it with conflicting `BlockDuration` elements, so AFACT WebM is inherently variable framerate. Leaving those elements out mean the frame durations are interpreted from the timestamps. So while I would prefer to produce a fixed framerate stream if one is requested through `HTMLCanvasElement::captureStream()` always returning variable framerate stream approximating the requested framerate complies with the spec.

Comment on attachment 8826163 [details] Bug 1231848 - Do not write framerate metadata to webm container in MediaRecorder. https://reviewboard.mozilla.org/r/104184/#review104982 > `FrameRate` is deprecated, so removing this entirely should be fine. I'd suggest removing the argument from `writeVideoTrack()` entirely. > > The only way to declare a fixed frame rate in WebM is to use the `DefaultDuration` element and not override it with conflicting `BlockDuration` elements, so AFACT WebM is inherently variable framerate. Leaving those elements out mean the frame durations are interpreted from the timestamps. > > So while I would prefer to produce a fixed framerate stream if one is requested through `HTMLCanvasElement::captureStream()` always returning variable framerate stream approximating the requested framerate complies with the spec. Ah, even better! Canvas captureStream runs off the refresh driver (so vsync, mostly), and I think if we got timestamps from the driver they'd be accurate for the most part. Of course if you overshoot *some* of your deadlines things might look odd, but we'd at least match the timestamps that were given to requestAnimationFrame().

Comment on attachment 8826163 [details] Bug 1231848 - Do not write framerate metadata to webm container in MediaRecorder. https://reviewboard.mozilla.org/r/104184/#review105104

Comment on attachment 8826164 [details] Bug 1231848 - Encode every frame as it comes in to VP8TrackEncoder. https://reviewboard.mozilla.org/r/104186/#review105108 ::: dom/media/encoder/VP8TrackEncoder.h:21 (Diff revision 1) > typedef struct vpx_image vpx_image_t; > > /** > * VP8TrackEncoder implements VideoTrackEncoder by using libvpx library. > - * We implement a realtime and fixed FPS encoder. In order to achieve that, > + * We implement a realtime and variable FPS encoder. In order to achieve that, > * there is a pick target frame and drop frame encoding policy implemented in Please remove the "pick target frame" words since we encode evey frames now.

Pushed by pehrsons@gmail.com: https://hg.mozilla.org/integration/autoland/rev/200e1510a9c1 Do not write framerate metadata to webm container in MediaRecorder. r=bechen,jesup,rillian https://hg.mozilla.org/integration/autoland/rev/c7bad8774250 Encode every frame as it comes in to VP8TrackEncoder. r=bechen,jesup

https://hg.mozilla.org/mozilla-central/rev/200e1510a9c1 https://hg.mozilla.org/mozilla-central/rev/c7bad8774250