add video hole support to the gn build

add video hole support to the gn build

BUG=329447
Committed: https://crrev.com/e6a9adc346e41d6c7676689174c12fe15ff5dbe1
Cr-Commit-Position: refs/heads/master@{#323201}

[Mostyn Bramley-Moore, 2015-03-31 16:09:23]

mostynb@opera.com changed reviewers:
+ brettw@chromium.org, tfarina@chromium.org, ycheo@chromium.org

[Mostyn Bramley-Moore, 2015-03-31 16:09:23]

@Brett, Thiago: please take a look.  The gyp equivalent was added in
https://codereview.chromium.org/132233042 - I don't have access to the linked
crbug, but it's probably relevant here too.

[ycheo (away), 2015-03-31 16:15:58]

ycheo@chromium.org changed reviewers:
+ qinmin@chromium.org, wonsik@chromium.org, xhwang@chromium.org
- ycheo@chromium.org

[tfarina, 2015-03-31 16:17:07]

https://codereview.chromium.org/1042543003/diff/1/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/1042543003/diff/1/build/config/features.gni#n...
build/config/features.gni:78: # Should we enable hole punching for protected
video?
I tend to prefer to keep this comment equal to the GYP version, just changing
'Set to 1' to 'Set to true'...

https://codereview.chromium.org/1042543003/diff/1/build/config/features.gni#n...
build/config/features.gni:79: enable_video_hole = is_android
Should this be = false? How GYP enables this? It seems to be set to 0 there.

[tfarina, 2015-03-31 16:17:50]

tfarina@chromium.org changed reviewers:
- qinmin@chromium.org, tfarina@chromium.org, wonsik@chromium.org,
xhwang@chromium.org

[tfarina, 2015-03-31 16:18:53]

The reviewers list was too big and could cause a delay on review (imo). I
trimmed it down to just Brett in hoping to get it reviewed earlier rather than
later.

[Mostyn Bramley-Moore, 2015-03-31 16:25:20]

Thanks.  (I included ycheo, since they own the original gyp CL, but they are
"away".)

https://codereview.chromium.org/1042543003/diff/1/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/1042543003/diff/1/build/config/features.gni#n...
build/config/features.gni:78: # Should we enable hole punching for protected
video?
On 2015/03/31 16:17:06, tfarina wrote:
> I tend to prefer to keep this comment equal to the GYP version, just changing
> 'Set to 1' to 'Set to true'...

I reworded this to improve the grammar.  Would you prefer my version be added to
build/common.gypi or that version here instead?

https://codereview.chromium.org/1042543003/diff/1/build/config/features.gni#n...
build/config/features.gni:79: enable_video_hole = is_android
On 2015/03/31 16:17:07, tfarina wrote:
> Should this be = false? How GYP enables this? It seems to be set to 0 there.

build/common.gypi has defaults for this in three places...

line 624 (not inside a condition AFAICT): default to 0
line 778: (inside an OS=="android" condition): default to 1
line 1844: (inside a chromecast==1 condition): default to 1

I couldn't find a chromecast gn variable or argument.

[brettw, 2015-03-31 22:50:13]

lgtm

[Mostyn Bramley-Moore, 2015-04-01 07:52:43]

https://codereview.chromium.org/1042543003/diff/1/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/1042543003/diff/1/build/config/features.gni#n...
build/config/features.gni:78: # Should we enable hole punching for protected
video?
On 2015/03/31 16:25:20, Mostyn Bramley-Moore wrote:
> On 2015/03/31 16:17:06, tfarina wrote:
> > I tend to prefer to keep this comment equal to the GYP version, just
changing
> > 'Set to 1' to 'Set to true'...
> 
> I reworded this to improve the grammar.  Would you prefer my version be added
to
> build/common.gypi or that version here instead?

I changed this to be the same as the gyp comment.  We can fix both later if you
like.

[Mostyn Bramley-Moore, 2015-04-01 07:52:52]

The CQ bit was checked by mostynb@opera.com

[Mostyn Bramley-Moore, 2015-04-01 07:52:52]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1042543003/#ps20001
(title: "copy gyp's video hole comment")

[commit-bot: I haz the power, 2015-04-01 07:53:10]

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

[commit-bot: I haz the power, 2015-04-01 08:27:00]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-01 08:27:36]

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

[tfarina, 2015-04-01 12:06:57]

tfarina@chromium.org changed reviewers:
+ tfarina@chromium.org

[tfarina, 2015-04-01 12:06:58]

lgtm