Slow video in local preview window in Nightly on appear.in

RESOLVED FIXED in Firefox 58

Status

()

P2
normal
Rank:
17
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: mjf, Assigned: mchiang)

Tracking

({stale-bug})

56 Branch
mozilla58
Unspecified
Mac OS X
stale-bug
Points:
---

Firefox Tracking Flags

(firefox58 fixed)

Details

(Whiteboard: regression)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
In appear.in calls (both 1:1 and 1:n) today I noticed my local preview window was super slow and smeared when I moved.  Others on the 1:n call noticed my video to them wasn't great either.  Using mozregression, I tracked it down to this:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=7e2c3de976857db485370c5fdecf70990216847b&tochange=c846596b4cb8cd51375621dd43fc75e0a887c61a
(Reporter)

Comment 1

a year ago
User agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:56.0) Gecko/20100101 Firefox/56.0
Depends on: 1374938
Whiteboard: regression
(Reporter)

Comment 2

a year ago
Interestingly, this morning after my laptop sat idle overnight (I did not log out, I did not restart, machine did not sleep) I can no longer reproduce this issue.  Yesterday it was reliably reproducible.  I tried multiple times (on AC power, on battery) and had no luck making it happen again today.  *sigh*
Dupe from bug 1376018 ?
(Reporter)

Comment 4

a year ago
Saw this again today.  It was with a fresh restart of Nightly.
Version 		57.0a1
Build ID 	20170809100326
User Agent 	Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:57.0) Gecko/20100101 Firefox/57.0
(Reporter)

Comment 5

a year ago
Saw this again today.  It was with a fresh restart of Nightly.
Version 		57.0a1
Build ID 	20170809100326
User Agent 	Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:57.0) Gecko/20100101 Firefox/57.0
This is a P1 bug without an assignee. 

P1 are bugs which are being worked on for the current release cycle/iteration/sprint. 

If the bug is not assigned by Monday, 28 August, the bug's priority will be reset to '--'.
Keywords: stale-bug
Still valid?
Rank: 17
Flags: needinfo?(mfroman)
Mass change P1->P2 to align with new Mozilla triage process
Priority: P1 → P2
(Reporter)

Comment 9

a year ago
I haven't seen this lately, so I guess we can call it gone.
Flags: needinfo?(mfroman)
We found a way to reproduce the bug.
steps:

1. Use MBP 2016 early model (without usb-c connector).
2. go to appear.in and join a room.
3. cover the camera completely. You will see a dark screen with some noise. (Actually you can found the fps is around ~1 fps by observing the noise)
4. uncover the camera. Now you will observe the bug.
Assignee: nobody → mchiang
Duplicate of this bug: 1401446
Currently we query supported AVFrameRateRange and choose the best one. [1]
Then we configure camera min & max fps according to the chosen AVFrameRateRange. [2]
MBP 2016 early model built-in camera only support one fps range (1-30) for each resolution.
So we set the min fps to 1.
When the camera is covered and the brightness is very low, the fps will reduce to almost 1 and cause this slow video issue when the camera is uncovered. The fps will increase gradually but it takes several (10+) seconds.

After reading the avfoundation document and doing some experiments, AVFrameRateRange expresses a range of valid frame rates as minimum and maximum rate. We can narrow down the range if necessary. So I would like to change the code to use fixed fps instead of floating fps to avoid very low fps. 

[1] http://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/media/webrtc/trunk/webrtc/modules/video_capture/mac/avfoundation/video_capture_avfoundation_objc.mm#129-133

[2] http://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/media/webrtc/trunk/webrtc/modules/video_capture/mac/avfoundation/video_capture_avfoundation_objc.mm#150-156
Comment hidden (mozreview-request)
I have confirmed chrome also use fixed frame rate by running this page https://jsfiddle.net/eo1ewsL7/ with MBP 2016 early model.

Comment 15

a year ago
mozreview-review
Comment on attachment 8912636 [details]
Bug 1382433 - use fixed fps instead of floating fps to avoid very low fps.

https://reviewboard.mozilla.org/r/183966/#review189520

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

I'm not sure I understand the fix. Code comments may help.

But even after reading comment 12 I'm not sure I understand the scope of the problem.

This fix doesn't appear to be solely for MBP 2016 early model. How will this affect other models?

