Moves video frame callbacks from VideoFrameCompositor to Renderer.

Moves video frame callbacks from VideoFrameCompositor to Renderer.

BUG=571155
Committed: https://crrev.com/5d86e9b21eaccaf56b05fdfde6fad745cc3470b7
Cr-Commit-Position: refs/heads/master@{#394209}

[alokp, 2016-05-16 06:05:52]

alokp@chromium.org changed reviewers:
+ sandersd@chromium.org, xhwang@chromium.org

[alokp, 2016-05-17 05:45:14]

ping!

[xhwang, 2016-05-17 06:24:11]

xhwang@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[xhwang, 2016-05-17 06:24:13]

nice patch!

+dalecurtis for another look at changes in VideoRendererImpl and
VideoFrameCompositor.

https://codereview.chromium.org/1978973002/diff/130001/media/base/renderer_cl...
File media/base/renderer_client.h (right):

https://codereview.chromium.org/1978973002/diff/130001/media/base/renderer_cl...
media/base/renderer_client.h:36: virtual void OnVideoOpacityChange(bool opaque)
= 0;
This client is also used by AudioRenderer, where these two calls don't make
sense. Add a comment/TODO about improve that later?

https://codereview.chromium.org/1978973002/diff/130001/media/blink/video_fram...
File media/blink/video_frame_compositor.h (right):

https://codereview.chromium.org/1978973002/diff/130001/media/blink/video_fram...
media/blink/video_frame_compositor.h:57: compositor_task_runner);
nit: explicit

https://codereview.chromium.org/1978973002/diff/130001/media/mojo/services/me...
File media/mojo/services/media_mojo_unittest.cc (right):

https://codereview.chromium.org/1978973002/diff/130001/media/mojo/services/me...
media/mojo/services/media_mojo_unittest.cc:55: void
OnVideoNaturalSizeChange(mojo::SizePtr size) override {}
You can define a MOCK_METHOD using non movable type and call it here to achieve
the same result.

[xhwang, 2016-05-17 06:24:43]

Also, I meant LGTM from me.

[alokp, 2016-05-17 16:25:49]

Dale/Dan: Any other issues?

https://codereview.chromium.org/1978973002/diff/130001/media/base/renderer_cl...
File media/base/renderer_client.h (right):

https://codereview.chromium.org/1978973002/diff/130001/media/base/renderer_cl...
media/base/renderer_client.h:36: virtual void OnVideoOpacityChange(bool opaque)
= 0;
On 2016/05/17 06:24:12, xhwang wrote:
> This client is also used by AudioRenderer, where these two calls don't make
> sense. Add a comment/TODO about improve that later?

Added comment. This is equivalent to DemuxerStream interface where a portion of
the API is specific to Audio and Video renderers. I do not think this warrants
separate Client interfaces for Audio and Video renderers.

I also already have checks in place to ensure that it does not get invoked by
AudioRenderer. See AudioRendererImplTest and RendererInternal.

https://codereview.chromium.org/1978973002/diff/130001/media/blink/video_fram...
File media/blink/video_frame_compositor.h (right):

https://codereview.chromium.org/1978973002/diff/130001/media/blink/video_fram...
media/blink/video_frame_compositor.h:57: compositor_task_runner);
On 2016/05/17 06:24:13, xhwang wrote:
> nit: explicit

Done.

https://codereview.chromium.org/1978973002/diff/130001/media/mojo/services/me...
File media/mojo/services/media_mojo_unittest.cc (right):

https://codereview.chromium.org/1978973002/diff/130001/media/mojo/services/me...
media/mojo/services/media_mojo_unittest.cc:55: void
OnVideoNaturalSizeChange(mojo::SizePtr size) override {}
On 2016/05/17 06:24:13, xhwang wrote:
> You can define a MOCK_METHOD using non movable type and call it here to
achieve
> the same result.

Acknowledged. I will append this to the comment. It can be done if we need to
use the mock method.

[DaleCurtis, 2016-05-17 17:56:38]

Just looked over VFC and VRI, but lgtm

[alokp, 2016-05-17 17:59:19]

The CQ bit was checked by alokp@chromium.org

[alokp, 2016-05-17 17:59:19]

The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org
Link to the patchset: https://codereview.chromium.org/1978973002/#ps150001
(title: "addresses comments")

[commit-bot: I haz the power, 2016-05-17 18:00:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1978973002/150001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1978973002/150001

[commit-bot: I haz the power, 2016-05-17 20:20:59]

Committed patchset #9 (id:150001)

[commit-bot: I haz the power, 2016-05-17 20:22:10]

Description was changed from

==========
Moves video frame callbacks from VideoFrameCompositor to Renderer.

BUG=571155
==========

to

==========
Moves video frame callbacks from VideoFrameCompositor to Renderer.

BUG=571155

Committed: https://crrev.com/5d86e9b21eaccaf56b05fdfde6fad745cc3470b7
Cr-Commit-Position: refs/heads/master@{#394209}
==========

[commit-bot: I haz the power, 2016-05-17 20:22:11]

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