Mac Video Capture: Add Blackmagic DeckLink SDK for Mac to third_party folder

Mac Video Capture: Add Blackmagic DeckLink SDK for Mac to third_party folder

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

[mcasas, 2014-08-28 09:40:52]

Patchset #1 (id:1) has been deleted

[mcasas, 2014-08-28 09:47:44]

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

[mcasas, 2014-08-28 09:47:44]

tommi@, magjed@ please take a _first_ glance at the files:

- LICENSE 
- OWNERS 
- README.chromium 
- decklink_mac.gyp

Please note that the license type is not mentioned to be of a standard
category.

[mcasas, 2014-08-28 09:53:55]

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

[mcasas, 2014-08-28 09:53:55]

perkj@ PTAL -- same comments as in the previous message.

[perkj_chrome, 2014-08-28 11:35:44]

Not all cc files are used in the gyp files. Should they still be included?
Is there a risk we want to add declink sdks on other platforms? Shouldn't the
structure then be third_party/declink/mac, third_party/declink/win etc..

https://codereview.chromium.org/509313002/diff/20001/third_party/decklink_mac...
File third_party/decklink_mac/include/DeckLinkAPIDispatch_v7_6.cpp (right):

https://codereview.chromium.org/509313002/diff/20001/third_party/decklink_mac...
third_party/decklink_mac/include/DeckLinkAPIDispatch_v7_6.cpp:32: #define
kDeckLinkAPI_BundlePath "/Library/Frameworks/DeckLinkAPI.framework"
This file is not used in the gyp file. Should it still be included?

[mcasas, 2014-08-29 09:56:05]

On 2014/08/28 11:35:44, perkj wrote:
> Not all cc files are used in the gyp files. Should they still be included?

I'd say yes, so we add in pristine/verbatim form whatever comes out of 
the decklink .zip, notwithstanding the info in README.chromium.

> Is there a risk we want to add declink sdks on other platforms? Shouldn't the
> structure then be third_party/declink/mac, third_party/declink/win etc..

Discussed offline, done.

> 
>
https://codereview.chromium.org/509313002/diff/20001/third_party/decklink_mac...
> File third_party/decklink_mac/include/DeckLinkAPIDispatch_v7_6.cpp (right):
> 
>
https://codereview.chromium.org/509313002/diff/20001/third_party/decklink_mac...
> third_party/decklink_mac/include/DeckLinkAPIDispatch_v7_6.cpp:32: #define
> kDeckLinkAPI_BundlePath "/Library/Frameworks/DeckLinkAPI.framework"
> This file is not used in the gyp file. Should it still be included?

See above.

[magjed_chromium, 2014-09-02 15:26:57]

lgtm as far as I can tell. 
The files contain tabs etc, do we want to keep them as close as possible to the
originals, or should we use clang-format?

[tommi (sloooow) - chröme, 2014-09-02 18:28:44]

On 2014/09/02 15:26:57, magjed wrote:
> lgtm as far as I can tell. 
> The files contain tabs etc, do we want to keep them as close as possible to
the
> originals, or should we use clang-format?

Let's keep them in their original form.

[mcasas, 2014-09-05 08:54:02]

mcasas@chromium.org changed reviewers:
+ darin@chromium.org, phajdan.jr@chromium.org

[mcasas, 2014-09-05 08:54:03]

phajdan.jr@chromium.org: Please review changes in 
tools/checklicenses/checklicenses.py

darin@chromium.org: PTAL.
NOTE: crbug.com/408534 deals with the appropriate
permissions/security/etc to land this code, but to speed
up the process, we can review the form of the code.

[Paweł Hajdan Jr., 2014-09-08 09:53:31]

Sorry, "not lgtm", see comments below.

https://codereview.chromium.org/509313002/diff/60001/third_party/decklink/REA...
File third_party/decklink/README.chromium (right):

https://codereview.chromium.org/509313002/diff/60001/third_party/decklink/REA...
third_party/decklink/README.chromium:4: License: *** TO BE DETERMINED ***
I don't think this can be checked in.

