WebRTC: Enable OpenH264 for WebRTC standalone bots.

WebRTC: Enable OpenH264 for WebRTC standalone bots.

hbos@ in the WebRTC team is working on enabling OpenH264 as an H.264
encoder in Chrome as part of http://crbug.com/468365. This CL sets the
necessary GYP variables to turn it on in the build for standalone WebRTC
bots and the Chromium bots we control.

NOTICE: Due to licensing and patent reasons, OpenH264 cannot be enabled
by default for end-users. Going forward, the plan is that it will only be
enabled on WebRTC controlled bots and the Chromium official builders.

BUG=576201
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=298217

[kjellander_chromium, 2016-01-11 09:41:51]

Description was changed from

==========
WebRTC: Enable OpenH264 for WebRTC standalone bots.

hbos@ in the WebRTC team is working on enabling OpenH264 as an H.264
encoder in Chrome as part of http://crbug.com/468365. This CL sets the
necessary GYP variables to turn it on in the build for standalone WebRTC
bots and the Chromium bots we control.
NOTICE: Due to licensing and patent reasons, OpenH264 cannot be enabled
by default for end-users. Going forward, the plan is that it will only be
enabled on WebRTC controlled bots and the Chromium official builders.

BUG=576201
==========

to

==========
WebRTC: Enable OpenH264 for WebRTC standalone bots.

hbos@ in the WebRTC team is working on enabling OpenH264 as an H.264
encoder in Chrome as part of http://crbug.com/468365. This CL sets the
necessary GYP variables to turn it on in the build for standalone WebRTC
bots and the Chromium bots we control.

NOTICE: Due to licensing and patent reasons, OpenH264 cannot be enabled
by default for end-users. Going forward, the plan is that it will only be
enabled on WebRTC controlled bots and the Chromium official builders.

BUG=576201
==========

[kjellander_chromium, 2016-01-11 09:41:59]

The CQ bit was checked by kjellander@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-11 09:42:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577773002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577773002/1

[commit-bot: I haz the power, 2016-01-11 09:44:46]

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

[commit-bot: I haz the power, 2016-01-11 09:44:47]

Dry run: This issue passed the CQ dry run.

[kjellander_chromium, 2016-01-11 10:45:01]

kjellander@chromium.org changed reviewers:
+ tandrii@chromium.org

[kjellander_chromium, 2016-01-11 10:52:47]

kjellander@chromium.org changed reviewers:
+ hbos@chromium.org

[kjellander_chromium, 2016-01-11 10:52:47]

+hbos

[tandrii(chromium), 2016-01-11 11:16:00]

infra perspective + owner: LGTM

[kjellander_chromium, 2016-01-11 13:55:32]

On 2016/01/11 11:16:00, tandrii(chromium) wrote:
> infra perspective + owner: LGTM

hbos: please take a look, since I've now updated the GN bots.

[hbos_chromium, 2016-01-11 14:21:09]

LGTM but I think "openh264" should be renamed to "third_party_h264" or
"webrtc_third_party_h264" or similar.

https://codereview.chromium.org/1577773002/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium/config.py (right):

https://codereview.chromium.org/1577773002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium/config.py:369: def openh264(c):
Hmm the name "openh264" might be misleading here since this h264 feature relies
on both openh264 and ffmpeg. That's why I refer to it as "third_party_h264"
(perhaps not the best name either) since it relies on multiple third_party
libraries.

(I didn't simply call it webrtc_h264 because there is an iOS enc/dec too with
video toolbox. And HW versions as well for android. And plans for more. Ugh.)

https://codereview.chromium.org/1577773002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium/config.py:370: ffmpeg_branding(c,
branding='Chrome')
(Any idea what happens if multiple defs are used and ffmpeg_branding is
accidentally set multiple times? Just wondering since I see ffmpeg_branding in
other places in this file.)

[hbos_chromium, 2016-01-11 14:23:59]

