Closed
Bug 1060796
Opened 10 years ago
Closed 10 years ago
Limit screen capture FPS
Categories
(Core :: WebRTC: Audio/Video, defect)
Core
WebRTC: Audio/Video
Tracking
()
VERIFIED
FIXED
mozilla35
Tracking | Status | |
---|---|---|
firefox32 | --- | unaffected |
firefox33 | --- | verified |
firefox34 | --- | verified |
firefox35 | --- | verified |
People
(Reporter: jesup, Assigned: gcp)
References
()
Details
Attachments
(1 file, 2 obsolete files)
3.90 KB,
patch
|
jesup
:
review+
lmandel
:
approval-mozilla-aurora+
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•10 years ago
|
||
Gian-Carlo, do you have the patches you did while investigating the flickering?
Flags: needinfo?(gpascutto)
Comment 2•10 years ago
|
||
Gian-Carlo, does this fit with what you've been working?
Assignee | ||
Comment 4•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8482752 -
Flags: review?(linuxwolf)
Assignee | ||
Updated•10 years ago
|
Attachment #8481811 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8481836 -
Attachment is obsolete: true
Assignee | ||
Comment 5•10 years ago
|
||
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)
Reporter | ||
Comment 6•10 years ago
|
||
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/desktop_capture_impl.cc
@@ +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+
Assignee | ||
Comment 7•10 years ago
|
||
Assignee | ||
Comment 8•10 years ago
|
||
Now with more qref (on the try patch, sigh):
https://tbpl.mozilla.org/?tree=Try&rev=1eb1e5637d32
Assignee | ||
Comment 9•10 years ago
|
||
Comment 10•10 years ago
|
||
Assignee: nobody → gpascutto
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Assignee | ||
Comment 11•10 years ago
|
||
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 12•10 years ago
|
||
Nevermind, the right thing did land, yay.
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•10 years ago
|
||
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?
Updated•10 years ago
|
status-firefox32:
--- → unaffected
status-firefox33:
--- → affected
status-firefox34:
--- → affected
status-firefox35:
--- → fixed
Comment 14•10 years ago
|
||
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+
Comment 15•10 years ago
|
||
Updated•10 years ago
|
Flags: qe-verify+
Comment 16•10 years ago
|
||
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)
Assignee | ||
Comment 17•10 years ago
|
||
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)
Comment 18•10 years ago
|
||
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%
Status: RESOLVED → VERIFIED
QA Contact: petruta.rasa
Reporter | ||
Updated•10 years ago
|
Whiteboard: [screensharing-uplift]
You need to log in
before you can comment on or make changes to this bug.
Description
•