[Blocking Requested - why for this release]: Disable the motion/spatial analysis on b2g (simply ifdef out the call), and hard-code the results to be "normal" for both (in a range of "low/normal/high"). (perhaps "high" for motion to create a preference for framerate over resolution). This will let the qm_select code operate normally. Inheriting the blocking request from bug 972639 (we'll want some verification it works and does in fact lower CPU use (or increase frame rate)).

Comment on attachment 8481128 [details] [diff] [review] disable frame motion/complexity analysis in webrtc on Gonk Review of attachment 8481128 [details] [diff] [review]: ----------------------------------------------------------------- Looks good but I don't see you *actually disabling* the analysis. http://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/video_engine/vie_encoder.cc#210 http://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/video_engine/vie_encoder.cc#291 Also note this: http://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/video_processing/main/source/frame_preprocessor.cc#131 It's currently set to 2. Maybe we can just bump that up.

Comment on attachment 8481249 [details] [diff] [review] disable frame motion/complexity analysis in webrtc on Gonk Thanks - I also disabled allocation of the unneeded prev_frame_ buffer

(In reply to Gian-Carlo Pascutto [:gcp] from comment #2) > Comment on attachment 8481128 [details] [diff] [review] > disable frame motion/complexity analysis in webrtc on Gonk > > Review of attachment 8481128 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks good but I don't see you *actually disabling* the analysis. > > http://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/ > video_engine/vie_encoder.cc#210 > http://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/ > video_engine/vie_encoder.cc#291 > > Also note this: > > http://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/ > modules/video_processing/main/source/frame_preprocessor.cc#131 > > It's currently set to 2. Maybe we can just bump that up. We could, but: 'motion' would be measured as unnaturally high, and it would still pay the memory and CPU cost for copying the frames (though at 1/Nth the cost). That is a viable alternative - bump it from 2 to say 20 should cut the CPU use to <1% (very likely <0.5%). Comments? of course, that patch is effectively a 1-liner-ifdef

Comment on attachment 8481249 [details] [diff] [review] disable frame motion/complexity analysis in webrtc on Gonk Review of attachment 8481249 [details] [diff] [review]: ----------------------------------------------------------------- Looks OK. How do you feel about just nuking this: http://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/video_coding/main/source/media_optimization.cc#438 then in VCMContentMetricsProcessing::UpdateContentData handle the ContentMetrics == NULL case with the "average values" you added in this patch, and modifying the enableContentAnalysis calls I pointed out in the previous review? I think that's slightly cleaner than this, but I can live with it, too.

(In reply to Randell Jesup [:jesup] from comment #6) > We could, but: 'motion' would be measured as unnaturally high, and it would > still pay the memory and CPU cost for copying the frames (though at 1/Nth > the cost). That is a viable alternative - bump it from 2 to say 20 should > cut the CPU use to <1% (very likely <0.5%). Comments? of course, that > patch is effectively a 1-liner-ifdef I sortof agree that if we plan to get rid of this anyway, then keeping the analysis enabled but in a way that we don't even know if it returns sensible results doesn't do us much good.

Jay, can you please confirm that this patch helps w/ CPU load to help turn the v2.0? into a +

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/2b60f30db362 for something in that push causing b2g build bustage like https://tbpl.mozilla.org/php/getParsedLog.php?id=47078117&tree=Mozilla-Inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/d07a9cabc185 This was the cause of the bustage; I didn't remove a bit that was no longer needed

jay - use the final landed patch (probably with the patch from the other bug as well to pass contentMetrics to h.264 AddVideoFrame). The gmp patch you don't need, and won't apply to 2.0.

(In reply to Michael Vines [:m1] [:evilmachines] from comment #10) > Jay, can you please confirm that this patch helps w/ CPU load to help turn > the v2.0? into a + Just check the patch https://hg.mozilla.org/mozilla-central/rev/d07a9cabc185 and https://hg.mozilla.org/mozilla-central/rev/f8055a6d7d10. The CPU utilization improved by 2.56%.

(In reply to Jay from comment #15) > (In reply to Michael Vines [:m1] [:evilmachines] from comment #10) > > Jay, can you please confirm that this patch helps w/ CPU load to help turn > > the v2.0? into a + > > Just check the patch https://hg.mozilla.org/mozilla-central/rev/d07a9cabc185 > and https://hg.mozilla.org/mozilla-central/rev/f8055a6d7d10. The CPU > utilization improved by 2.56%. Thanks; that seems to be what I would expect.

Note that due to recent policy changes, all B2G uplifts needs approval now regardless of blocking status. Please request b2g32 approval on this patch when you get a chance. Sorry for the inconvenience.

Comment on attachment 8481249 [details] [diff] [review] disable frame motion/complexity analysis in webrtc on Gonk NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. (Missed setting the patch b2g32? flag when the other blockers were resolved) [Approval Request Comment] Bug caused by (feature/regressing bug #): n/a User impact if declined: Significant extra CPU (and some memory) use on B2G Testing completed: On nightly and 34 for some time. Tested by myself and by QC; 2.5+% CPU reduction. Risk to taking this patch (and alternatives if risky): Very low risk. Avoids calling expensive code. String or UUID changes made by this patch: none