Preparation CL for H264 support in WebRTC.

H264 is about to be supported in WebRTC behind flag, this
is preparation work for enabling it.

Background: The flag (from WebRTC) is |rtc_use_h264|.
Additionally, for H264 decoding to work an
|ffmpeg_branding| has to be used that includes H264 (for
example "Chrome").

If |rtc_use_h264| is used, this CL adds code to make sure
FFmpeg is initialized before any decoding happens
(otherwise H264DecoderImpl::InitDecode would fail). This
code is commented out however, due to the below reason.

When building with |rtc_use_h264|,
webrtc::DisableRtcUseH264() is called which disables the
effects of |rtc_use_h264| (disables that H264 enc/dec
implementation). We do this because we plan to default
|rtc_use_h264| to |proprietary_codecs| so that Chromium
trybots will use it. This also makes Chrome use it, but we
are not ready to launch this feature yet. We will remove
DisableRtcUseH264() in a follow-up CL that adds H264
browser tests. That way we have proper test coverage before
we enable it (otherwise it is only tested in webrtc repo,
not full call stack).

BUG=chromium:500605, chromium:468365
Committed: https://crrev.com/2b803417873274a16a0f6c3d5d116d3525026d13
Cr-Commit-Position: refs/heads/master@{#373821}

[hbos_chromium, 2016-01-28 12:58:46]

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

[hbos_chromium, 2016-01-28 12:58:47]

Tommi, in order for H264 decoding to work we must initialize FFmpeg. Chromium
has initialization code that can't be called from WebRTC due to dependency
reasons, so we have to call it some place guaranteed to have run before WebRTC
does decoding. (WebRTC can't use its own FFmpeg initialization code when inside
of Chromium or we may end up initializing FFmpeg twice which could break
things.)

In an email discussion with Dale Curtis we decided it would be best to simply
call FFmpegGlue::InitializeFFmpeg() some place before WebRTC stuff runs.

In this CL I call it at
PeerConnectionDependencyFactory::CreatePeerConnectionFactory(). Is this an
appropriate place to do it or is there some other general "Initialize WebRTC"
place I should look at?

[hbos_chromium, 2016-02-01 09:25:55]

BUMP tommi

[tommi (sloooow) - chröme, 2016-02-01 17:51:20]

lgtm with some comment clarifications

https://codereview.chromium.org/1641163002/diff/1/content/renderer/media/webr...
File content/renderer/media/webrtc/peer_connection_dependency_factory.cc
(right):

https://codereview.chromium.org/1641163002/diff/1/content/renderer/media/webr...
content/renderer/media/webrtc/peer_connection_dependency_factory.cc:267: //
used.
"must be initialized externally from WebRTC" is a bit confusing.

Can we say that H264DecoderImpl depends on ffmpeg for h264 decoding and
therefore we need to initialize ffmpeg before going further?

[hbos_chromium, 2016-02-02 14:16:41]

Description was changed from

==========
H264 support in WebRTC behind flag.

The flag (from WebRTC) is |rtc_use_h264|. Additionally, for H264 decoding to
work an |ffmpeg_branding| has to be used that includes H264 (for example
"Chrome").
(This is not new with this CL.)

This CL makes it possible to use H264 by making sure FFmpeg is initialized
before any decoding happens (otherwise InitDecode will fail).
Calling FFmpegGlue::InitializeFFmpeg() at
PeerConnectionDependencyFactory::CreatePeerConnectionFactory().

BUG=chromium:500605, chromium:468365
==========

to

==========
H264 support in WebRTC behind flag.

The flag (from WebRTC) is |rtc_use_h264|. Additionally, for H264 decoding to
work an |ffmpeg_branding| has to be used that includes H264 (for example
"Chrome").
(This is not new with this CL.)

This CL makes it possible to use H264 by making sure FFmpeg is initialized
before any decoding happens (otherwise InitDecode will fail).
Calling FFmpegGlue::InitializeFFmpeg() at
PeerConnectionDependencyFactory::CreatePeerConnectionFactory().

BUG=chromium:500605, chromium:468365
COMMIT=False
==========

[hbos_chromium, 2016-02-02 14:16:41]

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

[hbos_chromium, 2016-02-02 14:20:12]

+jochen for content/renderer/BUILD.gn. Please take a look.

https://codereview.chromium.org/1641163002/diff/1/content/renderer/media/webr...
File content/renderer/media/webrtc/peer_connection_dependency_factory.cc
(right):

https://codereview.chromium.org/1641163002/diff/1/content/renderer/media/webr...
content/renderer/media/webrtc/peer_connection_dependency_factory.cc:267: //
used.
On 2016/02/01 17:51:20, tommi-chromium wrote:
> "must be initialized externally from WebRTC" is a bit confusing.
> 
> Can we say that H264DecoderImpl depends on ffmpeg for h264 decoding and
> therefore we need to initialize ffmpeg before going further?

Done.

[tommi (sloooow) - chröme, 2016-02-02 15:38:44]

lgtm

[jochen (gone - plz use gerrit), 2016-02-03 13:28:52]

jochen@chromium.org changed reviewers:
+ brettw@chromium.org

[jochen (gone - plz use gerrit), 2016-02-03 13:28:52]

hum, my feeling is that this should be a buildflag, not just a random define?

Brett, is there some documentation about what to do here?

[hbos_chromium, 2016-02-03 13:53:20]

On 2016/02/03 13:28:52, jochen wrote:
> hum, my feeling is that this should be a buildflag, not just a random define?
> 
> Brett, is there some documentation about what to do here?

What do you mean? (There is a build flag, and if it is set the macro is defined
as per the build files.)

[jochen (gone - plz use gerrit), 2016-02-03 13:54:33]

i'm referring to this:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/5c4ySpPsV14

[hbos_chromium, 2016-02-03 14:47:22]

On 2016/02/03 13:54:33, jochen wrote:
> i'm referring to this:
>
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/5c4ySpPsV14

Cool. I get this though:
../../content/renderer/media/webrtc/peer_connection_dependency_factory.cc:264:5:
error: token is not a valid binary operator in a preprocessor subexpression
#if BUILDFLAG(rtc_use_h264)

[brettw, 2016-02-03 18:20:12]

On 2016/02/03 14:47:22, hbos_chromium wrote:
> On 2016/02/03 13:54:33, jochen wrote:
> > i'm referring to this:
> >
>
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/5c4ySpPsV14
> 
> Cool. I get this though:
>
../../content/renderer/media/webrtc/peer_connection_dependency_factory.cc:264:5:
> error: token is not a valid binary operator in a preprocessor subexpression
> #if BUILDFLAG(rtc_use_h264)

That usually means you forgot to include the generated header for the flag.

[hbos_chromium, 2016-02-03 20:42:41]

PTAL jochen/brettw.

[brettw, 2016-02-03 22:19:56]

lgtm

https://codereview.chromium.org/1641163002/diff/40001/content/content.gyp
File content/content.gyp (right):

https://codereview.chromium.org/1641163002/diff/40001/content/content.gyp#new...
content/content.gyp:33: # This conditional looks insane, but without it
|rtc_use_h264| is not
I don't really care about this too much, but I *think* the right solution is to
replace the outer conditions with another variables block:
  'variables': {
     'variables': {
        'buildflag_hea...<other stuff>
      }
   }
Can you see if that fixes the problem before checking in?

https://codereview.chromium.org/1641163002/diff/40001/content/renderer/BUILD.gn
File content/renderer/BUILD.gn (right):

https://codereview.chromium.org/1641163002/diff/40001/content/renderer/BUILD....
content/renderer/BUILD.gn:31: # TODO(GYP) bug 376846 remove this. This should be
inherited from //net but
Can you delete this comment? It looks like it applies to your dep but it
doesn't. I think it's obsolete, or at least my intention was lost in time.

...actually it was removed last week! You will just need to merge this file.

[hbos_chromium, 2016-02-04 09:48:14]

Description was changed from

==========
H264 support in WebRTC behind flag.

The flag (from WebRTC) is |rtc_use_h264|. Additionally, for H264 decoding to
work an |ffmpeg_branding| has to be used that includes H264 (for example
"Chrome").
(This is not new with this CL.)

This CL makes it possible to use H264 by making sure FFmpeg is initialized
before any decoding happens (otherwise InitDecode will fail).
Calling FFmpegGlue::InitializeFFmpeg() at
PeerConnectionDependencyFactory::CreatePeerConnectionFactory().

BUG=chromium:500605, chromium:468365
COMMIT=False
==========

to

==========
H264 is about to be supported in WebRTC behind flag, this is preparation work
for enabling it.

Background: The flag (from WebRTC) is |rtc_use_h264|. Additionally, for H264
decoding to work an |ffmpeg_branding| has to be used that includes H264 (for
example "Chrome").

If |rtc_use_h264| is used, this CL adds code to make sure FFmpeg is initialized
before any decoding happens (otherwise H264DecoderImpl::InitDecode would fail).
This code is commented out however, due to the below reason.

When building with |rtc_use_h264|, webrtc::DisableRtcUseH264() is called which
disables the effects of |rtc_use_h264| (disables that H264 enc/dec
implementation). We do this because we plan to default |rtc_use_h264| to
|proprietary_codecs| so that Chromium trybots will use it. This also makes
Chrome use it, but we are not ready to launch this feature yet. We will remove
DisableRtcUseH264() in a follow-up CL that adds H264 browser tests. That way we
have proper test coverage before we enable it.

BUG=chromium:500605, chromium:468365
COMMIT=False
==========

[hbos_chromium, 2016-02-04 10:04:24]

Description was changed from

==========
H264 is about to be supported in WebRTC behind flag, this is preparation work
for enabling it.

Background: The flag (from WebRTC) is |rtc_use_h264|. Additionally, for H264
decoding to work an |ffmpeg_branding| has to be used that includes H264 (for
example "Chrome").

If |rtc_use_h264| is used, this CL adds code to make sure FFmpeg is initialized
before any decoding happens (otherwise H264DecoderImpl::InitDecode would fail).
This code is commented out however, due to the below reason.

When building with |rtc_use_h264|, webrtc::DisableRtcUseH264() is called which
disables the effects of |rtc_use_h264| (disables that H264 enc/dec
implementation). We do this because we plan to default |rtc_use_h264| to
|proprietary_codecs| so that Chromium trybots will use it. This also makes
Chrome use it, but we are not ready to launch this feature yet. We will remove
DisableRtcUseH264() in a follow-up CL that adds H264 browser tests. That way we
have proper test coverage before we enable it.

BUG=chromium:500605, chromium:468365
COMMIT=False
==========

to

==========
H264 is about to be supported in WebRTC behind flag, this is preparation work
for enabling it.

Background: The flag (from WebRTC) is |rtc_use_h264|. Additionally, for H264
decoding to work an |ffmpeg_branding| has to be used that includes H264 (for
example "Chrome").

If |rtc_use_h264| is used, this CL adds code to make sure FFmpeg is initialized
before any decoding happens (otherwise H264DecoderImpl::InitDecode would fail).
This code is commented out however, due to the below reason.

When building with |rtc_use_h264|, webrtc::DisableRtcUseH264() is called which
disables the effects of |rtc_use_h264| (disables that H264 enc/dec
implementation). We do this because we plan to default |rtc_use_h264| to
|proprietary_codecs| so that Chromium trybots will use it. This also makes
Chrome use it, but we are not ready to launch this feature yet. We will remove
DisableRtcUseH264() in a follow-up CL that adds H264 browser tests. That way we
have proper test coverage before we enable it (otherwise it is only tested in
webrtc repo, not full call stack).

BUG=chromium:500605, chromium:468365
COMMIT=False
==========

[hbos_chromium, 2016-02-04 10:37:58]

Patchset #7 (id:120001) has been deleted

[hbos_chromium, 2016-02-04 10:39:23]

Patchset #6 (id:100001) has been deleted

[hbos_chromium, 2016-02-04 10:47:32]

PTAL tommi, jochen. (Tommi I'll need a new lgtm from you after PS 5 changes.)

Note that I updated the CL description, because in PS 5 I disable the effects of
|rtc_use_h264| for reasons documented (in desc and comment). I was going to do
this in a follow-up CL but I figured I might speed things up by doing it here.

https://codereview.chromium.org/1641163002/diff/40001/content/content.gyp
File content/content.gyp (right):

https://codereview.chromium.org/1641163002/diff/40001/content/content.gyp#new...
content/content.gyp:33: # This conditional looks insane, but without it
|rtc_use_h264| is not
On 2016/02/03 22:19:55, brettw wrote:
> I don't really care about this too much, but I *think* the right solution is
to
> replace the outer conditions with another variables block:
>   'variables': {
>      'variables': {
>         'buildflag_hea...<other stuff>
>       }
>    }
> Can you see if that fixes the problem before checking in?

I tried that (including 'var': '<(var)' in the outer 'variables'), but it didn't
work. Undefined variable rtc_use_h264. GYP is horrendous isn't it. Keeping it
this way.

https://codereview.chromium.org/1641163002/diff/40001/content/renderer/BUILD.gn
File content/renderer/BUILD.gn (right):

https://codereview.chromium.org/1641163002/diff/40001/content/renderer/BUILD....
content/renderer/BUILD.gn:31: # TODO(GYP) bug 376846 remove this. This should be
inherited from //net but
On 2016/02/03 22:19:56, brettw wrote:
> Can you delete this comment? It looks like it applies to your dep but it
> doesn't. I think it's obsolete, or at least my intention was lost in time.
> 
> ...actually it was removed last week! You will just need to merge this file.

Merging got rid of it.

[tommi (sloooow) - chröme, 2016-02-04 10:59:48]

lgtm

https://codereview.chromium.org/1641163002/diff/140001/content/content.gyp
File content/content.gyp (right):

https://codereview.chromium.org/1641163002/diff/140001/content/content.gyp#ne...
content/content.gyp:36: # 'targets' is not an option, then we get compile
errors.
Please file a bug to fix this and add a link to it here

[hbos_chromium, 2016-02-04 11:14:21]

PTAL jochen

https://codereview.chromium.org/1641163002/diff/140001/content/content.gyp
File content/content.gyp (right):

https://codereview.chromium.org/1641163002/diff/140001/content/content.gyp#ne...
content/content.gyp:36: # 'targets' is not an option, then we get compile
errors.
On 2016/02/04 10:59:47, tommi-chromium wrote:
> Please file a bug to fix this and add a link to it here

Done.

[hbos_chromium, 2016-02-04 14:57:23]

Description was changed from

==========
H264 is about to be supported in WebRTC behind flag, this is preparation work
for enabling it.

Background: The flag (from WebRTC) is |rtc_use_h264|. Additionally, for H264
decoding to work an |ffmpeg_branding| has to be used that includes H264 (for
example "Chrome").

If |rtc_use_h264| is used, this CL adds code to make sure FFmpeg is initialized
before any decoding happens (otherwise H264DecoderImpl::InitDecode would fail).
This code is commented out however, due to the below reason.

When building with |rtc_use_h264|, webrtc::DisableRtcUseH264() is called which
disables the effects of |rtc_use_h264| (disables that H264 enc/dec
implementation). We do this because we plan to default |rtc_use_h264| to
|proprietary_codecs| so that Chromium trybots will use it. This also makes
Chrome use it, but we are not ready to launch this feature yet. We will remove
DisableRtcUseH264() in a follow-up CL that adds H264 browser tests. That way we
have proper test coverage before we enable it (otherwise it is only tested in
webrtc repo, not full call stack).

BUG=chromium:500605, chromium:468365
COMMIT=False
==========

to

==========
H264 is about to be supported in WebRTC behind flag, this
is preparation work for enabling it.

Background: The flag (from WebRTC) is |rtc_use_h264|.
Additionally, for H264 decoding to work an
|ffmpeg_branding| has to be used that includes H264 (for
example "Chrome").

If |rtc_use_h264| is used, this CL adds code to make sure
FFmpeg is initialized before any decoding happens
(otherwise H264DecoderImpl::InitDecode would fail). This
code is commented out however, due to the below reason.

When building with |rtc_use_h264|,
webrtc::DisableRtcUseH264() is called which disables the
effects of |rtc_use_h264| (disables that H264 enc/dec
implementation). We do this because we plan to default
|rtc_use_h264| to |proprietary_codecs| so that Chromium
trybots will use it. This also makes Chrome use it, but we
are not ready to launch this feature yet. We will remove
DisableRtcUseH264() in a follow-up CL that adds H264
browser tests. That way we have proper test coverage before
we enable it (otherwise it is only tested in webrtc repo,
not full call stack).

BUG=chromium:500605, chromium:468365
==========

[hbos_chromium, 2016-02-05 10:51:31]

(Bump)

[jochen (gone - plz use gerrit), 2016-02-05 15:00:44]

lgtm

[hbos_chromium, 2016-02-05 15:04:37]

On 2016/02/05 15:00:44, jochen (OOO) wrote:
> lgtm

Thanks. (Btw your name still says "(OOO)")

[hbos_chromium, 2016-02-05 15:04:51]

The CQ bit was checked by hbos@chromium.org

[hbos_chromium, 2016-02-05 15:04:51]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org,
tommi@chromium.org
Link to the patchset: https://codereview.chromium.org/1641163002/#ps160001
(title: "Created bug crbug.com/584219 and updated comment")

[commit-bot: I haz the power, 2016-02-05 15:05:18]

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

[commit-bot: I haz the power, 2016-02-05 16:19:52]

Description was changed from

==========
H264 is about to be supported in WebRTC behind flag, this
is preparation work for enabling it.

Background: The flag (from WebRTC) is |rtc_use_h264|.
Additionally, for H264 decoding to work an
|ffmpeg_branding| has to be used that includes H264 (for
example "Chrome").

If |rtc_use_h264| is used, this CL adds code to make sure
FFmpeg is initialized before any decoding happens
(otherwise H264DecoderImpl::InitDecode would fail). This
code is commented out however, due to the below reason.

When building with |rtc_use_h264|,
webrtc::DisableRtcUseH264() is called which disables the
effects of |rtc_use_h264| (disables that H264 enc/dec
implementation). We do this because we plan to default
|rtc_use_h264| to |proprietary_codecs| so that Chromium
trybots will use it. This also makes Chrome use it, but we
are not ready to launch this feature yet. We will remove
DisableRtcUseH264() in a follow-up CL that adds H264
browser tests. That way we have proper test coverage before
we enable it (otherwise it is only tested in webrtc repo,
not full call stack).

BUG=chromium:500605, chromium:468365
==========

to

==========
H264 is about to be supported in WebRTC behind flag, this
is preparation work for enabling it.

Background: The flag (from WebRTC) is |rtc_use_h264|.
Additionally, for H264 decoding to work an
|ffmpeg_branding| has to be used that includes H264 (for
example "Chrome").

If |rtc_use_h264| is used, this CL adds code to make sure
FFmpeg is initialized before any decoding happens
(otherwise H264DecoderImpl::InitDecode would fail). This
code is commented out however, due to the below reason.

When building with |rtc_use_h264|,
webrtc::DisableRtcUseH264() is called which disables the
effects of |rtc_use_h264| (disables that H264 enc/dec
implementation). We do this because we plan to default
|rtc_use_h264| to |proprietary_codecs| so that Chromium
trybots will use it. This also makes Chrome use it, but we
are not ready to launch this feature yet. We will remove
DisableRtcUseH264() in a follow-up CL that adds H264
browser tests. That way we have proper test coverage before
we enable it (otherwise it is only tested in webrtc repo,
not full call stack).

BUG=chromium:500605, chromium:468365
==========

[commit-bot: I haz the power, 2016-02-05 16:19:54]

Committed patchset #7 (id:160001)

[commit-bot: I haz the power, 2016-02-05 16:21:18]

Description was changed from

==========
H264 is about to be supported in WebRTC behind flag, this
is preparation work for enabling it.

Background: The flag (from WebRTC) is |rtc_use_h264|.
Additionally, for H264 decoding to work an
|ffmpeg_branding| has to be used that includes H264 (for
example "Chrome").

If |rtc_use_h264| is used, this CL adds code to make sure
FFmpeg is initialized before any decoding happens
(otherwise H264DecoderImpl::InitDecode would fail). This
code is commented out however, due to the below reason.

When building with |rtc_use_h264|,
webrtc::DisableRtcUseH264() is called which disables the
effects of |rtc_use_h264| (disables that H264 enc/dec
implementation). We do this because we plan to default
|rtc_use_h264| to |proprietary_codecs| so that Chromium
trybots will use it. This also makes Chrome use it, but we
are not ready to launch this feature yet. We will remove
DisableRtcUseH264() in a follow-up CL that adds H264
browser tests. That way we have proper test coverage before
we enable it (otherwise it is only tested in webrtc repo,
not full call stack).

BUG=chromium:500605, chromium:468365
==========

to

==========
H264 is about to be supported in WebRTC behind flag, this
is preparation work for enabling it.

Background: The flag (from WebRTC) is |rtc_use_h264|.
Additionally, for H264 decoding to work an
|ffmpeg_branding| has to be used that includes H264 (for
example "Chrome").

If |rtc_use_h264| is used, this CL adds code to make sure
FFmpeg is initialized before any decoding happens
(otherwise H264DecoderImpl::InitDecode would fail). This
code is commented out however, due to the below reason.

When building with |rtc_use_h264|,
webrtc::DisableRtcUseH264() is called which disables the
effects of |rtc_use_h264| (disables that H264 enc/dec
implementation). We do this because we plan to default
|rtc_use_h264| to |proprietary_codecs| so that Chromium
trybots will use it. This also makes Chrome use it, but we
are not ready to launch this feature yet. We will remove
DisableRtcUseH264() in a follow-up CL that adds H264
browser tests. That way we have proper test coverage before
we enable it (otherwise it is only tested in webrtc repo,
not full call stack).

BUG=chromium:500605, chromium:468365

Committed: https://crrev.com/2b803417873274a16a0f6c3d5d116d3525026d13
Cr-Commit-Position: refs/heads/master@{#373821}
==========

[commit-bot: I haz the power, 2016-02-05 16:21:19]

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