IPC->mojo of audio_renderer_host

media/mojo/interfaces/audio_output.mojom

Index: media/mojo/interfaces/audio_output.mojom
diff --git a/media/mojo/interfaces/audio_output.mojom b/media/mojo/interfaces/audio_output.mojom
index 895a93e4967727e4d2846679b50433a2de46ff94..e4a5ce3c3b785ef7e022e4dce9693a0990beeadb 100644
--- a/media/mojo/interfaces/audio_output.mojom
+++ b/media/mojo/interfaces/audio_output.mojom
@@ -5,29 +5,54 @@
 module media.mojom;
 
 import "media/mojo/interfaces/audio_parameters.mojom";
+import "url/mojo/origin.mojom";
+
+enum AudioOutputStreamState {
+  PLAYING,
+  PAUSED,
+  ERROR
+};
 
-// This interface handles audio output stream operations.
-// It allows to close a stream.
-// TODO(rchtara): Add methods that allow the interaction with audio output
-// streams: Play, Pause and SetVolume to this interface.
-// See crbug.com/606707 for more details.
 interface AudioOutputStream {
-  Close();
+  Play();
+  Pause();
+  SetVolume(double volume);
+};
+
+interface AudioOutputStreamClient {
+  // Called by the AudioOutputStreamService after any stream
+  // manipulation by the AudioOutputStreamClient.
+  OnStreamStateChange(AudioOutputStreamState state);

[Henrik Grunell, 2016/09/01 15:09:07]

I believe we can replace this with responses. Play and pause for sure. Some
(all?) of the errors could probably be signaled by simply deleting the
AudioOutputStream object, which the other side will be informed of.

[Max Morin, 2016/09/02 10:27:07]

I agree. I'll change this.

 };
 
-// This interface manages audio output streams.
-// It allows to create an AudioOutputStream.
-// TODO(rchtara): Add a method to request device authorization to this
-// interface.
-// See crbug.com/606707 for more details.
+// AudioOutput manages device authorizations and AudioOutputStreamClients
 interface AudioOutput {
-  // TODO(rchtara): Remove |stream_id| from AudioOutput::CreateStream when all
-  // the stream operations are mojofied.
+  // Used to request device authorization from the AudioOutputService.
+  // Returns |authorization_id| which will need to be used in CreateStream
+  // if device authorization is successful.
+  RequestDeviceAuthorization(
+      int32 stream_id,

[Henrik Grunell, 2016/09/01 15:09:07]

A follow-up CL should remove the stream id, but it requires some analysis of how
to deal with the authorization requests. It could be a good idea to do that
already now.

Historic not: Originally it was planned to be removed when 
RequestDeviceAuthorization was added to this interface. Hence it's existence in
CreateStream in the interface.

+      int32 render_frame_id,
+      int32 session_id,
+      string device_id,
+      url.mojom.Origin origin) =>
+      (int32 state,
+       AudioParameters output_params,
+       string matched_device_id);
+
+  // Returns a bound AudioOutputStream that will be used to directly
+  // communicate with an AudioOutputStreamService. It must use the
+  // provided AudioOutputStreamClient to signal errors.
   CreateStream(
       int32 stream_id,
+      int32 render_frame_id,

[Henrik Grunell, 2016/09/01 15:09:07]

Do you know what happened with the discussion around having a service per render
frame? Seems like the design is back to one service per render process.

[Max Morin, 2016/09/02 10:27:07]

I haven't read that discussion. Why would we have one per frame?

+      AudioOutputStreamClient client,
       AudioParameters params) =>
-      (int32 stream_id,
-       AudioOutputStream? stream,
-       handle<shared_buffer>? shared_buffer,
-       handle? socket_descriptor);
-};
+      (AudioOutputStream stream,
+       handle<shared_buffer> shared_buffer,
+       handle socket_descriptor);
+
+  // Closes a (possibly) active AudioOutputStreamService. Clears
+  // authorization data and any associated info with |stream_id|.
+  CloseStream(int32 stream_id);

[Henrik Grunell, 2016/09/01 15:09:07]

This should not be needed. Deleting the AudioOutputStream when closing will
afaik signal to the other side.

[Max Morin, 2016/09/02 10:27:07]

Ok, I'll remove it.

+};