[DO NOT COMMIT] Initial MediaPlayerRenderer plumbing and basic features

media/mojo/interfaces/renderer.mojom

Index: media/mojo/interfaces/renderer.mojom
diff --git a/media/mojo/interfaces/renderer.mojom b/media/mojo/interfaces/renderer.mojom
index 16267c4ce5a3a2b2f52b88b4e8ed3bdf7707a3d0..69ba95556e230ac7ef660be854b303b9fcedaabf 100644
--- a/media/mojo/interfaces/renderer.mojom
+++ b/media/mojo/interfaces/renderer.mojom
@@ -13,7 +13,9 @@ interface Renderer {
   // executing the callback with whether the initialization succeeded.
   Initialize(RendererClient client,
              DemuxerStream? audio,
-             DemuxerStream? video) => (bool success);
+             DemuxerStream? video,
+             string? url,

[xhwang, 2016/06/22 17:39:02]

In the future we may want to wrap audio, video and url into a mojo
MediaProvider. Can you add a TODO here?

+             int64 surface_id) => (bool success);

[xhwang, 2016/06/22 17:39:02]

In C++, currently we pass the |request_surface_cb| at construction time (in
RendererFactory::CreateRenderer). When using MojoRenderer in
MojoMediaApplication, we can add construction time parameters in
ServiceFactory::CreateRenderer()

https://chromiumcodereview.appspot.com/2016053003/

In your case, you are not using ServiceFactory, and GetInterface() won't allow
you to pass any additional parameters :( I think sandersd@ hit this problem as
well when working on mojo VideoDeocder. The solution there is to add a
Construct() call:

https://cs.chromium.org/chromium/src/media/mojo/interfaces/video_decoder.mojo...

I don't have a good solution here. You current change is okay, but please add a
TODO so we can revisit this later.

[xhwang, 2016/06/22 17:43:09]

For more background, we try to keep the mojo part a pure transport layer, so
mojom::Renderer::Initialize() matches Renderer::Initialize(). But as discussed
above this causes some headache. So maybe we should move all construction time
and initialization time parameters to mojom::Renderer::Initialize(), to make
things simple. On platforms that don't care about some of the parameters they
can just ignore them.

+sandersd: WDYT?

[sandersd (OOO until July 31), 2016/06/23 18:43:35]

The last time we talked, we decided against parameters for CreateVideoDecoder()
this because we anticipated a future where we only had GetInterface()-style
access. There was no actual technical restriction preventing us from doing so at
the time.

In general, there should not be any classes for which Initialize() is called
more than once. I don't see any problem with passing "constructor" parameters
along with "initialize" parameters to one method named Initialize(). If there
are many constructor parameters, they should go into a struct.

I agree that the rest of the parameters should match media:: interfaces as
closely as possible, and that it's fine to ignore some of those parameters.

 
   // Discards any buffered data, executing callback when completed.
   // NOTE: If an error occurs, RendererClient::OnError() can be called
@@ -59,4 +61,8 @@ interface RendererClient {
 
   // Executed for the first video frame and whenever opacity changes.
   OnVideoOpacityChange(bool opaque);
+
+  // Executed the first time the metadata is updated, and whenever the duration
+  // changes.
+  OnDurationChange(int64 duration_usec);
 };