Closed Bug 1363259 Opened 4 years ago Closed 4 years ago

getusermedia constraint frameRate doesn't take effect

Categories

(Core :: WebRTC: Audio/Video, enhancement, P1)

Unspecified
macOS
enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: mchiang, Assigned: mchiang)

Details

Attachments

(1 file)

No description provided.
Rank: 15
Priority: -- → P1
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
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.
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
https://hg.mozilla.org/mozilla-central/rev/12855beedd3e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.