New Pepper interfaces for audio/video capture device enumeration.

New Pepper interfaces for audio/video capture device enumeration.

BUG=None
TEST=None

[yzshen1, 2011-12-17 00:58:50]

Hi, Brett and Trung.

Would you please take a look at these interface definitions?

I am not planning to submit this CL before I write the implementation. I just
want to make sure that I am heading in the right direction.

Thanks!

[viettrungluu, 2011-12-17 01:37:02]

It mostly looks like what I imagined, except for the notes below....

http://codereview.chromium.org/8981009/diff/2001/ppapi/api/dev/ppb_stream_dev...
File ppapi/api/dev/ppb_stream_device_enumerator_dev.idl (right):

http://codereview.chromium.org/8981009/diff/2001/ppapi/api/dev/ppb_stream_dev...
ppapi/api/dev/ppb_stream_device_enumerator_dev.idl:7: * This file defines the
<code>PPB_StreamDeviceEnumerator_Dev</code> interface.
Ditto -- nothing stream-specific.

http://codereview.chromium.org/8981009/diff/2001/ppapi/api/dev/ppb_stream_dev...
ppapi/api/dev/ppb_stream_device_enumerator_dev.idl:59: uint32_t
GetDeviceNumber([in] PP_Resource enumerator);
I wonder if this is overkill, and we shouldn't just have a "GetNextDevice()" (or
whatever), as in the file chooser.

(Or maybe the file chooser should be more like this.)

http://codereview.chromium.org/8981009/diff/2001/ppapi/api/dev/ppb_stream_dev...
File ppapi/api/dev/ppb_stream_device_ref_dev.idl (right):

http://codereview.chromium.org/8981009/diff/2001/ppapi/api/dev/ppb_stream_dev...
ppapi/api/dev/ppb_stream_device_ref_dev.idl:7: * This file defines the
<code>PPB_StreamDeviceRef_Dev</code> interface.
There's very little stream-specific about this.

We may eventually want to enumerate other kinds of devices as well.

http://codereview.chromium.org/8981009/diff/2001/ppapi/api/dev/ppb_stream_dev...
ppapi/api/dev/ppb_stream_device_ref_dev.idl:14: * Media stream types.
I would call these "device types".

[yzshen1, 2011-12-19 19:11:54]

Thanks, Trung!

http://codereview.chromium.org/8981009/diff/2001/ppapi/api/dev/ppb_stream_dev...
File ppapi/api/dev/ppb_stream_device_enumerator_dev.idl (right):

http://codereview.chromium.org/8981009/diff/2001/ppapi/api/dev/ppb_stream_dev...
ppapi/api/dev/ppb_stream_device_enumerator_dev.idl:7: * This file defines the
<code>PPB_StreamDeviceEnumerator_Dev</code> interface.
On 2011/12/17 01:37:03, viettrungluu wrote:
> Ditto -- nothing stream-specific.

Done.

http://codereview.chromium.org/8981009/diff/2001/ppapi/api/dev/ppb_stream_dev...
ppapi/api/dev/ppb_stream_device_enumerator_dev.idl:59: uint32_t
GetDeviceNumber([in] PP_Resource enumerator);
I think this may be easier for plugins. For example, it may want to reserve
resources in advance.

What do you think?

On 2011/12/17 01:37:03, viettrungluu wrote:
> I wonder if this is overkill, and we shouldn't just have a "GetNextDevice()"
(or
> whatever), as in the file chooser.
> 
> (Or maybe the file chooser should be more like this.)

http://codereview.chromium.org/8981009/diff/2001/ppapi/api/dev/ppb_stream_dev...
File ppapi/api/dev/ppb_stream_device_ref_dev.idl (right):

http://codereview.chromium.org/8981009/diff/2001/ppapi/api/dev/ppb_stream_dev...
ppapi/api/dev/ppb_stream_device_ref_dev.idl:7: * This file defines the
<code>PPB_StreamDeviceRef_Dev</code> interface.
Thanks! Good idea!
On 2011/12/17 01:37:03, viettrungluu wrote:
> There's very little stream-specific about this.
> 
> We may eventually want to enumerate other kinds of devices as well.

http://codereview.chromium.org/8981009/diff/2001/ppapi/api/dev/ppb_stream_dev...
ppapi/api/dev/ppb_stream_device_ref_dev.idl:14: * Media stream types.
On 2011/12/17 01:37:03, viettrungluu wrote:
> I would call these "device types".

Done.

[brettw, 2011-12-19 22:46:38]

I'm curious how this will be presented in C++. In particular, how would we
return the correct wrapper object type for GetDeviceNumber? Options:

