[PPAPI] API definition for video media stream artifacts

ppapi/api/ppb_media_stream_video_track.idl

Index: ppapi/api/ppb_media_stream_video_track.idl
diff --git a/ppapi/api/ppb_media_stream_video_track.idl b/ppapi/api/ppb_media_stream_video_track.idl
new file mode 100644
index 0000000000000000000000000000000000000000..e085f3da447694c381c6f121bc7d9c7329c5a77e
--- /dev/null
+++ b/ppapi/api/ppb_media_stream_video_track.idl
@@ -0,0 +1,115 @@
+/* Copyright (c) 2013 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.
+ */
+
+/**
+ * Defines the <code>PPB_MediaStreamVideoTrack</code> interface. Used for
+ * receiving video frames from a MediaStream video track in the browser.

[dmichael (off chromium), 2013/12/20 17:42:52]

What's the plan for pushing frames to a track? Will that eventually go in this
API, or will that be a separate one? I ask mainly because the name you have here
seems to imply it will do both (or it will be hard to name the one for pushing
frames).

[Peng, 2013/12/20 18:49:17]

For pushing frames, we will have a separate one.
How about using MediaStreamVideoSource / MediaStreamVideoTrackSource? I feel
MSVTS is too long. :( And if we use *VideoSource, I think it is better rename
VideoProducer to VideoSink or VideoDestination as well.Thought?

BTW, I have another idea. MediaStreamVideoTrack and MediaStreamAudioTrack are
almost same. The only different is the |Configure()| function. I think we can
merge them to one interface MediaStreamTrackSource. And provide two configure
functions (ConfigureVideo() and ConfigureAudio()), and add GetType() function
for querying source type. It could save some code. And same idea for
VideoProducer/AudioProducer. Thought?

[Peng, 2013/12/27 21:24:14]

As I discussed with Yuzhu. I am going to land this CL first, and then we could
discuss the class name and rename it in a separate CL.

+ * This interface is still in development (Dev API status) and may change.
+ */
+label Chrome {
+  M33 = 0.1

[yzshen1, 2013/12/20 17:52:26]

I think we can now use the [channel=dev] tag, because Justin has landed the
support:
https://code.google.com/p/chromium/issues/detail?id=325403

[Peng, 2013/12/20 20:55:31]

Done.

+};
+
+/**
+ */
+interface PPB_MediaStreamVideoTrack {
+  /**
+   * Determines if a resource is a MediaStream video 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>
+   * otherwise.
+   */
+  PP_Bool IsMediaStreamVideoTrack([in]PP_Resource resource);

[dmichael (off chromium), 2013/12/20 17:42:52]

style/consistency nit: please put a space between [in] and the parameter type
(here and elsewhere)

[Peng, 2013/12/20 20:55:31]

Done.

+
+  /**
+   * Configure underlayer frame buffers for incoming frames. The provided

[dmichael (off chromium), 2013/12/20 17:42:52]

underlayer->underlying?

[yzshen1, 2013/12/20 17:52:26]

- Configure*s*
- underlayer -> underlying? 

[Peng, 2013/12/20 20:55:31]

Done.

[Peng, 2013/12/20 20:55:31]

Done.

+   * |frame_buffer_size| is provided in terms of the number of frames to use for

[yzshen1, 2013/12/20 17:52:26]

Other methods use @param[in] @return tag, please make them consistent.

[Peng, 2013/12/20 20:55:31]

Done.

+   * incoming frames. Should not be less than 1. If the application doesn't want
+   * to drop frames, then the |frame_buffer_size| should be chosen such that
+   * inter-frame processing time variability won't overrun the ring buffer.

[yzshen1, 2013/12/20 17:52:26]

Nit: no need to mention 'ring' buffer. That is an implementation detail.

[Peng, 2013/12/20 20:55:31]

Done.

+   * If the buffer is overfilled, then frames will be dropped. The application
+   * can detect this by examining the timestamp on returned frames.
+   * If |Configure()| is not used, default settings will be used.
+   */
+  int32_t Configure([in]PP_Resource resource, [in]uint32_t frame_buffer_size);

[dmichael (off chromium), 2013/12/20 17:42:52]

Here and in methods other than "IsMediaStreamVideoTrack", we would usually use a
more specific name than |resource| to imply that it must be a
MediaStreamVideoTrack resource. "video_track" is probably sufficient.

[Peng, 2013/12/20 20:55:31]

Done.

+
+  /**
+   * Returns the track ID of the underlying MediaStream video track.
+   *
+   * @param[in] resource The <code>PP_Resource</code> to check.
+   *
+   * @return A <code>PP_Var</code> containing the MediaStream track ID as
+   * a string.
+   */
+  PP_Var GetId([in]PP_Resource resource);

[yzshen1, 2013/12/20 17:52:26]

Except the IsMediaStreamVideoTrack() method, please use
|media_stream_video_track| as parameter name instead of |resource|. (here and
below)

[Peng, 2013/12/20 20:55:31]

Can I use video_track? media_stream_video_track is too long. Is using a full
name for resource a strict rule in pepper API style?

Done

[yzshen1, 2013/12/20 23:37:43]

Sounds good.

+
+  /**
+   * 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] resource 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([in]PP_Resource resource);
+
+  /**
+   * Gets the next video 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 |ReuseFrame()| is called to free a spot for
+   * another frame to be buffered.

[yzshen1, 2013/12/20 17:52:26]

It seems nice to also comment that once the input buffer is full, further
GetFrame() calls won't complete until ReuseFrame() is called to free up buffer
slots.

[Peng, 2013/12/20 20:55:31]

To make think easy. I plan to return error, if the caller holds a frame got
remove previous |GetFrame()| call. So the call must recycle frames, before using
this function.

Done

[yzshen1, 2013/12/20 23:37:43]

I think you should also comment what would happen if the producer side is too
slow and currently there is no contents in the buffer, I think the completion
callback won't return until a frame arrives, right?

[yzshen1, 2013/12/26 17:52:21]

In case you didn't notice this comment...

+   *
+   * @param[in] resource A <code>PP_Resource</code> corresponding to a video
+   * resource.
+   * @param[out] frame A <code>PP_Resource</code> corresponding to a VideoFrame
+   * resource.
+   * @param[in] callback A <code>PP_CompletionCallback</code> to be called upon
+   * completion of GetFrame().
+   *
+   * @return An int32_t containing a result code from <code>pp_errors.h</code>.
+   * Returns PP_ERROR_BADRESOURCE if resource isn't a valid video source.

[dmichael (off chromium), 2013/12/20 17:42:52]

(I don't think you need to list PP_ERROR_BADRESOURCE explicitly)

[Peng, 2013/12/20 20:55:31]

Done.

+   * Returns PP_ERROR_NOMEMORY if |frame_buffer_size| frames buffer was not
+   * allocated successfully.
+   * Returns PP_ERROR_FAILED if the source is not open, or if some other
+   * browser error occurs.

[dmichael (off chromium), 2013/12/20 17:42:52]

(Ditto for PP_ERROR_FAILED; these are pretty common and used consistently, so
referencing pp_errors.h is sufficient. Same applies to other places you list
these two.)

[Peng, 2013/12/20 20:55:31]

Done.

+   */
+  int32_t GetFrame([in] PP_Resource resource,
+                   [out] PP_Resource frame,
+                   [in] PP_CompletionCallback callback);
+
+  /**
+   * Release a frame got from |GetFrame()|, so the track can reuse it for

[yzshen1, 2013/12/20 17:52:26]

Release*s*

[Peng, 2013/12/20 20:55:31]

Changed to Recycles

+   * new frames.Gets the next video frame from the MediaStream track.

[dmichael (off chromium), 2013/12/20 17:42:52]

I don't think you want the last sentence here...

Also, it would be good to mention that the given frame is invalid after this
call.

[yzshen1, 2013/12/20 17:52:26]

- One space before "Gets", please.

- Please talk about "the application shouldn't use |frame| anymore and should
release references it holds to |frame|.

[Peng, 2013/12/20 20:55:31]

Done.

[Peng, 2013/12/20 20:55:31]

Done.

+   *
+   * @param[in] resource A <code>PP_Resource</code> corresponding to a video
+   * resource.
+   * @param[in] frame A <code>PP_Resource</code> corresponding to a VideoFrame
+   * resource got from |GetFrame()|
+   *
+   * @return An int32_t containing a result code from <code>pp_errors.h</code>.
+   * Returns PP_ERROR_BADRESOURCE if resource isn't a valid video source.
+   * Retruns PP_ERROR_BADARGUMENT if frame isn't a vaild video frame got from
+   * |GetFrame()|.
+   * Returns PP_ERROR_FAILED if the source is not open, or if some other
+   * browser error occurs.
+   */
+  int32_t ReuseFrame([in] PP_Resource resource,

[dmichael (off chromium), 2013/12/20 17:42:52]

I think we might be able to come up with a better name. "ReuseFrame" seems to
imply the frame is going to get used for something as a result of this call,
when really it's just telling the track that we're done with it.
One possible alternative is "RecycleFrame".

[Peng, 2013/12/20 20:55:31]

Done.

+                     [in] PP_Resource frame);
+
+  /**
+   * Closes the MediaStream video track.

[yzshen1, 2013/12/20 17:52:26]

Please talk about whether it is allowed to do Configure() / GetFrame() / etc.
after Close(). (I think we probably don't need to support re-Configure(),
right?)

[Peng, 2013/12/20 20:55:31]

Done.

+   *
+   * @param[in] source A <code>PP_Resource</code> corresponding to a
+   * MediaStream video track resource.
+   */
+  void Close([in] PP_Resource source);
+};
+