[PPAPI] API definition for audio media stream artifacts

[PPAPI] API definition for audio media stream artifacts

This API follows the design at
https://docs.google.com/a/google.com/document/d/1rlwmFhf7VCX8mfrBok8wqXNgvr_ERhL2k6Fqha-pgIo/edit?disco=AAAAAHos8Y8#

c++ interface will be implemented in a separate CL.

It defines new objects for the consumption of media audio tracks based
on the private VideoSource/VideoDestination classes.

BUG=330851
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=244009

[Peng, 2014-01-07 21:13:32]

Hi,

The MediaStreamAudioTrack API definition is copied and modified from
MediaStreamVideoTrack. PTAL. Thanks.

binji: PTAL changes in native_client_sdk. Thanks.

[binji, 2014-01-07 21:15:48]

native_client_sdk lgtm

[dmichael (off chromium), 2014-01-07 22:00:59]

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_audio_fram...
File ppapi/api/ppb_audio_frame.idl (right):

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_audio_fram...
ppapi/api/ppb_audio_frame.idl:54: * rignt now.
rignt->right

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_audio_fram...
ppapi/api/ppb_audio_frame.idl:64: * @return The number of channles in the audio
frame.
channles->channels

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_audio_fram...
ppapi/api/ppb_audio_frame.idl:75: * For 1 second sample rate 44100 stereo audio
frame, the number of samples
This is kind of confusingly worded to me. I think you mean something like:
"For example, at a sampling rate of 44,100 Hz in stereo audio, a frame
containing 44100 * 2 samples would have a duration of 1 second."
or something.

