Screen Video Capture: Implement suspend optimization.

Screen Video Capture: Implement suspend optimization.

Implements frame delivery suspend/resume functionality in tab capture,
Aura window/desktop capture, and Android screen capture. This allows the
screen capture devices to minimize resource utilization while there are
no consumers of the video frames attached downstream.

Prerequisite change: https://codereview.chromium.org/2365223002/

BUG=650113
Committed: https://crrev.com/a84c86dcab6fda82917b411bf0e47a9d5c2f5ee2
Cr-Commit-Position: refs/heads/master@{#421763}

[miu, 2016-09-25 23:42:05]

miu@chromium.org changed reviewers:
+ braveyao@chromium.org, xjz@chromium.org

[miu, 2016-09-25 23:42:06]

braveyao: PTAL at media/capture/content/android/screen_capture_machine_android.*

xjz: PTAL the rest.

[braveyao, 2016-09-26 17:14:47]

The suspend optimization won't have effect on Android, right?

https://codereview.chromium.org/2364413002/diff/1/media/capture/content/andro...
File media/capture/content/android/screen_capture_machine_android.h (right):

https://codereview.chromium.org/2364413002/diff/1/media/capture/content/andro...
media/capture/content/android/screen_capture_machine_android.h:59: void Resume()
override;
No definition to these two functions?

https://codereview.chromium.org/2364413002/diff/1/media/capture/content/andro...
media/capture/content/android/screen_capture_machine_android.h:74: bool
suspend_frame_capture_;
Due to above comments, this variable is not used at all.

https://codereview.chromium.org/2364413002/diff/1/media/capture/content/scree...
File media/capture/content/screen_capture_device_core.cc (right):

https://codereview.chromium.org/2364413002/diff/1/media/capture/content/scree...
media/capture/content/screen_capture_device_core.cc:142: static constexpr char*
kStateNames[] = {"Idle", "Capturing", "Suspended",
Compiling failure for Android:
ISO C++11 does not allow conversion from string literal to 'char *const'
[-Werror,-Wwritable-strings]

[xjz, 2016-09-27 05:08:57]

The rest lgtm. :)

[miu, 2016-09-27 20:46:51]

braveyao: PTAL.

https://codereview.chromium.org/2364413002/diff/1/media/capture/content/andro...
File media/capture/content/android/screen_capture_machine_android.h (right):

https://codereview.chromium.org/2364413002/diff/1/media/capture/content/andro...
media/capture/content/android/screen_capture_machine_android.h:59: void Resume()
override;
On 2016/09/26 17:14:47, braveyao wrote:
> No definition to these two functions? 

Oh no! Sorry, I missed that. Please see most recent patch set.

https://codereview.chromium.org/2364413002/diff/1/media/capture/content/andro...
media/capture/content/android/screen_capture_machine_android.h:74: bool
suspend_frame_capture_;
On 2016/09/26 17:14:46, braveyao wrote:
> Due to above comments, this variable is not used at all.

It is now. ;)

https://codereview.chromium.org/2364413002/diff/1/media/capture/content/scree...
File media/capture/content/screen_capture_device_core.cc (right):

https://codereview.chromium.org/2364413002/diff/1/media/capture/content/scree...
media/capture/content/screen_capture_device_core.cc:142: static constexpr char*
kStateNames[] = {"Idle", "Capturing", "Suspended",
On 2016/09/26 17:14:47, braveyao wrote:
> Compiling failure for Android:
> ISO C++11 does not allow conversion from string literal to 'char *const'
> [-Werror,-Wwritable-strings]

Fixed.

[braveyao, 2016-09-27 23:33:56]

Hi Miu,

The patch is good. But I still don't think we should suspend screen capture on
Android.

I see that the suspending starts from RenderViewImpl::OnPageWasHidden(). 
On Android, switching tab or putting chrome into background will trigger that.
But these two are both valid cases for screen sharing, i.e. user will want to
share the desktop by minimizing chrome. So I suppose Suspend()/Resume() should
be non-op on Android.

[miu, 2016-09-28 00:09:21]

On 2016/09/27 23:33:56, braveyao wrote:
> Hi Miu,
> 
> The patch is good. But I still don't think we should suspend screen capture on
> Android.
> 
> I see that the suspending starts from RenderViewImpl::OnPageWasHidden(). 
> On Android, switching tab or putting chrome into background will trigger that.
> But these two are both valid cases for screen sharing, i.e. user will want to
> share the desktop by minimizing chrome. So I suppose Suspend()/Resume() should
> be non-op on Android.

Okay, I've removed the Android screen capture changes. However, I encourage you
to follow-up at some point in the future, since power usage is an even bigger
deal on mobile. ;-)

The suspending will only occur on the page that *receives* the video frames
(which is often *not* the captured tab). To make sure I'm not breaking anything,
PTAL at the changes I'm making to VideoCaptureManager in the other change
(https://codereview.chromium.org/2365223002/diff/1/content/browser/renderer_ho...)

[braveyao, 2016-09-28 00:21:18]

lgtm.

Screen capture on Android won't go through controller any more. So it should be
OK.

And yes, if there is new request to Android, I'll follow it up :)

[miu, 2016-09-29 04:23:23]

The CQ bit was checked by miu@chromium.org

[miu, 2016-09-29 04:23:23]

The patchset sent to the CQ was uploaded after l-g-t-m from xjz@chromium.org
Link to the patchset: https://codereview.chromium.org/2364413002/#ps40001
(title: "Unwind ScreenCaptureMachineAndroid changes.")

[commit-bot: I haz the power, 2016-09-29 04:23:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 06:42:54]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-29 06:44:54]

Description was changed from

==========
Screen Video Capture: Implement suspend optimization.

Implements frame delivery suspend/resume functionality in tab capture,
Aura window/desktop capture, and Android screen capture. This allows the
screen capture devices to minimize resource utilization while there are
no consumers of the video frames attached downstream.

Prerequisite change: https://codereview.chromium.org/2365223002/

BUG=650113
==========

to

==========
Screen Video Capture: Implement suspend optimization.

Implements frame delivery suspend/resume functionality in tab capture,
Aura window/desktop capture, and Android screen capture. This allows the
screen capture devices to minimize resource utilization while there are
no consumers of the video frames attached downstream.

Prerequisite change: https://codereview.chromium.org/2365223002/

BUG=650113

Committed: https://crrev.com/a84c86dcab6fda82917b411bf0e47a9d5c2f5ee2
Cr-Commit-Position: refs/heads/master@{#421763}
==========

[commit-bot: I haz the power, 2016-09-29 06:44:56]

Patchset 3 (id:??) landed as
https://crrev.com/a84c86dcab6fda82917b411bf0e47a9d5c2f5ee2
Cr-Commit-Position: refs/heads/master@{#421763}