Make VideoCaptureController single-threaded and not ref counted.

Make VideoCaptureController single-threaded and not ref counted. 
Merge Allocate+Start, Stop+DeAllocate. 

Move the non-IO thread parts of VideoCaptureController, which were 
precisely the VideoCaptureDevice::EventHandler implementation, into a 
new class VCC::VideoCaptureDeviceClient, which retains only a WeakPtr 
to the VCC. 

Change the contract between VideoCaptureDevice and its client so that 
(1) The Start+Allocate, and Stop+DeAllocate phases of VCD are merged, 
and (2) the VCD owns its EventHandler*; which is to say, the VCD owns 
the VideoCaptureDeviceClient. 

The merging of the phases is justified because it will eliminate 
intermediate states from the 6+ implementations of the 
VideoCaptureDevices. 

The new VCD::EH ownership model is justified because it will enable 
some of the impls (MFWin, desktop capture, and tab capture, at least) 
to eliminate locks protecting the EventHandler* from being 
synchronously removed at the DeAllocate() step. 

Rather than make deep changes to each VCD implementation with this 
change, the interface changes to VideoCaptureDevice are effected 
through a new shim class, VideoCaptureDevice1, which bridges the old 
and new interfaces. Follow-on changes will eliminate this shim class, 
one VideoCaptureDevice implementation at a time. 

BUG=285562, 285569
TEST=unittests,basic webcam interaction

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

[ncarter (slow), 2013-09-13 18:13:23]

This is ready for review now.