Here's a slightly tweaked design that may also answer Trung's question on how
this overlaps with the file ref. We could have a "resource enumerator" object
that's used for all of these cases.

In this case, the file chooser interface would return a new resource enumeration
resource that returns FileRefs. The audio device interface would have a function
that returns a resource enumeration resource that creates audio input devices.
Etc. Since there would be a per-type constructor, it allows us to specify extra
parameters as needed (what type of files to enumerate, maybe in the future you
could specify the minimum capabilities of audio devices to return).

In this case, there would be a templatized C++ wrapper around the enumeration
resource so we could create the correct wrapper types:
  typedef ResourceEnumerator<FileRef> FileRefEnumerator;
  FileRefEnumerator fileChooser::GetFiles() const;

Interesting question: do we still have an enum for the enumeration type like you
do? I'm thinking yes, but another approach would be do have an IsFooEnumerator
on the interface that created the enumerator whether it's an enumerator for your
type.

I think I prefer Get[Device|Resource]Count over "Number".

[yzshen1, 2011-12-20 00:40:07]

On 2011/12/19 22:46:38, brettw wrote:
> I'm curious how this will be presented in C++. In particular, how would we
> return the correct wrapper object type for GetDeviceNumber? Options:
> 
> Here's a slightly tweaked design that may also answer Trung's question on how
> this overlaps with the file ref. We could have a "resource enumerator" object
> that's used for all of these cases.
> 
> In this case, the file chooser interface would return a new resource
enumeration
> resource that returns FileRefs. The audio device interface would have a
function
> that returns a resource enumeration resource that creates audio input devices.
> Etc. Since there would be a per-type constructor, it allows us to specify
extra
> parameters as needed (what type of files to enumerate, maybe in the future you
> could specify the minimum capabilities of audio devices to return).
> 
> In this case, there would be a templatized C++ wrapper around the enumeration
> resource so we could create the correct wrapper types:
>   typedef ResourceEnumerator<FileRef> FileRefEnumerator;
>   FileRefEnumerator fileChooser::GetFiles() const;
> 
> Interesting question: do we still have an enum for the enumeration type like
you
> do? I'm thinking yes, but another approach would be do have an IsFooEnumerator
> on the interface that created the enumerator whether it's an enumerator for
your
> type.
> 
> I think I prefer Get[Device|Resource]Count over "Number".

[I am not trying to argue that what I have written is better -- it is not. :)
Just want to make sure I understand your point and discuss another approach.]

Thanks for your suggestions!

Which approach do you mean?
= approach_1 ==============================================
  typedef ResourceEnumerator<FileRef> FileRefEnumerator;
  int32_t FileChooser::GetFiles(CompletionCallback callback);
  FileRefEnumerator FileChooser::GetReturnedFiles() const;

  FileRef FileRefEnumerator::Get(uint32_t index);
===========================================================
or
= approach_2 ==============================================
  typedef ResourceEnumerator<FileRef> FileRefEnumerator;
  FileRefEnumerator FileChooser::GetFiles() const;

  int32_t FileRefEnumerator::Enumerate(CompletionCallback callback);  //
Actually get those files.
  FileRef FileRefEnumerator::Get(uint32_t index);
===========================================================


I realized that if we extend PP_Var to support variable-length array (there is
already PP_VARTYPE_ARRAY defined), we can do:
= approach_3 ==============================================
  int32_t GetFiles(PP_Resource chooser, PP_CompletionCallback callback);
  PP_Var GetReturnedFiles(PP_Resource chooser);

and C++ wrappers:
  int32_t FileChooser::GetFiles(CompletionCallback callback);
  void FileChooser::GetReturnedFiles(std::vector<FileRef>* files);
===========================================================

It seems approach_3 is more intuitive for users.

Approach_2 seems to move an async operation to ResourceEnumerator and make its
creators (FileChooser, AudioInput, etc.) simpler. However, a general resource
enumerator cannot do |Enumerate| by itself. It needs to delegate that operation
to whoever creates it. So the internal implementation of approach_2 won't be
simpler than approach_3.

What do you think?

[brettw, 2011-12-20 23:00:20]