How will this affect users using the frameRate constraint?
Also, are we sure the symptoms in comment 0 and comment 10 are the same?

Michael, can you confirm if you're seeing comment 10 ? Or are you by chance seeing bug 1397844 (attachment 8905608 [details])?
Flags: needinfo?(mfroman)
(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #15)
> I'm not sure I understand the fix. Code comments may help.
I will add more comments.
The basic idea is to set the same value to min & max fps to achieve fixed frame rate.

> 
> But even after reading comment 12 I'm not sure I understand the scope of the
> problem.
I recorded the symptom. Please refer to https://youtu.be/6n3fWZUlCMs
The problem is if we configure camera as variant fps between 1-30, when the brightness is very low, camera aec will low down the fps to ~1 in order to increase the exposure time. Meanwhile, aec rely on the new images' brightness as the reference to adjust the fps. ~1 fps means aec will only get a new image as the reference in one second. Obviously, mbp 2016 early model built-in camera aec is not very aggresive, it won't adjust frame rate & ex[osure setting until receiving several over-exposured samples.

> 
> This fix doesn't appear to be solely for MBP 2016 early model. How will this
> affect other models?
I checked MBP 2016 late model and Logitec c920 webcam, their supported ranges are reported as
{30, 30}
{15, 15}
{10, 10}

So for this kind of camera devices, we already set fixed fps to hardware.

> 
> How will this affect users using the frameRate constraint?

User will get more compatible behavior.
When we expose the hardware capabilities, we only expose maxFPS [1].
FitnessDistance calculate the distance according to the maxFPS [2].
If user set a minimum fps to 30, we will choose a capability with maxFPS=30 but actually we use variant fps {1-30} (for MBP 2016 early model only), which would violate the constraint.

[1] http://searchfox.org/mozilla-central/rev/f54c1723befe6bcc7229f005217d5c681128fcad/media/webrtc/trunk/webrtc/modules/video_capture/mac/avfoundation/video_capture_avfoundation_info_objc.mm#212

[2] http://searchfox.org/mozilla-central/rev/f54c1723befe6bcc7229f005217d5c681128fcad/dom/media/webrtc/MediaEngineCameraVideoSource.cpp#73

Comment 18

a year ago
mozreview-review
Comment on attachment 8912636 [details]
Bug 1382433 - use fixed fps instead of floating fps to avoid very low fps.

https://reviewboard.mozilla.org/r/183966/#review189724

r=me with comments, but see concerns in comment 19.
Attachment #8912636 - Flags: review?(jib) → review+
(In reply to Munro Mengjue Chiang [:mchiang] from comment #17)
> I recorded the symptom. Please refer to https://youtu.be/6n3fWZUlCMs

Thanks, that's different from bug 1397844.

> I checked MBP 2016 late model and Logitec c920 webcam, their supported
> ranges are reported as
> {30, 30}
> {15, 15}
> {10, 10}
>
> So for this kind of camera devices, we already set fixed fps to hardware.

Are you sure? How are you logging these ranges? How to they map to what constraints reveal?

I'm asking because I also have an MBP 2016 late model (MacBook Pro (15-inch, 2016) the one with touchbar) and a c920 and I see the following in MOZ_LOG (repeated for every resolution), even with your patch here built locally:

MediaManager Capability:  640 x  480 x 31 maxFps, MJPEG, Unknown codec. Distance = 32
MediaManager Capability:  640 x  480 x 25 maxFps, MJPEG, Unknown codec. Distance = 166
MediaManager Capability:  640 x  480 x 20 maxFps, MJPEG, Unknown codec. Distance = 333
MediaManager Capability:  640 x  480 x 16 maxFps, MJPEG, Unknown codec. Distance = 466
MediaManager Capability:  640 x  480 x 10 maxFps, MJPEG, Unknown codec. Distance = 666
MediaManager Capability:  640 x  480 x  8 maxFps, MJPEG, Unknown codec. Distance = 733
MediaManager Capability:  640 x  480 x  5 maxFps, MJPEG, Unknown codec. Distance = 833

I'm assuming MJPEG is the c920.

For my internal camera, I still see every frame-rate possible frame rate (even with your patch):

MediaManager Capability:  640 x  480 x 31 maxFps, UYVY, Unknown codec. Distance = 32
MediaManager Capability:  640 x  480 x 30 maxFps, UYVY, Unknown codec. Distance = 0
MediaManager Capability:  640 x  480 x 29 maxFps, UYVY, Unknown codec. Distance = 33
MediaManager Capability:  640 x  480 x 28 maxFps, UYVY, Unknown codec. Distance = 66
MediaManager Capability:  640 x  480 x 27 maxFps, UYVY, Unknown codec. Distance = 100
MediaManager Capability:  640 x  480 x 25 maxFps, UYVY, Unknown codec. Distance = 166
MediaManager Capability:  640 x  480 x 25 maxFps, UYVY, Unknown codec. Distance = 166
MediaManager Capability:  640 x  480 x 24 maxFps, UYVY, Unknown codec. Distance = 200
MediaManager Capability:  640 x  480 x 23 maxFps, UYVY, Unknown codec. Distance = 233
MediaManager Capability:  640 x  480 x 22 maxFps, UYVY, Unknown codec. Distance = 266
MediaManager Capability:  640 x  480 x 20 maxFps, UYVY, Unknown codec. Distance = 333
MediaManager Capability:  640 x  480 x 20 maxFps, UYVY, Unknown codec. Distance = 333
MediaManager Capability:  640 x  480 x 19 maxFps, UYVY, Unknown codec. Distance = 366
MediaManager Capability:  640 x  480 x 18 maxFps, UYVY, Unknown codec. Distance = 400
MediaManager Capability:  640 x  480 x 16 maxFps, UYVY, Unknown codec. Distance = 466
MediaManager Capability:  640 x  480 x 16 maxFps, UYVY, Unknown codec. Distance = 466
MediaManager Capability:  640 x  480 x 15 maxFps, UYVY, Unknown codec. Distance = 500
MediaManager Capability:  640 x  480 x 14 maxFps, UYVY, Unknown codec. Distance = 533
MediaManager Capability:  640 x  480 x 13 maxFps, UYVY, Unknown codec. Distance = 566
MediaManager Capability:  640 x  480 x 12 maxFps, UYVY, Unknown codec. Distance = 600
MediaManager Capability:  640 x  480 x 10 maxFps, UYVY, Unknown codec. Distance = 666
MediaManager Capability:  640 x  480 x 10 maxFps, UYVY, Unknown codec. Distance = 666
MediaManager Capability:  640 x  480 x  8 maxFps, UYVY, Unknown codec. Distance = 733
MediaManager Capability:  640 x  480 x  8 maxFps, UYVY, Unknown codec. Distance = 733
MediaManager Capability:  640 x  480 x  7 maxFps, UYVY, Unknown codec. Distance = 766
MediaManager Capability:  640 x  480 x  5 maxFps, UYVY, Unknown codec. Distance = 833
MediaManager Capability:  640 x  480 x  4 maxFps, UYVY, Unknown codec. Distance = 866
MediaManager Capability:  640 x  480 x  4 maxFps, UYVY, Unknown codec. Distance = 866
MediaManager Capability:  640 x  480 x  2 maxFps, UYVY, Unknown codec. Distance = 933
MediaManager Capability:  640 x  480 x  1 maxFps, UYVY, Unknown codec. Distance = 966

Does that match your expectations?
Flags: needinfo?(mchiang)
(Reporter)

Comment 20

a year ago
(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #16)
> Also, are we sure the symptoms in comment 0 and comment 10 are the same?
> 
> Michael, can you confirm if you're seeing comment 10 ? Or are you by chance
> seeing bug 1397844 (attachment 8905608 [details])?

Definitely haven't seen bug 1397844.  I can repro comment 10.  Mine is a 2015 MBP.
Flags: needinfo?(mfroman)
(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #19)
> Are you sure? How are you logging these ranges? How to they map to what
> constraints reveal?
> 
> I'm asking because I also have an MBP 2016 late model (MacBook Pro (15-inch,
> 2016) the one with touchbar) and a c920 and I see the following in MOZ_LOG
> (repeated for every resolution), even with your patch here built locally:
> 
> MediaManager Capability:  640 x  480 x 31 maxFps, MJPEG, Unknown codec.
> Distance = 32
> MediaManager Capability:  640 x  480 x 25 maxFps, MJPEG, Unknown codec.
> Distance = 166
> MediaManager Capability:  640 x  480 x 20 maxFps, MJPEG, Unknown codec.
> Distance = 333
> MediaManager Capability:  640 x  480 x 16 maxFps, MJPEG, Unknown codec.
> Distance = 466
> MediaManager Capability:  640 x  480 x 10 maxFps, MJPEG, Unknown codec.
> Distance = 666
> MediaManager Capability:  640 x  480 x  8 maxFps, MJPEG, Unknown codec.
> Distance = 733
> MediaManager Capability:  640 x  480 x  5 maxFps, MJPEG, Unknown codec.
> Distance = 833
> 
> I'm assuming MJPEG is the c920.
> 
> For my internal camera, I still see every frame-rate possible frame rate
> (even with your patch):
> 
> MediaManager Capability:  640 x  480 x 31 maxFps, UYVY, Unknown codec.
> Distance = 32
> MediaManager Capability:  640 x  480 x 30 maxFps, UYVY, Unknown codec.
> Distance = 0
> MediaManager Capability:  640 x  480 x 29 maxFps, UYVY, Unknown codec.
> Distance = 33
> MediaManager Capability:  640 x  480 x 28 maxFps, UYVY, Unknown codec.
> Distance = 66
> MediaManager Capability:  640 x  480 x 27 maxFps, UYVY, Unknown codec.
> Distance = 100
> MediaManager Capability:  640 x  480 x 25 maxFps, UYVY, Unknown codec.
> Distance = 166
> MediaManager Capability:  640 x  480 x 25 maxFps, UYVY, Unknown codec.
> Distance = 166
> MediaManager Capability:  640 x  480 x 24 maxFps, UYVY, Unknown codec.
> Distance = 200
> MediaManager Capability:  640 x  480 x 23 maxFps, UYVY, Unknown codec.
> Distance = 233
> MediaManager Capability:  640 x  480 x 22 maxFps, UYVY, Unknown codec.
> Distance = 266
> MediaManager Capability:  640 x  480 x 20 maxFps, UYVY, Unknown codec.
> Distance = 333
> MediaManager Capability:  640 x  480 x 20 maxFps, UYVY, Unknown codec.
> Distance = 333
> MediaManager Capability:  640 x  480 x 19 maxFps, UYVY, Unknown codec.
> Distance = 366
> MediaManager Capability:  640 x  480 x 18 maxFps, UYVY, Unknown codec.
> Distance = 400
> MediaManager Capability:  640 x  480 x 16 maxFps, UYVY, Unknown codec.
> Distance = 466
> MediaManager Capability:  640 x  480 x 16 maxFps, UYVY, Unknown codec.
> Distance = 466
> MediaManager Capability:  640 x  480 x 15 maxFps, UYVY, Unknown codec.
> Distance = 500
> MediaManager Capability:  640 x  480 x 14 maxFps, UYVY, Unknown codec.
> Distance = 533
> MediaManager Capability:  640 x  480 x 13 maxFps, UYVY, Unknown codec.
> Distance = 566
> MediaManager Capability:  640 x  480 x 12 maxFps, UYVY, Unknown codec.
> Distance = 600
> MediaManager Capability:  640 x  480 x 10 maxFps, UYVY, Unknown codec.
> Distance = 666
> MediaManager Capability:  640 x  480 x 10 maxFps, UYVY, Unknown codec.
> Distance = 666
> MediaManager Capability:  640 x  480 x  8 maxFps, UYVY, Unknown codec.
> Distance = 733
> MediaManager Capability:  640 x  480 x  8 maxFps, UYVY, Unknown codec.
> Distance = 733
> MediaManager Capability:  640 x  480 x  7 maxFps, UYVY, Unknown codec.
> Distance = 766
> MediaManager Capability:  640 x  480 x  5 maxFps, UYVY, Unknown codec.
> Distance = 833
> MediaManager Capability:  640 x  480 x  4 maxFps, UYVY, Unknown codec.
> Distance = 866
> MediaManager Capability:  640 x  480 x  4 maxFps, UYVY, Unknown codec.
> Distance = 866
> MediaManager Capability:  640 x  480 x  2 maxFps, UYVY, Unknown codec.
> Distance = 933
> MediaManager Capability:  640 x  480 x  1 maxFps, UYVY, Unknown codec.
> Distance = 966
> 
> Does that match your expectations?

Sorry for the confusion. I didn't list all entries.
Here is the complete supported frame rate for VGA.
It matches the MediaManger log.

MBP 2016 late model built-in camera

range: min: 30.000030, max: 30.000030, ceil(max): 31
range: min: 29.000049, max: 29.000049, ceil(max): 30
range: min: 28.000067, max: 28.000067, ceil(max): 29
range: min: 27.000027, max: 27.000027, ceil(max): 28
range: min: 26.000026, max: 26.000026, ceil(max): 27
range: min: 25.000000, max: 25.000000, ceil(max): 25
range: min: 24.000038, max: 24.000038, ceil(max): 25
range: min: 23.000032, max: 23.000032, ceil(max): 24
range: min: 22.000022, max: 22.000022, ceil(max): 23
range: min: 21.000021, max: 21.000021, ceil(max): 22
range: min: 20.000000, max: 20.000000, ceil(max): 20
range: min: 19.000029, max: 19.000029, ceil(max): 20
range: min: 18.000018, max: 18.000018, ceil(max): 19
range: min: 17.000009, max: 17.000009, ceil(max): 18
range: min: 16.000000, max: 16.000000, ceil(max): 16
range: min: 15.000015, max: 15.000015, ceil(max): 16
range: min: 14.000014, max: 14.000014, ceil(max): 15
range: min: 13.000013, max: 13.000013, ceil(max): 14
range: min: 12.000005, max: 12.000005, ceil(max): 13
range: min: 11.000011, max: 11.000011, ceil(max): 12
range: min: 10.000000, max: 10.000000, ceil(max): 10
range: min: 9.000001, max: 9.000001, ceil(max): 10
range: min: 8.000000, max: 8.000000, ceil(max): 8
range: min: 7.000002, max: 7.000002, ceil(max): 8
range: min: 6.000002, max: 6.000002, ceil(max): 7
range: min: 5.000000, max: 5.000000, ceil(max): 5
range: min: 4.000000, max: 4.000000, ceil(max): 4
range: min: 3.000000, max: 3.000000, ceil(max): 4
range: min: 2.000000, max: 2.000000, ceil(max): 2
range: min: 1.000000, max: 1.000000, ceil(max): 1

Logitec C920

range: min: 30.000030, max: 30.000030, ceil(max): 31
range: min: 24.000038, max: 24.000038, ceil(max): 25
range: min: 20.000000, max: 20.000000, ceil(max): 20
range: min: 15.000015, max: 15.000015, ceil(max): 16
range: min: 10.000000, max: 10.000000, ceil(max): 10
range: min: 7.500002, max: 7.500002, ceil(max): 8
range: min: 5.000000, max: 5.000000, ceil(max): 5
Flags: needinfo?(mchiang)
Created attachment 8913588 [details] [diff] [review]
print_fps.patch

Attach the fps range log patch
Keywords: checkin-needed

Comment 23

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3c4afc844a6a
use fixed fps instead of floating fps to avoid very low fps. r=jib
Keywords: checkin-needed

Comment 24

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3c4afc844a6a
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
FWIW it seems odd to me that we report 31 for a range of min: 30.000030, max: 30.000030. Is this some sort of common practice?
Flags: needinfo?(mchiang)
When we call startCapture() with the capability (maxFPS=31), setCaptureHeight() will try to match a range which's max fps, e.g., 30.000030, is the closest (but smaller than or equal) to the target, e.g., 31. [1] 

[1] http://searchfox.org/mozilla-central/rev/a4702203522745baff21e519035b6c946b7d710d/media/webrtc/trunk/webrtc/modules/video_capture/mac/avfoundation/video_capture_avfoundation_objc.mm#130
Flags: needinfo?(mchiang)
Seems like this could be solved in the implementation, e.g. round to 3 or 4 decimals here (e.g. add 0.0001 to the left of the > comparison).

Unless there's some side-effect I'm not seeing, I think I'd prefer rounding in the implementation over exposing ceiling values (31) to JS.
Created attachment 8915546 [details] [diff] [review]
rounding.patch

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #27)
> Seems like this could be solved in the implementation, e.g. round to 3 or 4
> decimals here (e.g. add 0.0001 to the left of the > comparison).
> 
> Unless there's some side-effect I'm not seeing, I think I'd prefer rounding
> in the implementation over exposing ceiling values (31) to JS.

Rounding is indeed better.
What about just rounding to integer?
Attachment #8913588 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.