On 2016/01/11 14:21:09, hbos_chromium wrote:
> LGTM but I think "openh264" should be renamed to "third_party_h264" or
> "webrtc_third_party_h264" or similar.
> 
>
https://codereview.chromium.org/1577773002/diff/20001/scripts/slave/recipe_mo...
> File scripts/slave/recipe_modules/chromium/config.py (right):
> 
>
https://codereview.chromium.org/1577773002/diff/20001/scripts/slave/recipe_mo...
> scripts/slave/recipe_modules/chromium/config.py:369: def openh264(c):
> Hmm the name "openh264" might be misleading here since this h264 feature
relies
> on both openh264 and ffmpeg. That's why I refer to it as "third_party_h264"
> (perhaps not the best name either) since it relies on multiple third_party
> libraries.
> 
> (I didn't simply call it webrtc_h264 because there is an iOS enc/dec too with
> video toolbox. And HW versions as well for android. And plans for more. Ugh.)
> 
>
https://codereview.chromium.org/1577773002/diff/20001/scripts/slave/recipe_mo...
> scripts/slave/recipe_modules/chromium/config.py:370: ffmpeg_branding(c,
> branding='Chrome')
> (Any idea what happens if multiple defs are used and ffmpeg_branding is
> accidentally set multiple times? Just wondering since I see ffmpeg_branding in
> other places in this file.)

(Perhaps I should have called it openh264_ffmpeg_h264 instead... oh well.)

[kjellander_chromium, 2016-01-12 10:11:45]

https://codereview.chromium.org/1577773002/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium/config.py (right):

https://codereview.chromium.org/1577773002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium/config.py:369: def openh264(c):
On 2016/01/11 14:21:09, hbos_chromium wrote:
> Hmm the name "openh264" might be misleading here since this h264 feature
relies
> on both openh264 and ffmpeg. That's why I refer to it as "third_party_h264"
> (perhaps not the best name either) since it relies on multiple third_party
> libraries.
> 
> (I didn't simply call it webrtc_h264 because there is an iOS enc/dec too with
> video toolbox. And HW versions as well for android. And plans for more. Ugh.)

Well, OpenH264 is not used at all in Chromium, as the ffmpeg version implements
its own H264 decoder.
That makes me think this name is better than third_party_h264. Possibly
third_party_openh264 could be an option, but since there's no other openh264
anywhere, I think the third_party prefix doesn't add anything useful.
The naming of these configs also aren't that important, since they're only
referenced by the buildbot configurations.

https://codereview.chromium.org/1577773002/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium/config.py:370: ffmpeg_branding(c,
branding='Chrome')
On 2016/01/11 14:21:09, hbos_chromium wrote:
> (Any idea what happens if multiple defs are used and ffmpeg_branding is
> accidentally set multiple times? Just wondering since I see ffmpeg_branding in
> other places in this file.)

Any latter will overwrite the previous, but it's unlikely to happen since there
aren't that long chains that uses this setting. The resulting GYP_DEFINES will
also show up in the JSON files for the different bot configurations, allowing
reviewers to spot such an error.

[kjellander_chromium, 2016-01-12 14:31:12]

The CQ bit was checked by kjellander@chromium.org

[kjellander_chromium, 2016-01-12 14:31:13]

The patchset sent to the CQ was uploaded after l-g-t-m from tandrii@chromium.org
Link to the patchset: https://codereview.chromium.org/1577773002/#ps20001
(title: "Set the GN arguments properly")

[commit-bot: I haz the power, 2016-01-12 14:31:22]

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

[commit-bot: I haz the power, 2016-01-12 14:34:23]

Description was changed from

==========
WebRTC: Enable OpenH264 for WebRTC standalone bots.

hbos@ in the WebRTC team is working on enabling OpenH264 as an H.264
encoder in Chrome as part of http://crbug.com/468365. This CL sets the
necessary GYP variables to turn it on in the build for standalone WebRTC
bots and the Chromium bots we control.

NOTICE: Due to licensing and patent reasons, OpenH264 cannot be enabled
by default for end-users. Going forward, the plan is that it will only be
enabled on WebRTC controlled bots and the Chromium official builders.

BUG=576201
==========

to

==========
WebRTC: Enable OpenH264 for WebRTC standalone bots.

hbos@ in the WebRTC team is working on enabling OpenH264 as an H.264
encoder in Chrome as part of http://crbug.com/468365. This CL sets the
necessary GYP variables to turn it on in the build for standalone WebRTC
bots and the Chromium bots we control.

NOTICE: Due to licensing and patent reasons, OpenH264 cannot be enabled
by default for end-users. Going forward, the plan is that it will only be
enabled on WebRTC controlled bots and the Chromium official builders.

BUG=576201

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=298217
==========

[commit-bot: I haz the power, 2016-01-12 14:34:23]

Committed patchset #2 (id:20001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=298217

[kjellander_chromium, 2016-01-12 14:57:09]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1581653002/ by kjellander@chromium.org.

The reason for reverting is: Breaks on Win GN 
https://build.chromium.org/p/client.webrtc/builders/Win64%20Debug%20%28GN%29/...
and probably more (will add more info to the revert CL).
.