getusermedia constraint frameRate doesn't take effect

RESOLVED FIXED in Firefox 55

Status

()

Core
WebRTC: Audio/Video
P1
normal
Rank:
15
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: mchiang, Assigned: mchiang)

Tracking

unspecified
mozilla55
Unspecified
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
Comment hidden (mozreview-request)
Rank: 15
Priority: -- → P1

Comment 2

a year ago
mozreview-review
Comment on attachment 8865728 [details]
Bug 1363259 - set min and max fps through AVCaptureConnection.

https://reviewboard.mozilla.org/r/137350/#review140870

Lgtm, but could you add a description of the problem and steps to reproduce it? E.g. do you have a fiddle or case where I can observe a difference before and after this patch?

::: media/webrtc/trunk/webrtc/modules/video_capture/mac/avfoundation/video_capture_avfoundation_objc.mm:150
(Diff revision 1)
> -      [_captureDevice unlockForConfiguration];
> +    if ([captureConnection isVideoMinFrameDurationSupported]) {
> +      [captureConnection setVideoMinFrameDuration:bestFrameRateRange.minFrameDuration];
> +    }

I don't know this API at all, so I'm only raising questions in case it helps you think about things.

What happens if setVideoMinFrameDuration is not supported?

Did the old code not do anything? Are there cases where isVideoMinFrameDurationSupported is false and the old code would be useful, and we should fall back to it?
Attachment #8865728 - Flags: review?(jib) → review+
Assignee: nobody → mchiang
(Assignee)

Comment 3

a year ago
mozreview-review-reply
Comment on attachment 8865728 [details]
Bug 1363259 - set min and max fps through AVCaptureConnection.

https://reviewboard.mozilla.org/r/137350/#review140870

Sure, please try the fiddle https://jsfiddle.net/x42cr1gp/

> I don't know this API at all, so I'm only raising questions in case it helps you think about things.
> 
> What happens if setVideoMinFrameDuration is not supported?
> 
> Did the old code not do anything? Are there cases where isVideoMinFrameDurationSupported is false and the old code would be useful, and we should fall back to it?

The old code is based on this api
https://developer.apple.com/reference/avfoundation/avcapturedevice/1389290-activevideominframeduration?language=objc
The new code is based on this api
https://developer.apple.com/reference/avfoundation/avcaptureconnection/1388931-videominframeduration?language=objc

From the spec, suppose two apis should both work, but only the new code actually works on my two MBP laptops (2013 and 2016 model)
I also refer to Chromium's implementation. They use the new code to set the framerate.
Keywords: checkin-needed

Comment 4

a year ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/12855beedd3e
set min and max fps through AVCaptureConnection. r=jib
Keywords: checkin-needed

Comment 5

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/12855beedd3e
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.