We should try to avoid implying that the frame has 1 second of data in it. So
maybe even a more realistic example with shorter time duration would be better.

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_audio_fram...
ppapi/api/ppb_audio_frame.idl:91: * Gets the size of data buffer.
of +the+ data buffer +in bytes+.

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_media_stre...
File ppapi/api/ppb_media_stream_audio_track.idl (right):

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_media_stre...
ppapi/api/ppb_media_stream_audio_track.idl:17: interface
PPB_MediaStreamAudioTrack {
We had a discussion about the names before holiday break, but I don't think I
had a chance to respond, and I'm not sure if we concluded anything. I suspect
you're going to want this to be named something implying it's for input, like:
PPB_MediaStreamAudioInputTrack
or something?

Thoughts? (Sorry for forgetting where we were before)

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_media_stre...
ppapi/api/ppb_media_stream_audio_track.idl:115: * Closes the MediaStream audio
track and disconnects it from audio source.
from +the+ audio

[Peng, 2014-01-07 22:29:27]

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_audio_fram...
File ppapi/api/ppb_audio_frame.idl (right):

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_audio_fram...
ppapi/api/ppb_audio_frame.idl:54: * rignt now.
On 2014/01/07 22:00:59, dmichael wrote:
> rignt->right

Done.

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_audio_fram...
ppapi/api/ppb_audio_frame.idl:64: * @return The number of channles in the audio
frame.
On 2014/01/07 22:00:59, dmichael wrote:
> channles->channels

Done.

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_audio_fram...
ppapi/api/ppb_audio_frame.idl:75: * For 1 second sample rate 44100 stereo audio
frame, the number of samples
On 2014/01/07 22:00:59, dmichael wrote:
> This is kind of confusingly worded to me. I think you mean something like:
> "For example, at a sampling rate of 44,100 Hz in stereo audio, a frame
> containing 44100 * 2 samples would have a duration of 1 second."
> or something.
> 
> We should try to avoid implying that the frame has 1 second of data in it. So
> maybe even a more realistic example with shorter time duration would be
better.

Done.

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_audio_fram...
ppapi/api/ppb_audio_frame.idl:91: * Gets the size of data buffer.
On 2014/01/07 22:00:59, dmichael wrote:
> of +the+ data buffer +in bytes+.

Done.

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_media_stre...
File ppapi/api/ppb_media_stream_audio_track.idl (right):

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_media_stre...
ppapi/api/ppb_media_stream_audio_track.idl:17: interface
PPB_MediaStreamAudioTrack {
On 2014/01/07 22:00:59, dmichael wrote:
> We had a discussion about the names before holiday break, but I don't think I
> had a chance to respond, and I'm not sure if we concluded anything. I suspect
> you're going to want this to be named something implying it's for input, like:
> PPB_MediaStreamAudioInputTrack
> or something?
> 
> Thoughts? (Sorry for forgetting where we were before)

I asked yuzhu, the dev API can be changed. So I landed it. I thought it is
easier to fix and review it in a separate CL. Sorry for it.

When I was implementing the video track, I found it is very easy to combine
producer and consumer to a single Track interface. We just need add a Create()
method to create produce track.

So there are two solutions.
1. Single interface MediaStreamAudioTrack for both input and output
Pro:
 * It saves a lot of code.
 * HTML5 only has MediaStreamTrack object. Single interface is more familiar for
web developers.
Con:
 * Maybe it is a little confusing if the API is not well documented.

2. Two interfaces MediaStreamAudioTrackSource and
MediaStreamAudioTrackDestination.


I am slight prefer solution 1. What do you think?

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_media_stre...
ppapi/api/ppb_media_stream_audio_track.idl:115: * Closes the MediaStream audio
track and disconnects it from audio source.
On 2014/01/07 22:00:59, dmichael wrote:
> from +the+ audio

Done.

[dmichael (off chromium), 2014-01-09 17:07:45]

lgtm

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_media_stre...
File ppapi/api/ppb_media_stream_audio_track.idl (right):

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_media_stre...
ppapi/api/ppb_media_stream_audio_track.idl:17: interface
PPB_MediaStreamAudioTrack {
On 2014/01/07 22:29:27, Peng wrote:
> I asked yuzhu, the dev API can be changed. So I landed it. I thought it is
> easier to fix and review it in a separate CL. Sorry for it.
No, that's totally fine. We can come back around and change it still.

 
> When I was implementing the video track, I found it is very easy to combine
> producer and consumer to a single Track interface. We just need add a Create()
> method to create produce track.
> 
> So there are two solutions.
> 1. Single interface MediaStreamAudioTrack for both input and output
> Pro:
>  * It saves a lot of code.
>  * HTML5 only has MediaStreamTrack object. Single interface is more familiar
for
> web developers.
> Con:
>  * Maybe it is a little confusing if the API is not well documented.
In this world, would a plugin ever be able to use the same MediaStreamVideoTrack
to read _and_ write tracks? If so we *definitely* should not separate them. If
not, it feels slightly wrong to have them in the same API. Not sure. Maybe
adding a "read only" field later on will help with that confusion.

Another thing we have to think about is the interaction with JavaScript. Since
we want to be able to pass these back and forth using
PPB_Messaging/PPP_Messaging, it might be best to have a 1-1 mapping. So that
would argue for having 1 type for VideoStreamTrack:
http://dev.w3.org/2011/webrtc/editor/getusermedia.html#idl-def-VideoStreamTrack
...and 1 type for AudioStreamTrack...
http://dev.w3.org/2011/webrtc/editor/getusermedia.html#idl-def-AudioStreamTrack

(In fact, I wonder if PPB_AudioStreamTrack and PPB_VideoStreamTrack are better
names, to match the HTML spec?)

> 
> 2. Two interfaces MediaStreamAudioTrackSource and
> MediaStreamAudioTrackDestination.
> 
> 
> I am slight prefer solution 1. What do you think?
I think given that it matches better with HTML/JavaScript, I think I agree with
you. We'll have to keep an eye out for that when you add pushing frames to that
APIs, because the current file-level documentation doesn't reflect that.

[bbudge, 2014-01-09 19:03:22]

https://codereview.chromium.org/126373003/diff/120001/ppapi/api/ppb_audio_fra...
File ppapi/api/ppb_audio_frame.idl (right):

https://codereview.chromium.org/126373003/diff/120001/ppapi/api/ppb_audio_fra...
ppapi/api/ppb_audio_frame.idl:37: * start of the containing audio stream.
I think the first sentence is enough here, since you describe the parameter
below. The second sentence is actually a little ambiguous here.

https://codereview.chromium.org/126373003/diff/120001/ppapi/api/ppb_audio_fra...
ppapi/api/ppb_audio_frame.idl:76: * containing 4410 * 2 samples would have a
duration of 100 millisecond.
s/millisecond/milliseconds

https://codereview.chromium.org/126373003/diff/120001/ppapi/api/ppb_audio_fra...
ppapi/api/ppb_audio_frame.idl:81: * Gets the data buffer for audio frame
samples.
s/for/containing the

https://codereview.chromium.org/126373003/diff/120001/ppapi/ppapi_sources.gypi
File ppapi/ppapi_sources.gypi (right):

https://codereview.chromium.org/126373003/diff/120001/ppapi/ppapi_sources.gyp...
ppapi/ppapi_sources.gypi:64: 'c/ppb_video_frame.h',
Did you mean to add this file with this change?

[bbudge, 2014-01-09 19:11:33]

lgtm

[yzshen1, 2014-01-09 19:27:45]

LGTM

https://codereview.chromium.org/126373003/diff/120001/ppapi/api/ppb_audio_fra...
File ppapi/api/ppb_audio_frame.idl (right):

https://codereview.chromium.org/126373003/diff/120001/ppapi/api/ppb_audio_fra...
ppapi/api/ppb_audio_frame.idl:53: * @return The sample size of the audio frame.
It always returns 2 (16 bits)
nit: maybe we don't have to have the last sentence. In that way, we don't have
to update our API version when we decide to return a number other than 2.

[Peng, 2014-01-09 19:47:47]

https://codereview.chromium.org/126373003/diff/120001/ppapi/api/ppb_audio_fra...
File ppapi/api/ppb_audio_frame.idl (right):

https://codereview.chromium.org/126373003/diff/120001/ppapi/api/ppb_audio_fra...
ppapi/api/ppb_audio_frame.idl:37: * start of the containing audio stream.
On 2014/01/09 19:03:22, bbudge wrote:
> I think the first sentence is enough here, since you describe the parameter
> below. The second sentence is actually a little ambiguous here.

Done.

https://codereview.chromium.org/126373003/diff/120001/ppapi/api/ppb_audio_fra...
ppapi/api/ppb_audio_frame.idl:53: * @return The sample size of the audio frame.
It always returns 2 (16 bits)
On 2014/01/09 19:27:45, yzshen1 wrote:
> nit: maybe we don't have to have the last sentence. In that way, we don't have
> to update our API version when we decide to return a number other than 2. 

Done.

https://codereview.chromium.org/126373003/diff/120001/ppapi/api/ppb_audio_fra...
ppapi/api/ppb_audio_frame.idl:76: * containing 4410 * 2 samples would have a
duration of 100 millisecond.
On 2014/01/09 19:03:22, bbudge wrote:
> s/millisecond/milliseconds

Done.

https://codereview.chromium.org/126373003/diff/120001/ppapi/api/ppb_audio_fra...
ppapi/api/ppb_audio_frame.idl:76: * containing 4410 * 2 samples would have a
duration of 100 millisecond.
On 2014/01/09 19:03:22, bbudge wrote:
> s/millisecond/milliseconds

Done.

https://codereview.chromium.org/126373003/diff/120001/ppapi/api/ppb_audio_fra...
ppapi/api/ppb_audio_frame.idl:81: * Gets the data buffer for audio frame
samples.
On 2014/01/09 19:03:22, bbudge wrote:
> s/for/containing the

Done.

https://codereview.chromium.org/126373003/diff/120001/ppapi/ppapi_sources.gypi
File ppapi/ppapi_sources.gypi (right):

https://codereview.chromium.org/126373003/diff/120001/ppapi/ppapi_sources.gyp...
ppapi/ppapi_sources.gypi:64: 'c/ppb_video_frame.h',
On 2014/01/09 19:03:22, bbudge wrote:
> Did you mean to add this file with this change?

Yeah. I forgot it in video API cl. Probably it is better to add it in video api
thunk CL.

Remove it.

[commit-bot: I haz the power, 2014-01-09 19:48:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/penghuang@chromium.org/126373003/260001

[commit-bot: I haz the power, 2014-01-09 19:48:35]

Failed to apply patch for ppapi/c/ppb_media_stream_audio_track.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         ppapi/c/ppb_media_stream_audio_track.h
  Copied ppapi/c/ppb_media_stream_video_track.h ->
ppapi/c/ppb_media_stream_audio_track.h
  patching file ppapi/c/ppb_media_stream_audio_track.h
  Hunk #1 FAILED at 3.
  Hunk #5 succeeded at 96 with fuzz 2 (offset 3 lines).
  Hunk #6 succeeded at 107 (offset 3 lines).
  Hunk #7 succeeded at 116 (offset 3 lines).
  1 out of 7 hunks FAILED -- saving rejects to file
ppapi/c/ppb_media_stream_audio_track.h.rej

Patch:   NR 
ppapi/c/ppb_media_stream_video_track.h->ppapi/c/ppb_media_stream_audio_track.h
Index: ppapi/c/ppb_media_stream_audio_track.h
diff --git a/ppapi/c/ppb_media_stream_video_track.h
b/ppapi/c/ppb_media_stream_audio_track.h
similarity index 62%
copy from ppapi/c/ppb_media_stream_video_track.h
copy to ppapi/c/ppb_media_stream_audio_track.h
index
dd93c4b00d6cf181885579e82eadfd4c8844727e..89c78d07b47309e2871f725e81aaf5d3e8d90d1b
100644
--- a/ppapi/c/ppb_media_stream_video_track.h
+++ b/ppapi/c/ppb_media_stream_audio_track.h
@@ -3,10 +3,10 @@
  * found in the LICENSE file.
  */
 
-/* From ppb_media_stream_video_track.idl modified Tue Jan  7 10:21:20 2014. */
+/* From ppb_media_stream_audio_track.idl modified Tue Jan  7 17:05:06 2014. */
 
-#ifndef PPAPI_C_PPB_MEDIA_STREAM_VIDEO_TRACK_H_
-#define PPAPI_C_PPB_MEDIA_STREAM_VIDEO_TRACK_H_
+#ifndef PPAPI_C_PPB_MEDIA_STREAM_AUDIO_TRACK_H_
+#define PPAPI_C_PPB_MEDIA_STREAM_AUDIO_TRACK_H_
 
 #include "ppapi/c/pp_bool.h"
 #include "ppapi/c/pp_completion_callback.h"
@@ -15,12 +15,12 @@
 #include "ppapi/c/pp_stdint.h"
 #include "ppapi/c/pp_var.h"
 
-#define PPB_MEDIASTREAMVIDEOTRACK_INTERFACE_0_1 \
-    "PPB_MediaStreamVideoTrack;0.1" /* dev */
+#define PPB_MEDIASTREAMAUDIOTRACK_INTERFACE_0_1 \
+    "PPB_MediaStreamAudioTrack;0.1" /* dev */
 /**
  * @file
- * Defines the <code>PPB_MediaStreamVideoTrack</code> interface. Used for
- * receiving video frames from a MediaStream video track in the browser.
+ * Defines the <code>PPB_MediaStreamAudioTrack</code> interface. Used for
+ * receiving audio frames from a MediaStream audio track in the browser.
  * This interface is still in development (Dev API status) and may change.
  */
 
@@ -31,17 +31,17 @@
  */
 /**
  */
-struct PPB_MediaStreamVideoTrack_0_1 { /* dev */
+struct PPB_MediaStreamAudioTrack_0_1 { /* dev */
   /**
-   * Determines if a resource is a MediaStream video track resource.
+   * Determines if a resource is a MediaStream audio track resource.
    *
    * @param[in] resource The <code>PP_Resource</code> to test.
    *
    * @return A <code>PP_Bool</code> with <code>PP_TRUE</code> if the given
-   * resource is a Mediastream video track resource or <code>PP_FALSE</code>
+   * resource is a Mediastream audio track resource or <code>PP_FALSE</code>
    * otherwise.
    */
-  PP_Bool (*IsMediaStreamVideoTrack)(PP_Resource resource);
+  PP_Bool (*IsMediaStreamAudioTrack)(PP_Resource resource);
   /**
    * Configures underlying frame buffers for incoming frames.
    * If the application doesn't want to drop frames, then the
@@ -51,36 +51,39 @@ struct PPB_MediaStreamVideoTrack_0_1 { /* dev */
    * this by examining the timestamp on returned frames.
    * If <code>Configure()</code> is not used, default settings will be used.
    *
-   * @param[in] video_track A <code>PP_Resource</code> corresponding to a video
+   * @param[in] audio_track A <code>PP_Resource</code> corresponding to an
audio
    * resource.
-   * @param[in] max_buffered_frames The maximum number of video frames to
+   * @param[in] samples_per_frame The number of audio samples in an audio
frame.
+   * @param[in] max_buffered_frames The maximum number of audio frames to
    * hold in the input buffer.
    *
    * @return An int32_t containing a result code from <code>pp_errors.h</code>.
    */
-  int32_t (*Configure)(PP_Resource video_track, uint32_t max_buffered_frames);
+  int32_t (*Configure)(PP_Resource audio_track,
+                       uint32_t samples_per_frame,
+                       uint32_t max_buffered_frames);
   /**
-   * Returns the track ID of the underlying MediaStream video track.
+   * Returns the track ID of the underlying MediaStream audio track.
    *
-   * @param[in] video_track The <code>PP_Resource</code> to check.
+   * @param[in] audio_track The <code>PP_Resource</code> to check.
    *
    * @return A <code>PP_Var</code> containing the MediaStream track ID as
    * a string.
    */
-  struct PP_Var (*GetId)(PP_Resource video_track);
+  struct PP_Var (*GetId)(PP_Resource audio_track);
   /**
    * Checks whether the underlying MediaStream track has ended.
    * Calls to GetFrame while the track has ended are safe to make and will
    * complete, but will fail.
    *
-   * @param[in] video_track The <code>PP_Resource</code> to check.
+   * @param[in] audio_track The <code>PP_Resource</code> to check.
    *
    * @return A <code>PP_Bool</code> with <code>PP_TRUE</code> if the given
    * MediaStream track has ended or <code>PP_FALSE</code> otherwise.
    */
-  PP_Bool (*HasEnded)(PP_Resource video_track);
+  PP_Bool (*HasEnded)(PP_Resource audio_track);
   /**
-   * Gets the next video frame from the MediaStream track.
+   * Gets the next audio frame from the MediaStream track.
    * If internal processing is slower than the incoming frame rate, new frames
    * will be dropped from the incoming stream. Once the input buffer is full,
    * frames will be dropped until <code>RecycleFrame()</code> is called to free
@@ -90,9 +93,9 @@ struct PPB_MediaStreamVideoTrack_0_1 { /* dev */
    * <code>callback</code> will be called, when a new frame is received or an
    * error happens.
    *
-   * @param[in] video_track A <code>PP_Resource</code> corresponding to a video
+   * @param[in] audio_track A <code>PP_Resource</code> corresponding to an
audio
    * resource.
-   * @param[out] frame A <code>PP_Resource</code> corresponding to a VideoFrame
+   * @param[out] frame A <code>PP_Resource</code> corresponding to an
AudioFrame
    * resource.
    * @param[in] callback A <code>PP_CompletionCallback</code> to be called upon
    * completion of GetFrame().
@@ -101,7 +104,7 @@ struct PPB_MediaStreamVideoTrack_0_1 { /* dev */
    * Returns PP_ERROR_NOMEMORY if <code>max_buffered_frames</code> frames
buffer
    * was not allocated successfully.
    */
-  int32_t (*GetFrame)(PP_Resource video_track,
+  int32_t (*GetFrame)(PP_Resource audio_track,
                       PP_Resource* frame,
                       struct PP_CompletionCallback callback);
   /**
@@ -110,26 +113,26 @@ struct PPB_MediaStreamVideoTrack_0_1 { /* dev */
    * invalid. The caller should release all references it holds to
    * <code>frame</code> and not use it anymore.
    *
-   * @param[in] video_track A <code>PP_Resource</code> corresponding to a video
+   * @param[in] audio_track A <code>PP_Resource</code> corresponding to an
audio
    * resource.
-   * @param[in] frame A <code>PP_Resource</code> corresponding to a VideoFrame
+   * @param[in] frame A <code>PP_Resource</code> corresponding to an AudioFrame
    * resource returned by <code>GetFrame()</code>.
    *
    * @return An int32_t containing a result code from <code>pp_errors.h</code>.
    */
-  int32_t (*RecycleFrame)(PP_Resource video_track, PP_Resource frame);
+  int32_t (*RecycleFrame)(PP_Resource audio_track, PP_Resource frame);
   /**
-   * Closes the MediaStream video track and disconnects it from video source.
-   * After calling <code>Close()</code>, no new frames will be received.
+   * Closes the MediaStream audio track and disconnects it from the audio
+   * source. After calling <code>Close()</code>, no new frames will be
received.
    *
-   * @param[in] video_track A <code>PP_Resource</code> corresponding to a
-   * MediaStream video track resource.
+   * @param[in] audio_track A <code>PP_Resource</code> corresponding to a
+   * MediaStream audio track resource.
    */
-  void (*Close)(PP_Resource video_track);
+  void (*Close)(PP_Resource audio_track);
 };
 /**
  * @}
  */
 
-#endif  /* PPAPI_C_PPB_MEDIA_STREAM_VIDEO_TRACK_H_ */
+#endif  /* PPAPI_C_PPB_MEDIA_STREAM_AUDIO_TRACK_H_ */

[commit-bot: I haz the power, 2014-01-09 20:03:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/penghuang@chromium.org/126373003/370001

[commit-bot: I haz the power, 2014-01-09 20:03:25]

Failed to apply patch for
ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
  Hunk #1 FAILED at 1.
  Hunk #2 succeeded at 45 (offset -1 lines).
  Hunk #3 succeeded at 59 (offset -1 lines).
  Hunk #4 succeeded at 156 (offset -1 lines).
  Hunk #5 succeeded at 250 (offset -1 lines).
  Hunk #6 succeeded at 920 (offset -1 lines).
  Hunk #7 FAILED at 4075.
  Hunk #8 FAILED at 4254.
  Hunk #9 succeeded at 5229 (offset -5 lines).
  Hunk #10 succeeded at 5751 (offset -5 lines).
  3 out of 10 hunks FAILED -- saving rejects to file
ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c.rej

Patch:       ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
Index: ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
diff --git a/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
b/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
index
c83a1f6d5b812bcea6ae2c088ec575d67e31f93b..c2f4bfb3feff131b25aa38933038506baf54e652
100644
--- a/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
+++ b/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2013 The Chromium Authors. All rights reserved.
+/* Copyright (c) 2014 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.
  */
@@ -46,6 +46,7 @@
 #include "ppapi/c/extensions/dev/ppb_ext_socket_dev.h"
 #include "ppapi/c/ppb_audio.h"
 #include "ppapi/c/ppb_audio_config.h"
+#include "ppapi/c/ppb_audio_frame.h"
 #include "ppapi/c/ppb_console.h"
 #include "ppapi/c/ppb_core.h"
 #include "ppapi/c/ppb_file_io.h"
@@ -59,6 +60,7 @@
 #include "ppapi/c/ppb_image_data.h"
 #include "ppapi/c/ppb_input_event.h"
 #include "ppapi/c/ppb_instance.h"
+#include "ppapi/c/ppb_media_stream_audio_track.h"
 #include "ppapi/c/ppb_media_stream_video_track.h"
 #include "ppapi/c/ppb_message_loop.h"
 #include "ppapi/c/ppb_messaging.h"
@@ -155,6 +157,7 @@ static struct __PnaclWrapperInfo
Pnacl_WrapperInfo_PPB_WheelInputEvent_1_0;
 static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_KeyboardInputEvent_1_0;
 static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_TouchInputEvent_1_0;
 static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_IMEInputEvent_1_0;
+static struct __PnaclWrapperInfo
Pnacl_WrapperInfo_PPB_MediaStreamAudioTrack_0_1;
 static struct __PnaclWrapperInfo
Pnacl_WrapperInfo_PPB_MediaStreamVideoTrack_0_1;
 static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_MessageLoop_1_0;
 static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_Messaging_1_0;
@@ -248,6 +251,8 @@ static struct __PnaclWrapperInfo
Pnacl_WrapperInfo_PPB_Ext_Socket_Dev_0_2;
 
 /* Not generating wrapper methods for PPB_AudioConfig_1_1 */
 
+/* Not generating wrapper methods for PPB_AudioFrame_0_1 */
+
 /* Begin wrapper methods for PPB_Console_1_0 */
 
 static void Pnacl_M25_PPB_Console_Log(PP_Instance instance, PP_LogLevel level,
struct PP_Var* value) {
@@ -916,6 +921,45 @@ static void
Pnacl_M13_PPB_IMEInputEvent_GetSelection(PP_Resource ime_event, uint
 
 /* Not generating wrapper methods for PPB_Instance_1_0 */
 
+/* Begin wrapper methods for PPB_MediaStreamAudioTrack_0_1 */
+
+static PP_Bool
Pnacl_M34_PPB_MediaStreamAudioTrack_IsMediaStreamAudioTrack(PP_Resource
resource) {
+  const struct PPB_MediaStreamAudioTrack_0_1 *iface =
Pnacl_WrapperInfo_PPB_MediaStreamAudioTrack_0_1.real_iface;
+  return iface->IsMediaStreamAudioTrack(resource);
+}
+
+static int32_t Pnacl_M34_PPB_MediaStreamAudioTrack_Configure(PP_Resource
audio_track, uint32_t samples_per_frame, uint32_t max_buffered_frames) {
+  const struct PPB_MediaStreamAudioTrack_0_1 *iface =
Pnacl_WrapperInfo_PPB_MediaStreamAudioTrack_0_1.real_iface;
+  return iface->Configure(audio_track, samples_per_frame, max_buffered_frames);
+}
+
+static void Pnacl_M34_PPB_MediaStreamAudioTrack_GetId(struct PP_Var*
_struct_result, PP_Resource audio_track) {
+  const struct PPB_MediaStreamAudioTrack_0_1 *iface =
Pnacl_WrapperInfo_PPB_MediaStreamAudioTrack_0_1.real_iface;
+  *_struct_result = iface->GetId(audio_track);
+}
+
+static PP_Bool Pnacl_M34_PPB_MediaStreamAudioTrack_HasEnded(PP_Resource
audio_track) {
+  const struct PPB_MediaStreamAudioTrack_0_1 *iface =
Pnacl_WrapperInfo_PPB_MediaStreamAudioTrack_0_1.real_iface;
+  return iface->HasEnded(audio_track);
+}
+
+static int32_t Pnacl_M34_PPB_MediaStreamAudioTrack_GetFrame(PP_Resource
audio_track, PP_Resource* frame, struct PP_CompletionCallback* callback) {
+  const struct PPB_MediaStreamAudioTrack_0_1 *iface =
Pnacl_WrapperInfo_PPB_MediaStreamAudioTrack_0_1.real_iface;
+  return iface->GetFrame(audio_track, frame, *callback);
+}
+
+static int32_t Pnacl_M34_PPB_MediaStreamAudioTrack_RecycleFrame(PP_Resource
audio_track, PP_Resource frame) {
+  const struct PPB_MediaStreamAudioTrack_0_1 *iface =
Pnacl_WrapperInfo_PPB_MediaStreamAudioTrack_0_1.real_iface;
+  return iface->RecycleFrame(audio_track, frame);
+}
+
+static void Pnacl_M34_PPB_MediaStreamAudioTrack_Close(PP_Resource audio_track)
{
+  const struct PPB_MediaStreamAudioTrack_0_1 *iface =
Pnacl_WrapperInfo_PPB_MediaStreamAudioTrack_0_1.real_iface;
+  iface->Close(audio_track);
+}
+
+/* End wrapper methods for PPB_MediaStreamAudioTrack_0_1 */
+
 /* Begin wrapper methods for PPB_MediaStreamVideoTrack_0_1 */
 
 static PP_Bool
Pnacl_M34_PPB_MediaStreamVideoTrack_IsMediaStreamVideoTrack(PP_Resource
resource) {
@@ -4031,6 +4075,8 @@ static int32_t
Pnacl_M29_PPB_Ext_Socket_Dev_GetJoinedGroups(PP_Instance instance
 
 /* Not generating wrapper interface for PPB_AudioConfig_1_1 */
 
+/* Not generating wrapper interface for PPB_AudioFrame_0_1 */
+
 struct PPB_Console_1_0 Pnacl_Wrappers_PPB_Console_1_0 = {
     .Log = (void (*)(PP_Instance instance, PP_LogLevel level, struct PP_Var
value))&Pnacl_M25_PPB_Console_Log,
     .LogWithSource = (void (*)(PP_Instance instance, PP_LogLevel level, struct
PP_Var source, struct PP_Var value))&Pnacl_M25_PPB_Console_LogWithSource
@@ -4210,6 +4256,16 @@ struct PPB_IMEInputEvent_1_0
Pnacl_Wrappers_PPB_IMEInputEvent_1_0 = {
 
 /* Not generating wrapper interface for PPB_Instance_1_0 */
 
+struct PPB_MediaStreamAudioTrack_0_1
Pnacl_Wrappers_PPB_MediaStreamAudioTrack_0_1 = {
+    .IsMediaStreamAudioTrack = (PP_Bool (*)(PP_Resource
resource))&Pnacl_M34_PPB_MediaStreamAudioTrack_IsMediaStreamAudioTrack,
+    .Configure = (int32_t (*)(PP_Resource audio_track, uint32_t
samples_per_frame, uint32_t
max_buffered_frames))&Pnacl_M34_PPB_MediaStreamAudioTrack_Configure,
+    .GetId = (struct PP_Var (*)(PP_Resource
audio_track))&Pnacl_M34_PPB_MediaStreamAudioTrack_GetId,
+    .HasEnded = (PP_Bool (*)(PP_Resource
audio_track))&Pnacl_M34_PPB_MediaStreamAudioTrack_HasEnded,
+    .GetFrame = (int32_t (*)(PP_Resource audio_track, PP_Resource* frame,
struct PP_CompletionCallback
callback))&Pnacl_M34_PPB_MediaStreamAudioTrack_GetFrame,
+    .RecycleFrame = (int32_t (*)(PP_Resource audio_track, PP_Resource
frame))&Pnacl_M34_PPB_MediaStreamAudioTrack_RecycleFrame,
+    .Close = (void (*)(PP_Resource
audio_track))&Pnacl_M34_PPB_MediaStreamAudioTrack_Close
+};
+
 struct PPB_MediaStreamVideoTrack_0_1
Pnacl_Wrappers_PPB_MediaStreamVideoTrack_0_1 = {
     .IsMediaStreamVideoTrack = (PP_Bool (*)(PP_Resource
resource))&Pnacl_M34_PPB_MediaStreamVideoTrack_IsMediaStreamVideoTrack,
     .Configure = (int32_t (*)(PP_Resource video_track, uint32_t
max_buffered_frames))&Pnacl_M34_PPB_MediaStreamVideoTrack_Configure,
@@ -5190,6 +5246,12 @@ static struct __PnaclWrapperInfo
Pnacl_WrapperInfo_PPB_IMEInputEvent_1_0 = {
   .real_iface = NULL
 };
 
+static struct __PnaclWrapperInfo
Pnacl_WrapperInfo_PPB_MediaStreamAudioTrack_0_1 = {
+  .iface_macro = PPB_MEDIASTREAMAUDIOTRACK_INTERFACE_0_1,
+  .wrapped_iface = (void *) &Pnacl_Wrappers_PPB_MediaStreamAudioTrack_0_1,
+  .real_iface = NULL
+};
+
 static struct __PnaclWrapperInfo
Pnacl_WrapperInfo_PPB_MediaStreamVideoTrack_0_1 = {
   .iface_macro = PPB_MEDIASTREAMVIDEOTRACK_INTERFACE_0_1,
   .wrapped_iface = (void *) &Pnacl_Wrappers_PPB_MediaStreamVideoTrack_0_1,
@@ -5706,6 +5768,7 @@ static struct __PnaclWrapperInfo *s_ppb_wrappers[] = {
   &Pnacl_WrapperInfo_PPB_KeyboardInputEvent_1_0,
   &Pnacl_WrapperInfo_PPB_TouchInputEvent_1_0,
   &Pnacl_WrapperInfo_PPB_IMEInputEvent_1_0,
+  &Pnacl_WrapperInfo_PPB_MediaStreamAudioTrack_0_1,
   &Pnacl_WrapperInfo_PPB_MediaStreamVideoTrack_0_1,
   &Pnacl_WrapperInfo_PPB_MessageLoop_1_0,
   &Pnacl_WrapperInfo_PPB_Messaging_1_0,

[commit-bot: I haz the power, 2014-01-09 20:22:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/penghuang@chromium.org/126373003/420001

[Peng, 2014-01-09 20:59:57]

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_media_stre...
File ppapi/api/ppb_media_stream_audio_track.idl (right):

https://codereview.chromium.org/126373003/diff/30001/ppapi/api/ppb_media_stre...
ppapi/api/ppb_media_stream_audio_track.idl:17: interface
PPB_MediaStreamAudioTrack {
On 2014/01/09 17:07:46, dmichael wrote:
> On 2014/01/07 22:29:27, Peng wrote:
> > I asked yuzhu, the dev API can be changed. So I landed it. I thought it is
> > easier to fix and review it in a separate CL. Sorry for it.
> No, that's totally fine. We can come back around and change it still.
> 
>  
> > When I was implementing the video track, I found it is very easy to combine
> > producer and consumer to a single Track interface. We just need add a
Create()
> > method to create produce track.
> > 
> > So there are two solutions.
> > 1. Single interface MediaStreamAudioTrack for both input and output
> > Pro:
> >  * It saves a lot of code.
> >  * HTML5 only has MediaStreamTrack object. Single interface is more familiar
> for
> > web developers.
> > Con:
> >  * Maybe it is a little confusing if the API is not well documented.
> In this world, would a plugin ever be able to use the same
MediaStreamVideoTrack
> to read _and_ write tracks? If so we *definitely* should not separate them. If
> not, it feels slightly wrong to have them in the same API. Not sure. Maybe
> adding a "read only" field later on will help with that confusion.

Right now, I can not image how a plugin side track can be used both for both
read and write. Adding a "read only" field should help a lot.

> 
> Another thing we have to think about is the interaction with JavaScript. Since
> we want to be able to pass these back and forth using
> PPB_Messaging/PPP_Messaging, it might be best to have a 1-1 mapping. So that
> would argue for having 1 type for VideoStreamTrack:
>
http://dev.w3.org/2011/webrtc/editor/getusermedia.html#idl-def-VideoStreamTrack
> ...and 1 type for AudioStreamTrack...
>
http://dev.w3.org/2011/webrtc/editor/getusermedia.html#idl-def-AudioStreamTrack
> 
> (In fact, I wonder if PPB_AudioStreamTrack and PPB_VideoStreamTrack are better
> names, to match the HTML spec?)

I agree. JS NaCL 1-1 mapping is better. So AudioStreamTrack and VideoStreamTrack
are better names. I will rename them later before moving them to stable channel.

> 
> > 
> > 2. Two interfaces MediaStreamAudioTrackSource and
> > MediaStreamAudioTrackDestination.
> > 
> > 
> > I am slight prefer solution 1. What do you think?
> I think given that it matches better with HTML/JavaScript, I think I agree
with
> you. We'll have to keep an eye out for that when you add pushing frames to
that
> APIs, because the current file-level documentation doesn't reflect that.

[commit-bot: I haz the power, 2014-01-09 22:55:20]

Change committed as 244009