RESOURCE_UTILIZATION in VideoFrameMetadata, a consumer feedback signal.

RESOURCE_UTILIZATION in VideoFrameMetadata, a consumer feedback signal.

Adds a RESOURCE_UTILIZATION to media::VideoFrameMetadata which will allow
consumers to convey a "throttling feedback signal" to the producers of
media::VideoFrames.  Plumbs-in browser-side and renderer-side passing of
the metadata value across the IPC layer.

Also in this change, the media::cast::VideoSender sets the
RESOURCE_UTILIZATION metadata value to indicate CPU encode and network
bandwidth bottlenecks in the end-to-end system (sending a video stream
to a remote receiver).  An upcoming change will allow the screen capture
pipeline to use this feedback to automatically adjust resolution and
framerate during a session, maintaining the highest-quality user
experience possible in the current environment.  See bug for more
details.

BUG=156767
Committed: https://crrev.com/5dface5ccc2e0662365ce127987348cc2d04819b
Cr-Commit-Position: refs/heads/master@{#333313}

[miu, 2015-05-28 19:07:34]

Patchset #1 (id:1) has been deleted

[miu, 2015-05-28 19:09:24]

miu@chromium.org changed reviewers:
+ hubbe@chromium.org

[miu, 2015-05-28 19:09:25]

hubbe: Could you take the first look before I loop in more people?  The main
"points of interest" are media/base/video_frame_metadata.h,
content/renderer/media/video_capture_impl.cc and
content/browser/media/capture/content_video_capture_device_core.cc.

[hubbe, 2015-06-02 00:23:22]

Initial set of comments.

https://codereview.chromium.org/1157193002/diff/20001/content/renderer/media/...
File content/renderer/media/video_capture_impl.cc (right):

https://codereview.chromium.org/1157193002/diff/20001/content/renderer/media/...
content/renderer/media/video_capture_impl.cc:27: // This is called on an unknown
thread when the VideoFrame destructor executes.
This explains what happens, but not why.
I think somewhere we need to explain why there are two callbacks per frame.

https://codereview.chromium.org/1157193002/diff/20001/media/base/video_frame_...
File media/base/video_frame_metadata.h (right):

https://codereview.chromium.org/1157193002/diff/20001/media/base/video_frame_...
media/base/video_frame_metadata.h:60: STRESS_LEVEL,
I don't particularly like this name.
How about RESOURCE_UTILIZATION_FRACTION?
DOWNSTREAM_BOTTLENECK_PERCENTAGE?
MAX_PIXELS_PER_SECOND_FRACTION?
PPS_PERCENTAGE?
RESOLUTION_DIVIDER?

Ok, so some of these ideas are bad, I just wanted to provide some ideas.
My preferred suggest would probably be "RESOURCE_UTILIZATION".

https://codereview.chromium.org/1157193002/diff/20001/media/cast/sender/video...
File media/cast/sender/video_sender.cc (right):

https://codereview.chromium.org/1157193002/diff/20001/media/cast/sender/video...
media/cast/sender/video_sender.cc:262: // frames.
I really wish we could include key frames too.
Not sure how to do that in a *good* way though.

[miu, 2015-06-02 03:07:34]

https://codereview.chromium.org/1157193002/diff/20001/content/renderer/media/...
File content/renderer/media/video_capture_impl.cc (right):

https://codereview.chromium.org/1157193002/diff/20001/content/renderer/media/...
content/renderer/media/video_capture_impl.cc:27: // This is called on an unknown
thread when the VideoFrame destructor executes.
On 2015/06/02 00:23:22, hubbe wrote:
> This explains what happens, but not why.
> I think somewhere we need to explain why there are two callbacks per frame.

Done.  Basically, it's the only interface.  I don't like it, but fixing it would
be outside the scope of this CL.

https://codereview.chromium.org/1157193002/diff/20001/media/base/video_frame_...
File media/base/video_frame_metadata.h (right):

https://codereview.chromium.org/1157193002/diff/20001/media/base/video_frame_...
media/base/video_frame_metadata.h:60: STRESS_LEVEL,
On 2015/06/02 00:23:22, hubbe wrote:
> I don't particularly like this name.
> ...
> My preferred suggest would probably be "RESOURCE_UTILIZATION".

Done.  This is actually what I had in my working prototype, so sgtm.

https://codereview.chromium.org/1157193002/diff/20001/media/cast/sender/video...
File media/cast/sender/video_sender.cc (right):

https://codereview.chromium.org/1157193002/diff/20001/media/cast/sender/video...
media/cast/sender/video_sender.cc:262: // frames.
On 2015/06/02 00:23:22, hubbe wrote:
> I really wish we could include key frames too.
> Not sure how to do that in a *good* way though.

Done.  I thought about this for awhile, and there really isn't a good way. 
Instead of providing nothing, I've explicitly capped the value to 1.0 to avoid
throwing things off.

[hubbe, 2015-06-04 22:08:54]

LGTM + nits

https://codereview.chromium.org/1157193002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

https://codereview.chromium.org/1157193002/diff/40001/content/browser/rendere...
content/browser/renderer_host/media/video_capture_controller.cc:253: // Set the
RESOURCE_UTILIZATION to the maximum of those provided by each
This comment is confusing me a bit as it seems to suggest that there are
multiple consumers of the frame...

There is definitely two different sources of resource_utilization here though,
perhaps the comment can clarify where they come from?

https://codereview.chromium.org/1157193002/diff/40001/media/base/video_frame_...
File media/base/video_frame_metadata.h (right):

https://codereview.chromium.org/1157193002/diff/40001/media/base/video_frame_...
media/base/video_frame_metadata.h:53: // immediately.
I would remove the word "immediately" here.

https://codereview.chromium.org/1157193002/diff/40001/media/cast/sender/video...
File media/cast/sender/video_sender.cc (right):

https://codereview.chromium.org/1157193002/diff/40001/media/cast/sender/video...
media/cast/sender/video_sender.cc:264: static const double kAttenuationFactor =
0.75;
Move to top of file?

[miu, 2015-06-06 00:29:22]

https://codereview.chromium.org/1157193002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

https://codereview.chromium.org/1157193002/diff/40001/content/browser/rendere...
content/browser/renderer_host/media/video_capture_controller.cc:253: // Set the
RESOURCE_UTILIZATION to the maximum of those provided by each
On 2015/06/04 22:08:53, hubbe wrote:
> This comment is confusing me a bit as it seems to suggest that there are
> multiple consumers of the frame...

There are!  :)  Throughout the VideoCaptureController, this is a known part of
the design.

