Mac VideoCapture: Support for Blackmagic DeckLink device & capabilities enumeration, using SDK.

Mac VideoCapture: Support for Blackmagic DeckLink device & capabilities enumeration, using SDK.

This CL adds a new class VideoCaptureDeviceDeckLinkMac with
two static methods EnumerateDevices() and
EnumerateDeviceCapabilities(). This class also has some
empty methods to comply with VCD interface.

DeckLink uses ScopedComPtr style APIs, so an internal
template class is provided with the needed bits for automatic
reference counting inside VCDDeckLinkMac.

Static methods for enumeration are used from VCDFactoryMac.

Support for Blackmagic SDK as a third party is discussed in
http://crbug.com/408534 and is necessary prerequisite
for landing this CL. This SDK landing is under review in
http://crrev.com/509313002.

Other CL(s) will follow to use DeckLink devices for capture.

BUG=408493, 408534
Committed: https://crrev.com/9b46fe6874291e1e50c9230b720293932dea6231
Cr-Commit-Position: refs/heads/master@{#295952}

[mcasas, 2014-08-29 14:20:06]

Patchset #1 (id:1) has been deleted

[mcasas, 2014-08-29 14:23:19]

Patchset #1 (id:20001) has been deleted

[mcasas, 2014-09-01 07:15:50]

mcasas@chromium.org changed reviewers:
+ magjed@chromium.org, perkj@chromium.org

[mcasas, 2014-09-01 07:15:51]

perkj@, magjed@ PTAL.

You can safely ignore any Obj-C++ parts

[perkj_chrome, 2014-09-01 09:50:41]

I don't understand how you select between AVFOUNDATION and DECLINK? 

What if you have a build in camera or a normal USB camera and a DECLINK you want
to choose between ?

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:98: != S_OK) {
nit try to keep  != on the line with the first argument you compare with. 

if (decklink_input->GetDisplayModeIterator(
    display_mode_iter.Receive()) != S_OK) {

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:134:
scoped_ptr<VideoCaptureDevice::Client> client) {}
add NOTIMPLEMENTED

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:136: void
VideoCaptureDeviceDeckLinkMac::StopAndDeAllocate() {}
add NOTIMPLEMENTED

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_factory_mac.mm (right):

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_factory_mac.mm:163:
VideoCaptureDeviceDeckLinkMac::EnumerateDevices(device_names);
Isn't there a risk that we enumerate BM devices twice here? 
Will the AVFoundation enumeration above not list them at all on any OS version?

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_factory_mac.mm:211:
DCHECK_EQ(device.capture_api_type(), VideoCaptureDevice::Name::DECKLINK);
How do we now that |device| is Declink?

[magjed_chromium, 2014-09-01 10:41:13]

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:31: if (ptr_ !=
NULL) {
Sometimes you compare with NULL, sometimes you don't. I'm not sure what the
style guide dictates, but I guess you should be consistent?

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:66:
device_names->push_back(name);
Should we really add this device even if GetModelName and/or GetDisplayName
failed?

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:118: DVLOG(2) <<
device.name() << " resolution: "
I have seen similar code like this in at least four other places. Maybe we
should add a convenience function
std::ostream& operator<<(std::ostream&, const media::VideoCaptureFormat&)
and replace the duplicated code?

[mcasas, 2014-09-01 14:04:50]

guys PTAL.

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:31: if (ptr_ !=
NULL) {
On 2014/09/01 10:41:13, magjed wrote:
> Sometimes you compare with NULL, sometimes you don't. I'm not sure what the
> style guide dictates, but I guess you should be consistent?

This is copy-paste from [1] and I wanted to leave it as close to the 
original as possible. Makes sense?

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/base/win/scoped_co...

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:66:
device_names->push_back(name);
On 2014/09/01 10:41:13, magjed wrote:
> Should we really add this device even if GetModelName and/or GetDisplayName
> failed?

Done.

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:98: != S_OK) {
On 2014/09/01 09:50:41, perkj wrote:
> nit try to keep  != on the line with the first argument you compare with. 
> 
> if (decklink_input->GetDisplayModeIterator(
>     display_mode_iter.Receive()) != S_OK) {
> 

Done.

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:118: DVLOG(2) <<
device.name() << " resolution: "
On 2014/09/01 10:41:13, magjed wrote:
> I have seen similar code like this in at least four other places. Maybe we
> should add a convenience function
> std::ostream& operator<<(std::ostream&, const media::VideoCaptureFormat&)
> and replace the duplicated code?

Probably. http://crbug.com/409636

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:134:
scoped_ptr<VideoCaptureDevice::Client> client) {}
On 2014/09/01 09:50:41, perkj wrote:
> add NOTIMPLEMENTED

Done.

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:136: void
VideoCaptureDeviceDeckLinkMac::StopAndDeAllocate() {}
On 2014/09/01 09:50:41, perkj wrote:
> add NOTIMPLEMENTED

Done.

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_factory_mac.mm (right):

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_factory_mac.mm:163:
VideoCaptureDeviceDeckLinkMac::EnumerateDevices(device_names);
On 2014/09/01 09:50:41, perkj wrote:
> Isn't there a risk that we enumerate BM devices twice here? 
> Will the AVFoundation enumeration above not list them at all on any OS
version?

Short answer: _not_ in 64bit builds.

Long answer: could happen in 32b builds (disappearing now), but still 
Blackmagic-enumerated would have different names to AVFoundation ones.

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_factory_mac.mm:211:
DCHECK_EQ(device.capture_api_type(), VideoCaptureDevice::Name::DECKLINK);
On 2014/09/01 09:50:41, perkj wrote:
> How do we now that |device| is Declink?

Missed this one, should check in l.210.

[perkj_chrome, 2014-09-01 14:44:08]

lgtm if the below is addressed.

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:104:
ScopedDeckLinkPtr<IDeckLinkDisplayMode> displayMode;
name display_mode

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:108:
media::VideoPixelFormat pixelFormat = media::PIXEL_FORMAT_UNKNOWN;
dito pixel_format

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_factory_mac.mm (right):

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_factory_mac.mm:190: if
(device.capture_api_type() == VideoCaptureDevice::Name::AVFOUNDATION) {
nit switch(device.capture_api_type()) ?

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_factory_mac.mm:215: DCHECK(false);
NOTREACHED()

[magjed_chromium, 2014-09-01 14:44:37]

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:31: if (ptr_ !=
NULL) {
On 2014/09/01 14:04:49, mcasas wrote:
> On 2014/09/01 10:41:13, magjed wrote:
> > Sometimes you compare with NULL, sometimes you don't. I'm not sure what the
> > style guide dictates, but I guess you should be consistent?
> 
> This is copy-paste from [1] and I wanted to leave it as close to the 
> original as possible. Makes sense?
> 
> [1]
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/win/scoped_co...

Ok, but you do it in the rest of the code as well, e.g.
DLOG_IF(ERROR, decklink_iter != NULL)
and
if (!decklink_iter)

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:47: DLOG_IF(ERROR,
decklink_iter != NULL) << "Error creating DeckLink iterator";
This looks wrong, shouldn't it be "decklink_iter == NULL" instead of
"decklink_iter != NULL"?
Also, you might want to change to "!decklink_iter" to be consistent with the
next line.

https://codereview.chromium.org/518073002/diff/60001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_decklink_mac.mm:78: DLOG_IF(ERROR,
decklink_iter != NULL) << "Error creating DeckLink iterator";
Same as above.

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:68:
device_names->push_back(name);
You must move this line into the scope above?

[mcasas, 2014-09-02 12:45:53]

mcasas@chromium.org changed reviewers:
+ rsesek@chromium.org, vrk@chromium.org

[mcasas, 2014-09-02 12:45:54]

vrk@: media/media.gyp

rsesek@ PTAL at our dearest Obj-C++ parts

magjed@ comments addressed, PTAL.

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:68:
device_names->push_back(name);
On 2014/09/01 14:44:37, magjed wrote:
> You must move this line into the scope above?

Done.

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:104:
ScopedDeckLinkPtr<IDeckLinkDisplayMode> displayMode;
On 2014/09/01 14:44:08, perkj wrote:
> name display_mode

Done.

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:108:
media::VideoPixelFormat pixelFormat = media::PIXEL_FORMAT_UNKNOWN;
On 2014/09/01 14:44:08, perkj wrote:
> dito pixel_format

Done.

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_factory_mac.mm (right):

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_factory_mac.mm:190: if
(device.capture_api_type() == VideoCaptureDevice::Name::AVFOUNDATION) {
On 2014/09/01 14:44:08, perkj wrote:
> nit switch(device.capture_api_type()) ?

Done.

https://codereview.chromium.org/518073002/diff/110001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_factory_mac.mm:215: DCHECK(false);
On 2014/09/01 14:44:08, perkj wrote:
> NOTREACHED()

Done.

[magjed_chromium, 2014-09-02 12:59:20]

lgtm

[Robert Sesek, 2014-09-02 18:50:50]

https://codereview.chromium.org/518073002/diff/170001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/170001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:60:
[(NSString*)device_display_name UTF8String];
SysCFStringRefToUTF8

https://codereview.chromium.org/518073002/diff/170001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:63:
VideoCaptureDevice::Name name([(NSString*)device_model_name UTF8String],
SysCFStringRefToUTF8

https://codereview.chromium.org/518073002/diff/170001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:64:
[(NSString*)device_display_name UTF8String],
SysCFStringRefToUTF8

https://codereview.chromium.org/518073002/diff/170001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:88: if
(device.id().compare([(NSString*)device_model_name UTF8String]) != 0)
SysCFStringRefToUTF8

https://codereview.chromium.org/518073002/diff/170001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:137:
NOTIMPLEMENTED();
Are these going to need to be implemented? What happens now if a user selects a
DeckLink-based device?

[mcasas, 2014-09-03 09:04:41]

mcasas@chromium.org changed reviewers:
+ dalecurtis@chromium.org
- vrk@chromium.org

[mcasas, 2014-09-03 09:04:42]

rsesek@ PTAL.

-vrk@ +dalecurtis@: media/media.gyp

https://codereview.chromium.org/518073002/diff/170001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/170001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:60:
[(NSString*)device_display_name UTF8String];
On 2014/09/02 18:50:50, rsesek wrote:
> SysCFStringRefToUTF8

Done.

https://codereview.chromium.org/518073002/diff/170001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:63:
VideoCaptureDevice::Name name([(NSString*)device_model_name UTF8String],
On 2014/09/02 18:50:50, rsesek wrote:
> SysCFStringRefToUTF8

Done.

https://codereview.chromium.org/518073002/diff/170001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:64:
[(NSString*)device_display_name UTF8String],
On 2014/09/02 18:50:50, rsesek wrote:
> SysCFStringRefToUTF8

Done.

https://codereview.chromium.org/518073002/diff/170001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:88: if
(device.id().compare([(NSString*)device_model_name UTF8String]) != 0)
On 2014/09/02 18:50:50, rsesek wrote:
> SysCFStringRefToUTF8

Done.

https://codereview.chromium.org/518073002/diff/170001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:137:
NOTIMPLEMENTED();
On 2014/09/02 18:50:50, rsesek wrote:
> Are these going to need to be implemented? What happens now if a user selects
a
> DeckLink-based device?

Yes, there is a follow up CL implementing these two (I got the code
working locally).

If the user selects a DecLink device before all code is in place, she
would  hit a NOTIMPLEMENTED(), which is a warning in Release 
Builds IINM. The video capture device would be created nontheless
producing no frame, so the user would see a black screen and the
JS MediaStreamTrack onMute() event would fire up.

[Robert Sesek, 2014-09-03 17:51:29]

https://codereview.chromium.org/518073002/diff/190001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/518073002/diff/190001/media/media.gyp#newcode872
media/media.gyp:872: '../third_party/decklink/decklink.gyp:decklink',
Has this gone through proper third_party review for OSS and security?

https://codereview.chromium.org/518073002/diff/190001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.h (right):

https://codereview.chromium.org/518073002/diff/190001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.h:21: static void
EnumerateDevices(VideoCaptureDevice::Names* device_names);
Public members need documentation.

https://codereview.chromium.org/518073002/diff/190001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/190001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:127:
decklink.Release();
Can't you return here?

[DaleCurtis, 2014-09-03 20:56:25]

not lgtm until decklink issues are resolved.

https://codereview.chromium.org/518073002/diff/190001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/518073002/diff/190001/media/media.gyp#newcode500
media/media.gyp:500: 'video/capture/mac/video_capture_device_decklink_mac.h',
Update build.gn too please.

https://codereview.chromium.org/518073002/diff/190001/media/media.gyp#newcode872
media/media.gyp:872: '../third_party/decklink/decklink.gyp:decklink',
On 2014/09/03 17:51:29, rsesek wrote:
> Has this gone through proper third_party review for OSS and security?

+1

[mcasas, 2014-09-04 10:00:37]

rsesek@, dalecurtis@ PTAL.

https://codereview.chromium.org/518073002/diff/190001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/518073002/diff/190001/media/media.gyp#newcode500
media/media.gyp:500: 'video/capture/mac/video_capture_device_decklink_mac.h',
On 2014/09/03 20:56:24, DaleCurtis wrote:
> Update build.gn too please.

Done.

https://codereview.chromium.org/518073002/diff/190001/media/media.gyp#newcode872
media/media.gyp:872: '../third_party/decklink/decklink.gyp:decklink',
On 2014/09/03 20:56:24, DaleCurtis wrote:
> On 2014/09/03 17:51:29, rsesek wrote:
> > Has this gone through proper third_party review for OSS and security?
> 
> +1

rsesek@, dalecurtis@: http://crbug.com/408534 is dealing with
the necessary third_party process, including security and licensing
review. This CL is a follow up to that bug's http://crrev.com/509313002.

The plan is to get the CLs depending on that 3rd party approved.
Apologies if this was not clear in the CR description, will update.

https://codereview.chromium.org/518073002/diff/190001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.h (right):

https://codereview.chromium.org/518073002/diff/190001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.h:21: static void
EnumerateDevices(VideoCaptureDevice::Names* device_names);
On 2014/09/03 17:51:29, rsesek wrote:
> Public members need documentation.

Done.

https://codereview.chromium.org/518073002/diff/190001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/190001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:127:
decklink.Release();
On 2014/09/03 17:51:29, rsesek wrote:
> Can't you return here?

Yes, good catch.

https://codereview.chromium.org/518073002/diff/210001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/210001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:90:
decklink.Release();
Gosh we totally missed this one :'(

Lurking bug, since the practical totality of BlackMagic setups have
only one connected device, so this code path would have hardly 
ever been taken.

[Robert Sesek, 2014-09-04 14:25:01]

https://codereview.chromium.org/518073002/diff/210001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.h (right):

https://codereview.chromium.org/518073002/diff/210001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.h:24: // Gets the
supported formats of a particular device attached to the system,
nit: blank line before

https://codereview.chromium.org/518073002/diff/210001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/210001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:104: continue;
Do you need to declink.Release() here, too? Would it make sense to release
declink into a while-loop-local ScopedDeckLinkPtr, so that these early returns
and continues will be cleaned up automatically?

[mcasas, 2014-09-04 15:58:30]

Patchset #6 (id:230001) has been deleted

[mcasas, 2014-09-04 15:59:21]

rsesek@, dalecurtis@ PTAL.

https://codereview.chromium.org/518073002/diff/210001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.h (right):

https://codereview.chromium.org/518073002/diff/210001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.h:24: // Gets the
supported formats of a particular device attached to the system,
On 2014/09/04 14:25:01, rsesek wrote:
> nit: blank line before

Done.

https://codereview.chromium.org/518073002/diff/210001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/210001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:90:
decklink.Release();
On 2014/09/04 10:00:36, mcasas wrote:
> Gosh we totally missed this one :'(
> 
> Lurking bug, since the practical totality of BlackMagic setups have
> only one connected device, so this code path would have hardly 
> ever been taken.

Acknowledged.

https://codereview.chromium.org/518073002/diff/210001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:104: continue;
On 2014/09/04 14:25:01, rsesek wrote:
> Do you need to declink.Release() here, too? Would it make sense to release
> declink into a while-loop-local ScopedDeckLinkPtr, so that these early returns
> and continues will be cleaned up automatically?

Done.

[DaleCurtis, 2014-09-04 23:07:35]

media.gyp/gn changes lgtm

[mcasas, 2014-09-12 13:50:22]

perkj@, magjed@, rsesek@ PTAL.

There is a change @ VideoCaptureDeviceDeckLinkMac, 
concretely EnumerateDevices() and 
EnumerateDeviceCapabilities(): instead of enumerating
all N capabilities for device X, each capability is presented
separated, so we get N devices X with a capability suffix,
e.g.:
UltraStudio Mini Recorder - HD 1080p 30
UltraStudio Mini Recorder - NTSC 
etc.

[perkj_chrome, 2014-09-15 09:13:17]

Looks good to me. But approval from an mm expert would be nice.

https://codereview.chromium.org/518073002/diff/290001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/290001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:54: // there are no
Blackmagic DeckLink devices in the system, don't print error.
What do you mean by" don't print error." ?

https://codereview.chromium.org/518073002/diff/290001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:123:
decklink_input.ReceiveVoid()) != S_OK) {
4 more spaces right?

https://codereview.chromium.org/518073002/diff/290001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:141: if
(display_mode->GetName(&format_name) == S_OK && device.id() !=
nit : prever to move device.id() != to the next line for readability.

[Robert Sesek, 2014-09-18 14:30:19]

LGTM

[mcasas, 2014-09-22 08:39:09]

https://codereview.chromium.org/518073002/diff/290001/media/video/capture/mac...
File media/video/capture/mac/video_capture_device_decklink_mac.mm (right):

https://codereview.chromium.org/518073002/diff/290001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:54: // there are no
Blackmagic DeckLink devices in the system, don't print error.
On 2014/09/15 09:13:16, perkj wrote:
> What do you mean by" don't print error." ?

Rewritten. What I meant is that l.55 is a DVLOG_IF() and not
an DPLOG(ERROR, ...)

https://codereview.chromium.org/518073002/diff/290001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:123:
decklink_input.ReceiveVoid()) != S_OK) {
On 2014/09/15 09:13:16, perkj wrote:
> 4 more spaces right? 

Done.

https://codereview.chromium.org/518073002/diff/290001/media/video/capture/mac...
media/video/capture/mac/video_capture_device_decklink_mac.mm:141: if
(display_mode->GetName(&format_name) == S_OK && device.id() !=
On 2014/09/15 09:13:16, perkj wrote:
> nit : prever to move device.id() != to the next line for readability.

Would love to but then I'd have to break the next line and gets even
more confusing :(

[mcasas, 2014-09-22 08:39:35]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2014-09-22 08:39:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/518073002/310001

[commit-bot: I haz the power, 2014-09-22 08:49:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-22 08:49:31]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/58894)

[mcasas, 2014-09-22 11:36:12]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2014-09-22 11:36:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/518073002/330001

[commit-bot: I haz the power, 2014-09-22 12:28:50]

Committed patchset #10 (id:330001) as 5ed1920476faf004b7036da4f8b20bdea84de73f

[commit-bot: I haz the power, 2014-09-22 12:29:35]

Patchset 10 (id:??) landed as
https://crrev.com/9b46fe6874291e1e50c9230b720293932dea6231
Cr-Commit-Position: refs/heads/master@{#295952}