On 2011/12/20 00:40:07, yzshen1 wrote:
> On 2011/12/19 22:46:38, brettw wrote:
> > I'm curious how this will be presented in C++. In particular, how would we
> > return the correct wrapper object type for GetDeviceNumber? Options:
> > 
> > Here's a slightly tweaked design that may also answer Trung's question on
how
> > this overlaps with the file ref. We could have a "resource enumerator"
object
> > that's used for all of these cases.
> > 
> > In this case, the file chooser interface would return a new resource
> enumeration
> > resource that returns FileRefs. The audio device interface would have a
> function
> > that returns a resource enumeration resource that creates audio input
devices.
> > Etc. Since there would be a per-type constructor, it allows us to specify
> extra
> > parameters as needed (what type of files to enumerate, maybe in the future
you
> > could specify the minimum capabilities of audio devices to return).
> > 
> > In this case, there would be a templatized C++ wrapper around the
enumeration
> > resource so we could create the correct wrapper types:
> >   typedef ResourceEnumerator<FileRef> FileRefEnumerator;
> >   FileRefEnumerator fileChooser::GetFiles() const;
> > 
> > Interesting question: do we still have an enum for the enumeration type like
> you
> > do? I'm thinking yes, but another approach would be do have an
IsFooEnumerator
> > on the interface that created the enumerator whether it's an enumerator for
> your
> > type.
> > 
> > I think I prefer Get[Device|Resource]Count over "Number".
> 
> [I am not trying to argue that what I have written is better -- it is not. :)
> Just want to make sure I understand your point and discuss another approach.]
> 
> Thanks for your suggestions!
> 
> Which approach do you mean?
> = approach_1 ==============================================
>   typedef ResourceEnumerator<FileRef> FileRefEnumerator;
>   int32_t FileChooser::GetFiles(CompletionCallback callback);
>   FileRefEnumerator FileChooser::GetReturnedFiles() const;
> 
>   FileRef FileRefEnumerator::Get(uint32_t index);
> ===========================================================
> or
> = approach_2 ==============================================
>   typedef ResourceEnumerator<FileRef> FileRefEnumerator;
>   FileRefEnumerator FileChooser::GetFiles() const;
> 
>   int32_t FileRefEnumerator::Enumerate(CompletionCallback callback);  //
> Actually get those files.
>   FileRef FileRefEnumerator::Get(uint32_t index);
> ===========================================================
> 
> 
> I realized that if we extend PP_Var to support variable-length array (there is
> already PP_VARTYPE_ARRAY defined), we can do:
> = approach_3 ==============================================
>   int32_t GetFiles(PP_Resource chooser, PP_CompletionCallback callback);
>   PP_Var GetReturnedFiles(PP_Resource chooser);
> 
> and C++ wrappers:
>   int32_t FileChooser::GetFiles(CompletionCallback callback);
>   void FileChooser::GetReturnedFiles(std::vector<FileRef>* files);
> ===========================================================
> 
> It seems approach_3 is more intuitive for users.
> 
> Approach_2 seems to move an async operation to ResourceEnumerator and make its
> creators (FileChooser, AudioInput, etc.) simpler. However, a general resource
> enumerator cannot do |Enumerate| by itself. It needs to delegate that
operation
> to whoever creates it. So the internal implementation of approach_2 won't be
> simpler than approach_3.

Vars and resources don't really interact. A var really corresponds to a JS
variable, which is unrelated to a resource. So I don't think we would support
Var arrays of PP_Resources. This also gets tricky with the refcounting since the
Var refcounting is different than the resource refcounting.

For the File chooser, I think I was thinking more along the lines of your #1.

Do you think we should call it a ResourceArray instead of ResourceEnumerator?

I just also realized that we could even skip the C++ FileRefEnumerator and just
return a std::vector<FileRef>. This might be easier and more obvious to use.
Then we just need an internal helper function to convert a ResourceArray to a
std::vector.

Sorry, I don't want to delay this patch too much with bikeshedding. Let me know
if you think this is too hard. You shouldn't try converting the file chooser in
the same patch, either.

[yzshen1, 2011-12-21 00:04:05]

> Vars and resources don't really interact. A var really corresponds to a JS
> variable, which is unrelated to a resource. So I don't think we would support
> Var arrays of PP_Resources. This also gets tricky with the refcounting since
the
> Var refcounting is different than the resource refcounting.

Ah, I forgot that. :) Thanks for pointing out.

> 
> For the File chooser, I think I was thinking more along the lines of your #1.
> 
> Do you think we should call it a ResourceArray instead of ResourceEnumerator?

Yes. I think that is easier to understand for users.

> 
> I just also realized that we could even skip the C++ FileRefEnumerator and
just
> return a std::vector<FileRef>. This might be easier and more obvious to use.
> Then we just need an internal helper function to convert a ResourceArray to a
> std::vector.

Yes.

> 
> Sorry, I don't want to delay this patch too much with bikeshedding. Let me
know
> if you think this is too hard. You shouldn't try converting the file chooser
in
> the same patch, either.

I would like to make the change and upload a new patch. I don't think it is too
hard.
And I will leave file chooser as it is for now.

Thanks for your suggestion!