[Ami GONE FROM CHROMIUM, 2013-09-13 21:17:59]

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.cc:119: return
result.Pass();
l.117-119 are equiv to
return make_scoped_ptr(new VideoCaptureDeviceClient(this->AsWeakPtr())));

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.cc:367: if
(!buffer_pool_)
There's a crbug to not need to defend against this, right?
(TODO-point at it here?)

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.cc:440: //
OnIncomingCapturedVideoFrame is called the thread running the capture device.
english

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_controller.h (left):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.h:153: //
TODO(nick): Make it so that this lock isn't required.
You made it so!

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_controller.h (right):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.h:75:
base::WeakPtr<VideoCaptureController> AsWeakPtr();
"As" connotes SupportsWeakPtr, which is generally meh.  Being part of the public
API makes it meh-er.
I mildly prefer GetWeakPtr().  Use your judgement.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.h:114: class
VideoCaptureDeviceClient
Could this move into the .cc file with merely a fwd decl here?

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.h:193: bool
frame_info_available_;
VideoCaptureCapability's default ctor zero-initializes fields that guarantee its
IsValid() method will return false until stuff is set explicitly, so I think you
can probably drop this member in favor of using IsValid().

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_host.cc (right):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_host.cc:17: struct
VideoCaptureHost::Entry {
What's the point of this struct now? 
(feels like the answer is either "oops, no longer necessary" or "shhh!  I'm
doing something icky with managing weakptrs across threads!  Don't look too
closely!"; I'm hoping the answer is the former :))

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_host.h (right):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_host.h:101:
base::WeakPtr<VideoCaptureController> controller);
I guess all these base::WeakPtr<VideoCaptureController>'s are implicitly about
the IO thread, since VCC is so IOthread-ish now.  I just wish there was a way to
annotate them as such in headers.
(nothing actionable for you here unless you can think of a way to effect this
annotation cleverly)

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_manager.cc (right):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_manager.cc:142:
scoped_ptr<media::VideoCaptureDevice::EventHandler> device_client) {
IWBN if someday VCD::EH was named VCD::Client.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_manager.h (left):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_manager.h:142:
media::VideoCaptureDevice::EventHandler* controller_as_handler);
Amen.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_manager.h (right):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_manager.h:140: // the device.
s/device/device pointed to by |entry|/ ?

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
File media/video/capture/video_capture_device.h (right):

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
media/video/capture/video_capture_device.h:223: //     remain valid even after
the device is stopped. Whereas old style devices
s/remain/to remain/

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
media/video/capture/video_capture_device.h:224: //     may not dereference their
handlers after DeAllocate().
I don't follow.  AFAICT from the commentary above (and the use of
scoped_ptr<EventHandler> in AllocateAndStart()), handlers must be assumed to be
deleted once SADA() returns (b/c they may be deleted asynchronously, but the
caller can't know when).  What am I missing?

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
media/video/capture/video_capture_device.h:240: // Gets the names of all video
capture devices
stray?

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
media/video/capture/video_capture_device.h:266: scoped_ptr<EventHandler>
client_;
This thing is known as a "Handler" (by the type), a "client" (by the variable
name), and as an "observer" (by the parameter name to AllocateAndStart()).  IMO
"client" is the best name, but hopefully whatever you like best can be a single
thing, some day.

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
File media/video/capture/video_capture_device_dummy.h (left):

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
media/video/capture/video_capture_device_dummy.h:20: class
VideoCaptureDeviceDummy : public VideoCaptureDevice {
lolol

[ncarter (slow), 2013-09-14 00:07:23]

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.cc:119: return
result.Pass();
On 2013/09/13 21:17:59, Ami Fischman wrote:
> l.117-119 are equiv to
> return make_scoped_ptr(new VideoCaptureDeviceClient(this->AsWeakPtr())));

Actually, clang throws a hissy fit on that. Because we're casting to the
interface type, I'd have to use
PassAs<media::VideoCaptureDevice::EventHandler>() or a
static_cast<media::VideoCaptureDevice::EventHandler> around the operator new. I
thought this was more readable that those other options.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.cc:367: if
(!buffer_pool_)
On 2013/09/13 21:17:59, Ami Fischman wrote:
> There's a crbug to not need to defend against this, right?
> (TODO-point at it here?)

Yes. I've added the TODO to a different place in the file, where we create and
share the buffer pool.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.cc:440: //
OnIncomingCapturedVideoFrame is called the thread running the capture device.
On 2013/09/13 21:17:59, Ami Fischman wrote:
> english

Deleted and elaborated the class comment.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_controller.h (left):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.h:153: //
TODO(nick): Make it so that this lock isn't required.
On 2013/09/13 21:17:59, Ami Fischman wrote:
> You made it so!

Done.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_controller.h (right):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.h:75:
base::WeakPtr<VideoCaptureController> AsWeakPtr();
On 2013/09/13 21:17:59, Ami Fischman wrote:
> "As" connotes SupportsWeakPtr, which is generally meh.  Being part of the
public
> API makes it meh-er.
> I mildly prefer GetWeakPtr().  Use your judgement.

Done. Having thought about it, I agree with you.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.h:114: class
VideoCaptureDeviceClient
On 2013/09/13 21:17:59, Ami Fischman wrote:
> Could this move into the .cc file with merely a fwd decl here?

Sure. Done. FWIW, I put it here in the header for "theoretical unit test"
reasons as well as for hard-to-universalize "I like to grep for
member_variable_declarations_ in .h files" reasons. But once we move the
color-conversion responsibility to the Rigs, this class will just be a fairly
dumb proxy, probably not worth unit testing alone or splitting into its own
file.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.h:193: bool
frame_info_available_;
On 2013/09/13 21:17:59, Ami Fischman wrote:
> VideoCaptureCapability's default ctor zero-initializes fields that guarantee
its
> IsValid() method will return false until stuff is set explicitly, so I think
you
> can probably drop this member in favor of using IsValid().

Neato. Done.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_host.cc (right):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_host.cc:17: struct
VideoCaptureHost::Entry {
On 2013/09/13 21:17:59, Ami Fischman wrote:
> What's the point of this struct now? 
> (feels like the answer is either "oops, no longer necessary" or "shhh!  I'm
> doing something icky with managing weakptrs across threads!  Don't look too
> closely!"; I'm hoping the answer is the former :))

What was the point of it before?

The point of this struct is to give you and me jobs. As to the point of that,
it's a harder question to answer.

Nuked it from space.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_host.h (right):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_host.h:101:
base::WeakPtr<VideoCaptureController> controller);
On 2013/09/13 21:17:59, Ami Fischman wrote:
> I guess all these base::WeakPtr<VideoCaptureController>'s are implicitly about
> the IO thread, since VCC is so IOthread-ish now.  I just wish there was a way
to
> annotate them as such in headers.
> (nothing actionable for you here unless you can think of a way to effect this
> annotation cleverly)

renamed to handler_as_controller. Just kidding; but seriously I know what you
mean. My preferred fix here is actually to stop using the VCC pointer as the
registration token altogether, the whole process by which the VCH/VCC become
mutually aware is kind of weird actually. The old code did a
make_scoped_refptr<> on a bare pointer, so I'm still counting the use of weakptr
here as a slight improvement, is how I sleep at night.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_manager.cc (right):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_manager.cc:142:
scoped_ptr<media::VideoCaptureDevice::EventHandler> device_client) {
On 2013/09/13 21:17:59, Ami Fischman wrote:
> IWBN if someday VCD::EH was named VCD::Client.

It's on my list, but I've been de-prioritizing it since it's a pain from a
patch-juggling perspective. I'll try to send you something over the weekend.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_manager.h (left):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_manager.h:142:
media::VideoCaptureDevice::EventHandler* controller_as_handler);
On 2013/09/13 21:17:59, Ami Fischman wrote:
> Amen.

Done.

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_manager.h (right):

https://codereview.chromium.org/24133002/diff/69001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_manager.h:140: // the device.
On 2013/09/13 21:17:59, Ami Fischman wrote:
> s/device/device pointed to by |entry|/ ?

I meant the device mentioned in the previous sentence.

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
File media/video/capture/video_capture_device.h (right):

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
media/video/capture/video_capture_device.h:223: //     remain valid even after
the device is stopped. Whereas old style devices
On 2013/09/13 21:17:59, Ami Fischman wrote:
> s/remain/to remain/

Done.

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
media/video/capture/video_capture_device.h:224: //     may not dereference their
handlers after DeAllocate().
On 2013/09/13 21:17:59, Ami Fischman wrote:
> I don't follow.  AFAICT from the commentary above (and the use of
> scoped_ptr<EventHandler> in AllocateAndStart()), handlers must be assumed to
be
> deleted once SADA() returns (b/c they may be deleted asynchronously, but the
> caller can't know when).  What am I missing?

I think I figured out what was missing. I made the comment on new interface more
explicit about the required behavior if delete is async.

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
media/video/capture/video_capture_device.h:240: // Gets the names of all video
capture devices
On 2013/09/13 21:17:59, Ami Fischman wrote:
> stray?

Done.

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
media/video/capture/video_capture_device.h:266: scoped_ptr<EventHandler>
client_;
On 2013/09/13 21:17:59, Ami Fischman wrote:
> This thing is known as a "Handler" (by the type), a "client" (by the variable
> name), and as an "observer" (by the parameter name to AllocateAndStart()). 
IMO
> "client" is the best name, but hopefully whatever you like best can be a
single
> thing, some day.

"Client" is right term and it might have been a mistake for me to not go ahead
and rename the EventHandler interface already. I removed "observer" from this
context, that was an oversight.

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
File media/video/capture/video_capture_device_dummy.h (left):

https://codereview.chromium.org/24133002/diff/69001/media/video/capture/video...
media/video/capture/video_capture_device_dummy.h:20: class
VideoCaptureDeviceDummy : public VideoCaptureDevice {
On 2013/09/13 21:17:59, Ami Fischman wrote:
> lolol

Done.

[Ami GONE FROM CHROMIUM, 2013-09-14 00:32:23]

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.cc:103: //
v4l2_thread on Linux, and the UI thread for the tab capture.
s/the tab/tab/

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_host.h (right):

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_host.h:156: // it is
connected. An entry in this map temporarily has holds a null
has holds

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_manager.h (right):

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_manager.h:38: void(const
base::WeakPtr<VideoCaptureController>&)> DoneCB;
"DoneCB"?  Oh, so your "callback" signifies being "done"?
</youdontsay>

Not that I have a fantastic alternate suggestion :(

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_manager.h:80: DoneCB done_cb);
Shouldn't callbacks be passed by const&?

https://codereview.chromium.org/24133002/diff/95001/media/video/capture/video...
File media/video/capture/video_capture_device.h (right):

https://codereview.chromium.org/24133002/diff/95001/media/video/capture/video...
media/video/capture/video_capture_device.h:214: // valid but enters a lame state
where it is detached from any consumer, and
I don't understand how this can be useful.  Two paragraphs up you promise
eventual deletion, which presumably may happen inline during
StopAndDeAllocate(), or it may not.
But here you promise *client "remains valid".  Until when?  How can a caller
know how much time *client has remaining to live?

[ncarter (slow), 2013-09-14 01:29:30]

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.cc:103: //
v4l2_thread on Linux, and the UI thread for the tab capture.
On 2013/09/14 00:32:24, Ami Fischman wrote:
> s/the tab/tab/

Done.

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_host.h (right):

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_host.h:156: // it is
connected. An entry in this map temporarily has holds a null
On 2013/09/14 00:32:24, Ami Fischman wrote:
> has holds

Done.

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_manager.h (right):

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_manager.h:38: void(const
base::WeakPtr<VideoCaptureController>&)> DoneCB;
On 2013/09/14 00:32:24, Ami Fischman wrote:
> "DoneCB"?  Oh, so your "callback" signifies being "done"?
> </youdontsay>
> 
> Not that I have a fantastic alternate suggestion :(

One must pick one's battles.

https://codereview.chromium.org/24133002/diff/95001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_manager.h:80: DoneCB done_cb);
On 2013/09/14 00:32:24, Ami Fischman wrote:
> Shouldn't callbacks be passed by const&?

Done.

https://codereview.chromium.org/24133002/diff/95001/media/video/capture/video...
File media/video/capture/video_capture_device.h (right):

https://codereview.chromium.org/24133002/diff/95001/media/video/capture/video...
media/video/capture/video_capture_device.h:214: // valid but enters a lame state
where it is detached from any consumer, and
On 2013/09/14 00:32:24, Ami Fischman wrote:
> I don't understand how this can be useful.  Two paragraphs up you promise
> eventual deletion, which presumably may happen inline during
> StopAndDeAllocate(), or it may not.
> But here you promise *client "remains valid".  Until when?  How can a caller
> know how much time *client has remaining to live?

It remains valid in that it's not illegal for stuff you've queued up to interact
with it. But those interactions won't do anything. The VCD owns the pointer, so
it can delete it when it wants. What I'm trying to say is -- "it's like a
weakptr". The current language seems clear to me. It's owned by the device, but
it's detached from the downstream pipe.

I just deleted this paragraph since it seems to be causing more confusion than
elucidation. My goal was to communicate to implementors, "you don't need to be
paranoid about not making calls into the EventHandler if you've already queued
some up somewhere."

[Ami GONE FROM CHROMIUM, 2013-09-14 02:16:29]

lgtm

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2013-09-14 22:11:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nick@chromium.org/24133002/110001

[commit-bot: I haz the power, 2013-09-14 22:18:07]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...

[commit-bot: I haz the power, 2013-09-15 21:24:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nick@chromium.org/24133002/83001

[commit-bot: I haz the power, 2013-09-16 03:05:29]

Change committed as 223307

[perkj_chrome, 2013-09-16 08:50:15]

Nice- please also see the discussion in
https://codereview.chromium.org/24079003/

https://codereview.chromium.org/24133002/diff/83001/media/video/capture/video...
File media/video/capture/video_capture_device.h (right):

https://codereview.chromium.org/24133002/diff/83001/media/video/capture/video...
media/video/capture/video_capture_device.h:123: virtual ~EventHandler() {}
Why make this public? Do you really want to allow the implementor of this class
to be deleted through the EventHandler interface?

[ncarter (slow), 2013-09-16 15:18:17]

https://codereview.chromium.org/24133002/diff/83001/media/video/capture/video...
File media/video/capture/video_capture_device.h (right):

https://codereview.chromium.org/24133002/diff/83001/media/video/capture/video...
media/video/capture/video_capture_device.h:123: virtual ~EventHandler() {}
On 2013/09/16 08:50:15, perkj wrote:
> Why make this public? Do you really want to allow the implementor of this
class
> to be deleted through the EventHandler interface?

Yes, that's the design. With this new change, the EventHandler is a
VideoCaptureController::VideoCaptureDeviceClient, which is a proxy to the
Controller, and owned by the Device. There is a comment in the changelist
description that justifies this decision.

[wjia(left Chromium), 2013-09-18 01:29:01]

https://codereview.chromium.org/24133002/diff/83001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

https://codereview.chromium.org/24133002/diff/83001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.cc:145: :
state_(VIDEO_CAPTURE_STATE_STARTED),
|state_| is not really needed any more since there are only 2 states: valid and
error. A boolean is good enough, e.g., has_error_.

https://codereview.chromium.org/24133002/diff/83001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.cc:671:
reserved_frame->shared_memory_handle());
In case number of live clients (i.e., the |count| below) is zero, does it make
sense to reserve a buffer?

https://codereview.chromium.org/24133002/diff/83001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.cc:700: // Allocate
memory only when device has been started.
This would be changed to "Allocate memory only when device has no error.".