Closed Bug 1060796 Opened 10 years ago Closed 10 years ago

Limit screen capture FPS


(Core :: WebRTC: Audio/Video, defect)

Not set



Tracking Status
firefox32 --- unaffected
firefox33 --- verified
firefox34 --- verified
firefox35 --- verified


(Reporter: jesup, Assigned: gcp)





(1 file, 2 obsolete files)

gcp identified that the capture code was i-looping and capturing as fast as it could while investigating bug 1053264.

We should limit the capture rate to max 30fps.  In the future, we may want to limit it or make it controllable either at getUserMedia time, or via RTPSender.
Gian-Carlo, do you have the patches you did while investigating the flickering?
Flags: needinfo?(gpascutto)
Attached patch Limit screen capture FPS (obsolete) — Splinter Review
Gian-Carlo, does this fit with what you've been working?
Attached patch Patch 1. limit-capture-rate (obsolete) — Splinter Review
Not really.
Flags: needinfo?(gpascutto)
Attachment #8482752 - Flags: review?(linuxwolf)
Attachment #8481811 - Attachment is obsolete: true
Attachment #8481836 - Attachment is obsolete: true
Comment on attachment 8482752 [details] [diff] [review]
Limit screen capture FPS

Review of attachment 8482752 [details] [diff] [review]:

Matthew is apparently working on something else, shifting reviewer.
Attachment #8482752 - Flags: review?(linuxwolf) → review?(rjesup)
Comment on attachment 8482752 [details] [diff] [review]
Limit screen capture FPS

Review of attachment 8482752 [details] [diff] [review]:

Note, this will likely have some level of merge conflict with jimm's patches for windows, which will likely need to use a different process()/eventloop

::: media/webrtc/trunk/webrtc/video_engine/
@@ +801,5 @@
>    desktop_capturer_cursor_composer_->Capture(DesktopRegion());
> +  const uint32_t processTime =
> +      (uint32_t)(TickTime::Now() - startProcessTime).Milliseconds();
> +  // Use at most 50% CPU or limit framerate, 60 fps = 16ms
> +  time_event_.Wait(std::max<uint32_t>(16, processTime));

We create the channel with a max FPS of 30, so any frames beyond that will be thrown away anyways.

replace 16 with 1000/_requestedCapability.maxFPS, and consider hoisting the 30 there into a #define (for now); we can look at making it configurable in some manner by the app in the future.

We will also want to teach the content analysis stuff that screen/window captures should not be downrezed unless the frame rate gets really low, but that's a separate bug.
Attachment #8482752 - Flags: review?(rjesup) → review+
Depends on: 1060738
Now with more qref (on the try patch, sigh):
Assignee: nobody → gpascutto
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
This landed but I forgot to qref the patch with the latest changes, as a result it doesn't address all review comments. I will fix this here.
Resolution: FIXED → ---
Nevermind, the right thing did land, yay.
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Comment on attachment 8482752 [details] [diff] [review]
Limit screen capture FPS

Approval Request Comment
[Feature/regressing bug #]: Bug 983504 - Basic WebRTC screensharing support
[User impact if declined]: High CPU usage during screensharing
[Describe test coverage new/current, TBPL]: Landed on m-c a week ago
[Risks and why]: At worst it does funny things to the frames per second.
[String/UUID change made/needed]: N/A
Attachment #8482752 - Flags: approval-mozilla-beta?
Attachment #8482752 - Flags: approval-mozilla-aurora?
Comment on attachment 8482752 [details] [diff] [review]
Limit screen capture FPS

Beta+ and Aurora+
Attachment #8482752 - Flags: approval-mozilla-beta?
Attachment #8482752 - Flags: approval-mozilla-beta+
Attachment #8482752 - Flags: approval-mozilla-aurora?
Attachment #8482752 - Flags: approval-mozilla-aurora+
Flags: qe-verify+
Using Nightly 34.0a1 2014-08-30 under Win 7 64-bit I had about 200Mb memory and under 15% CPU usage while performing screen sharing on the test page.
The CPU rate seems lower with Firefox 33 beta 3.

Is there a tool I could use to measure FPS rate in order to verify this bug is fixed? Or how else should I proceed? Thanks
Flags: needinfo?(gpascutto)
I don't think you can externally observe the capture rate, given that IIRC the encoder would encode at 30fps anyway.

CPU usage in Windows is for all processors, so for example if you have 4 CPU cores, then "100% of 1 cpu" would show as "25%" (100/4) in Windows. We limit to 50% of the CPU that is capturing, so your number sound about right for a quad-core system. (100/4/2 = 12.5%)
Flags: needinfo?(gpascutto)
Thanks gcp!

Tested and observed the CPU rates using Firefox 33 beta 4, latest Aurora 34.0a2 (20140915004005) and latest Nightly 35.0a1 (20140915030204) under Win 7 64-bit (4 CPU cores), Ubuntu 12.10 32-bit (3 CPU) and Mac OSX 10.8.5 (4 CPU). 

On those versions the CPU rate is significantly reduced as against Nightly builds before the fix:
Win 7: 9-11% vs 15%
Ubuntu: 33-39% vs 51-60%
Mac OSX: 25-28% vs 80-121%
QA Contact: petruta.rasa
Whiteboard: [screensharing-uplift]
You need to log in before you can comment on or make changes to this bug.