https://codereview.chromium.org/509313002/diff/60001/third_party/decklink/REA...
third_party/decklink/README.chromium:8: DeckLink SDK is a proprietary library to
interface and manipulate Blackmagic
It's not obvious whether we can check proprietary library into the public repo.

https://codereview.chromium.org/509313002/diff/60001/tools/checklicenses/chec...
File tools/checklicenses/checklicenses.py (right):

https://codereview.chromium.org/509313002/diff/60001/tools/checklicenses/chec...
tools/checklicenses/checklicenses.py:179: 'third_party/decklink': [  #
http://crbug.com/408534
This needs to be an *upstream* bug report about adding per-file license headers.

Note that files I see *do* have license headers, so could you make
licensecheck.pl recognize them? It's just writing a regex...

[tommi (sloooow) - chröme, 2014-09-08 17:04:51]

https://codereview.chromium.org/509313002/diff/60001/third_party/decklink/REA...
File third_party/decklink/README.chromium (right):

https://codereview.chromium.org/509313002/diff/60001/third_party/decklink/REA...
third_party/decklink/README.chromium:4: License: *** TO BE DETERMINED ***
On 2014/09/08 09:53:31, Paweł Hajdan Jr. wrote:
> I don't think this can be checked in.

I think this is the Boost Software License.
http://www.boost.org/LICENSE_1_0.txt

https://codereview.chromium.org/509313002/diff/60001/third_party/decklink/REA...
third_party/decklink/README.chromium:8: DeckLink SDK is a proprietary library to
interface and manipulate Blackmagic
On 2014/09/08 09:53:31, Paweł Hajdan Jr. wrote:
> It's not obvious whether we can check proprietary library into the public
repo.

There's a launch bug for this (crbug.com/408534) that you can take a look at and
it has been reviewed by our open source lawyer.  If there's anything missing it
would be good to know.

[mcasas, 2014-09-08 21:16:10]

pawel, darin@, PTAL.

https://codereview.chromium.org/509313002/diff/60001/tools/checklicenses/chec...
File tools/checklicenses/checklicenses.py (right):

https://codereview.chromium.org/509313002/diff/60001/tools/checklicenses/chec...
tools/checklicenses/checklicenses.py:179: 'third_party/decklink': [  #
http://crbug.com/408534
On 2014/09/08 09:53:31, Paweł Hajdan Jr. wrote:
> This needs to be an *upstream* bug report about adding per-file license
headers.
> 
> Note that files I see *do* have license headers, so could you make
> licensecheck.pl recognize them? It's just writing a regex...

Done in licensecheck.pl, thanks!

[Paweł Hajdan Jr., 2014-09-09 12:31:36]

https://codereview.chromium.org/509313002/diff/60001/third_party/decklink/REA...
File third_party/decklink/README.chromium (right):

https://codereview.chromium.org/509313002/diff/60001/third_party/decklink/REA...
third_party/decklink/README.chromium:8: DeckLink SDK is a proprietary library to
interface and manipulate Blackmagic
On 2014/09/08 17:04:51, tommi wrote:
> On 2014/09/08 09:53:31, Paweł Hajdan Jr. wrote:
> > It's not obvious whether we can check proprietary library into the public
> repo.
> 
> There's a launch bug for this (crbug.com/408534) that you can take a look at
and
> it has been reviewed by our open source lawyer.  If there's anything missing
it
> would be good to know.

I see no public evidence that this has been reviewed by OSPO. I'd be fine with
some flag flip on the bug, but couldn't find that either.

I can still see mention of "proprietary library" in this file, and that's what
I'm concerned about.

https://codereview.chromium.org/509313002/diff/80001/third_party/devscripts/l...
File third_party/devscripts/licensecheck.pl (left):

https://codereview.chromium.org/509313002/diff/80001/third_party/devscripts/l...
third_party/devscripts/licensecheck.pl:599: if ($licensetext =~ /Permission is
hereby granted, free of charge, to any person or organization obtaining a copy
of the software and accompanying documentation covered by this license \(the
\"Software\"\)/ or
This is likely to regress other cases where quotes are used. Can you make it
handle both cases, quotes and no quotes? I'm fine with just making each of the
\" optional.

[mcasas, 2014-09-09 15:27:35]

Patchset #5 (id:100001) has been deleted

[mcasas, 2014-09-09 15:31:50]

Pawel, PTAL

I suggest we try to round up the form of this CL and
get in parallel the necessary OSS approval.

https://codereview.chromium.org/509313002/diff/60001/third_party/decklink/REA...
File third_party/decklink/README.chromium (right):

https://codereview.chromium.org/509313002/diff/60001/third_party/decklink/REA...
third_party/decklink/README.chromium:8: DeckLink SDK is a proprietary library to
interface and manipulate Blackmagic
On 2014/09/09 12:31:36, Paweł Hajdan Jr. wrote:
> On 2014/09/08 17:04:51, tommi wrote:
> > On 2014/09/08 09:53:31, Paweł Hajdan Jr. wrote:
> > > It's not obvious whether we can check proprietary library into the public
> > repo.
> > 
> > There's a launch bug for this (crbug.com/408534) that you can take a look at
> and
> > it has been reviewed by our open source lawyer.  If there's anything missing
> it
> > would be good to know.
> 
> I see no public evidence that this has been reviewed by OSPO. I'd be fine with
> some flag flip on the bug, but couldn't find that either.
> 
> I can still see mention of "proprietary library" in this file, and that's what
> I'm concerned about.

I changed it to quote verbatim what BlackMagic says in 
the SDK documentation.

https://codereview.chromium.org/509313002/diff/80001/third_party/devscripts/l...
File third_party/devscripts/licensecheck.pl (left):

https://codereview.chromium.org/509313002/diff/80001/third_party/devscripts/l...
third_party/devscripts/licensecheck.pl:599: if ($licensetext =~ /Permission is
hereby granted, free of charge, to any person or organization obtaining a copy
of the software and accompanying documentation covered by this license \(the
\"Software\"\)/ or
On 2014/09/09 12:31:36, Paweł Hajdan Jr. wrote:
> This is likely to regress other cases where quotes are used. Can you make it
> handle both cases, quotes and no quotes? I'm fine with just making each of the
> \" optional.

Done.

[Paweł Hajdan Jr., 2014-09-11 12:10:42]

LGTM

[mcasas, 2014-09-12 09:51:45]

mcasas@chromium.org changed reviewers:
+ brettw@chromium.org, cpu@chromium.org

[mcasas, 2014-09-12 09:51:46]

brettw@/ cpu@/ darin@: RS OWNERS please.

This landing has been approved by legal and OSS, see
http://crbug.com/408534 .

[brettw, 2014-09-12 22:26:40]

On 2014/09/12 09:51:46, mcasas wrote:
> brettw@/ cpu@/ darin@: RS OWNERS please.
> 
> This landing has been approved by legal and OSS, see
> http://crbug.com/408534 .

You will need to get legal review for this license, or did you do that already?
Add dannyb@

[tommi (sloooow) - chröme, 2014-09-12 22:37:15]

Yes, Danny has already approved.

On Fri, Sep 12, 2014 at 3:26 PM, <brettw@chromium.org> wrote:

> On 2014/09/12 09:51:46, mcasas wrote:
>
>> brettw@/ cpu@/ darin@: RS OWNERS please.
>>
>
>  This landing has been approved by legal and OSS, see
>> http://crbug.com/408534 .
>>
>
> You will need to get legal review for this license, or did you do that
> already?
> Add dannyb@
>
>
> https://codereview.chromium.org/509313002/
>
> To unsubscribe from this group and stop receiving emails from it, send an
> email to chromium-reviews+unsubscribe@chromium.org.
>

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

[brettw, 2014-09-12 22:40:27]

lgtm

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

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2014-09-12 22:54:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/509313002/120001

[commit-bot: I haz the power, 2014-09-13 00:18:13]

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

[commit-bot: I haz the power, 2014-09-13 00:18:14]

Failed to commit the patch.

[commit-bot: I haz the power, 2014-09-13 00:42:47]

Patchset 5 (id:??) landed as
https://crrev.com/a2decf1c031fa2e085b5e6a86434a7fdd8ee6caf
Cr-Commit-Position: refs/heads/master@{#294701}