Mojo interfaces needed for switching audio rendering stream creation and closing from IPC to Mojo

media/mojo/interfaces/audio_output.mojom

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+module media.interfaces;

[mcasas, 2016/04/29 17:14:11]

It's customary to call these modules

module media.mojom;

as e.g. content.mojom, blink.mojom, etc
I understand this is larger than this CL,
could you please make a crbug.com for it
and add xhwang@? Thanks.

[xhwang, 2016/04/29 17:16:24]

We already have a bug for this:
https://bugs.chromium.org/p/chromium/issues/detail?id=597968

[xhwang, 2016/04/29 17:47:56]

Actually feel free to use media.mojom for new interfaces. I'll fix the old ones
at some point.

[rchtara, 2016/05/02 15:54:11]

Done.

[rchtara, 2016/05/02 15:54:11]

Done.

[rchtara, 2016/05/02 15:54:11]

Done.

+
+import "media/mojo/interfaces/media_types.mojom";
+
+// This interface handles audio output stream operations. It will be used by
+// the AudioOutputClient in the renderer to perform operations on
+// AudioOutputStreamImpl in the browser.

[xhwang, 2016/04/29 17:47:56]

It's still odd that you create this stream, doing nothing on it, and then will
Close() it. Are we planning to add more methods on this interface?

[xhwang, 2016/04/29 17:47:56]

Please document what this interface does instead of where it'll be
implemented/used.

[rchtara, 2016/05/02 15:54:10]

yes, we are going to add play, pause, setvolume

[rchtara, 2016/05/02 15:54:11]

Done.

+interface AudioOutputStream {
+  Close();
+};
+
+// This interface manages audio output streams. It will be used by
+// the AudioOutputClient in the renderer to perform operations on
+// AudioOutputImpl in the browser.

[xhwang, 2016/04/29 17:47:56]

Please document what this interface does instead of where it'll be
implemented/used.

[rchtara, 2016/05/02 15:54:11]

Done.

+interface AudioOutput {
+  CreateStream(
+      int32 stream_id,
+      int32 render_frame_id,

[xhwang, 2016/04/29 17:47:56]

This still looks wrong; media should not know about render frames. I noticed
that in the implementation, you have one AudioRendererHost and one
AudioOutputImpl per render process. Does it make sense to keep one
AudioRendererHost per render process, but have one AudioOutputImpl per render
frame?

Basically AudioOutputImpl should be registered at the RenderFrame level instead
of RenderProcess level:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr...

Then since we are already at the RenderFrame level, AudioOutputImpl should
already know about the |render_frame_id|, and we don't need to pass it in this
interface.

[rchtara, 2016/05/02 15:54:11]

Done.

[rchtara, 2016/05/04 08:38:43]

we are going to move the interface to the content to avoid problems.

+      AudioOutputStreamParameters params) =>
+      (int32 stream_id,
+       AudioOutputStream? stream,
+       handle<shared_buffer>? shared_buffer,
+       handle? socket_descriptor);
+};