Clarified the comment, in that multiple calls of the method are coming from
different consumers that all reference the same VideoFrame.

https://codereview.chromium.org/1157193002/diff/40001/media/base/video_frame_...
File media/base/video_frame_metadata.h (right):

https://codereview.chromium.org/1157193002/diff/40001/media/base/video_frame_...
media/base/video_frame_metadata.h:53: // immediately.
On 2015/06/04 22:08:53, hubbe wrote:
> I would remove the word "immediately" here.

Done.

https://codereview.chromium.org/1157193002/diff/40001/media/cast/sender/video...
File media/cast/sender/video_sender.cc (right):

https://codereview.chromium.org/1157193002/diff/40001/media/cast/sender/video...
media/cast/sender/video_sender.cc:264: static const double kAttenuationFactor =
0.75;
On 2015/06/04 22:08:53, hubbe wrote:
> Move to top of file?

Done.

[miu, 2015-06-06 00:30:54]

miu@chromium.org changed reviewers:
+ dalecurtis@chromium.org, kenrb@chromium.org

[miu, 2015-06-06 00:30:55]

dalecurtis: OWNERS stamp of approval.

kenrb: Security review of IPC changes in
content/common/media/video_capture_messages.h.

[kenrb, 2015-06-08 17:11:00]

ipc lgtm

[DaleCurtis, 2015-06-08 17:50:48]

lgtm

[miu, 2015-06-08 17:51:54]

The CQ bit was checked by miu@chromium.org

[miu, 2015-06-08 17:51:55]

The patchset sent to the CQ was uploaded after l-g-t-m from hubbe@chromium.org
Link to the patchset: https://codereview.chromium.org/1157193002/#ps60001
(title: "Addressed hubbe's round 2 comments, and REBASE.")

[commit-bot: I haz the power, 2015-06-08 17:53:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1157193002/60001

[commit-bot: I haz the power, 2015-06-08 19:17:30]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-06-08 19:19:18]

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