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

Specify what happens when frameRate is omitted #27

Closed
foolip opened this issue Feb 18, 2016 · 10 comments
Closed

Specify what happens when frameRate is omitted #27

foolip opened this issue Feb 18, 2016 · 10 comments

Comments

@foolip
Copy link
Member

foolip commented Feb 18, 2016

http://w3c.github.io/mediacapture-fromelement/#widl-HTMLCanvasElement-captureStream-MediaStream-double-frameRate doesn't say but the spec used to say "If the frameRate value is omitted, the user agent should capture new frames each time that the content of the canvas changes."

What Blink does is to treat it as 60, which doesn't seem right.

@foolip
Copy link
Member Author

foolip commented Feb 18, 2016

The problem with 60 is that if the update rate is close to 60, then jitter ought to cause frames to be dropped. Capturing all frames, or at least every frame that the user ends up seeing, would make more sense.

@martinthomson
Copy link
Member

Firefox captures changed frames as they are displayed. That means that we get something close to the frame rate unless no changes are made.

What change are you suggesting? I think that the Firefox implementation is good. It took a fair bit of work to get right, but it is quite a bit more efficient in the end.

@foolip
Copy link
Member Author

foolip commented Feb 19, 2016

I think what Firefox does sounds good, and that the spec should unambiguously require this.

What is the event that causes Firefox to capture a frame? In Blink it's a HTMLCanvasElement::notifyListenersCanvasChanged that's called after the canvas has been updated, although I'm not sure of the exact timing.

In spec terms, it probably has to be something involving the output bitmap. Happy to add whatever hooks are needed to HTML to make this work properly.

@uysalere
Copy link
Contributor

Right now, Blink also captures changed frames as they are displayed as well. I think Blink side definition of kDefaultFrameRate=60 is causing some misunderstanding here.

Chrome-side implementation of VideoCapturerSource interface requires each video source to report the formats-including framerate- that can be used. Then, it goes through the given formats to decide which one to use and starts capture with its decided format. Since doesn't have a set frame rate in captureStream() case, I decided to report 60 as the default value. It is purely a Chrome side artifact that shouldn't affect the canvas capture. I am not sure if Firefox has any requirements to report any frame rate, or if it is worth mentioning it in the spec.

@foolip
Copy link
Member Author

foolip commented Feb 23, 2016

So, kDefaultFrameRate=60 will not affect how often frames are captured nor be observable to web contents in any other way? How about if the MediaStream is piped to MediaStreamRecorder, will the frame rate show up in the resulting file? That's a relatively unimportant detail, just trying to understand what this is, and that there is no potential for dropped frames due to this.

@uysalere
Copy link
Contributor

kDefaultFrameRate will not affect how often frames are captured. For captureStream() case, we will capture each frame whenever canvas is updated; fps can be 1, 80, etc...

In the case where the MediaStream is piped to MediaStreamRecorder, kDefaultFrameRate will be used in the WebM header. However, it won't affect the actual captured number of frames. There will be as many frames recorded as the number of frames coming from the MediaStream.

@foolip
Copy link
Member Author

foolip commented Feb 24, 2016

Would it be possible to simply omit the FrameRate header for MediaStreamRecorder, regardless of the input, or at the very least for cases like these? https://www.webmproject.org/docs/container/ says that the field is deprecated and informational only.

Whatever is the case, the spec should probably say something about interaction with MediaStreamRecorder when it comes to framerate, but that's low priority compared to the other open issues.

@uysalere
Copy link
Contributor

Thanks for pointing out. I made a bug for it.

@foolip
Copy link
Member Author

foolip commented Feb 25, 2016

Thanks @uysalere! What remains of this issue is similar to #29, namely to define precisely what "the canvas is painted" means, probably with a hook in HTML.

@martinthomson
Copy link
Member

OK, let's move to addressing #29.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants