Closed
Bug 1382433
Opened 7 years ago
Closed 7 years ago
Slow video in local preview window in Nightly on appear.in
Categories
(Core :: WebRTC, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: mjf, Assigned: mchiang)
References
Details
(Keywords: stale-bug, Whiteboard: regression)
Attachments
(2 files, 1 obsolete file)
(deleted),
text/x-review-board-request
|
jib
:
review+
|
Details |
(deleted),
patch
|
Details | Diff | Splinter Review |
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•7 years ago
|
||
User agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:56.0) Gecko/20100101 Firefox/56.0
Reporter | ||
Comment 2•7 years 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*
Comment 3•7 years ago
|
||
Dupe from bug 1376018 ?
Reporter | ||
Comment 4•7 years 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•7 years 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
Comment 8•7 years ago
|
||
Mass change P1->P2 to align with new Mozilla triage process
Priority: P1 → P2
Reporter | ||
Comment 9•7 years ago
|
||
I haven't seen this lately, so I guess we can call it gone.
Flags: needinfo?(mfroman)
Assignee | ||
Comment 10•7 years ago
|
||
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 | ||
Updated•7 years ago
|
Assignee: nobody → mchiang
Assignee | ||
Comment 12•7 years ago
|
||
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) |
Assignee | ||
Comment 14•7 years ago
|
||
I have confirmed chrome also use fixed frame rate by running this page https://jsfiddle.net/eo1ewsL7/ with MBP 2016 early model.
Comment 15•7 years 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?
Comment 16•7 years ago
|
||
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)
Assignee | ||
Comment 17•7 years ago
|
||
(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•7 years 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+
Comment 19•7 years ago
|
||
(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•7 years 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)
Assignee | ||
Comment 21•7 years ago
|
||
(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)
Assignee | ||
Comment 22•7 years ago
|
||
Attach the fps range log patch
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 23•7 years 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•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Comment 25•7 years ago
|
||
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)
Assignee | ||
Comment 26•7 years ago
|
||
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)
Comment 27•7 years ago
|
||
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.
Assignee | ||
Comment 28•7 years ago
|
||
(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.
Description
•