Add ffmpeg (chromium's branch) to third_party for use in mojo media.

Add ffmpeg (chromium's branch) to third_party for use in mojo media.
Update build/config/BUILDCONFIG.gn and build/config/compiler/BUILD.gn
to add some things required by ffmpeg/BUILD.gn.

BUG=
R=jamesr@chromium.org, brettw@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/150a0d8130245f42b9834411f182b3086c6c9a9f

[dalesat, 2015-10-13 00:08:47]

dalesat@chromium.org changed required reviewers:
+ brettw@chromium.org

[jamesr, 2015-10-13 00:13:55]

jamesr@chromium.org changed reviewers:
- brettw@chromium.org

[jamesr, 2015-10-13 00:13:55]

jamesr@chromium.org changed required reviewers:
- brettw@chromium.org

[jamesr, 2015-10-13 00:13:56]

Removing brettw from reviewer line, he doesn't work on this project.

https://codereview.chromium.org/1399363002/diff/1/build/config/compiler/BUILD.gn
File build/config/compiler/BUILD.gn (right):

https://codereview.chromium.org/1399363002/diff/1/build/config/compiler/BUILD...
build/config/compiler/BUILD.gn:1076: # the gcc 4.9 toolchains.
I'm pretty sure we will never use a gcc 4.9 toolchain so I don't think we need
the thing this comment is referring to

https://codereview.chromium.org/1399363002/diff/1/build/config/compiler/BUILD...
build/config/compiler/BUILD.gn:1081: } else if (current_cpu == "mipsel") {
we aren't building anything targeting mips

https://codereview.chromium.org/1399363002/diff/1/build/config/compiler/BUILD...
build/config/compiler/BUILD.gn:1083: } else if (is_chromeos && current_cpu ==
"arm") {
we aren't building for chromeos

https://codereview.chromium.org/1399363002/diff/1/build/config/compiler/BUILD...
build/config/compiler/BUILD.gn:1085: }
was there supposed to be something gcc 4.9 related here?

[brettw, 2015-10-13 21:35:36]

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

[brettw, 2015-10-13 21:35:36]

https://codereview.chromium.org/1399363002/diff/1/build/config/BUILDCONFIG.gn
File build/config/BUILDCONFIG.gn (right):

https://codereview.chromium.org/1399363002/diff/1/build/config/BUILDCONFIG.gn...
build/config/BUILDCONFIG.gn:429: # //third_party/ffmpeg/BUILD.gn wants to remove
this, add it. Does nothing.
You should migrate to Chrome's configuration for this, where the default
optimization config is added unconditionally and this expands to the current
default behavior. We do the same for symbols, etc now since it makes it much
easier to do what ffmpeg is doing. If you don't migrate to this, you'll lose
optimization for the targets that opt into max-optimization.

[dalesat, 2015-10-13 22:29:00]

On 2015/10/13 00:13:56, jamesr wrote:
> Removing brettw from reviewer line, he doesn't work on this project.
> 
>
https://codereview.chromium.org/1399363002/diff/1/build/config/compiler/BUILD.gn
> File build/config/compiler/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1399363002/diff/1/build/config/compiler/BUILD...
> build/config/compiler/BUILD.gn:1076: # the gcc 4.9 toolchains.
> I'm pretty sure we will never use a gcc 4.9 toolchain so I don't think we need
> the thing this comment is referring to
> 
>
https://codereview.chromium.org/1399363002/diff/1/build/config/compiler/BUILD...
> build/config/compiler/BUILD.gn:1081: } else if (current_cpu == "mipsel") {
> we aren't building anything targeting mips
> 
>
https://codereview.chromium.org/1399363002/diff/1/build/config/compiler/BUILD...
> build/config/compiler/BUILD.gn:1083: } else if (is_chromeos && current_cpu ==
> "arm") {
> we aren't building for chromeos
> 
>
https://codereview.chromium.org/1399363002/diff/1/build/config/compiler/BUILD...
> build/config/compiler/BUILD.gn:1085: }
> was there supposed to be something gcc 4.9 related here?

Sorry...blind paste. I've removed the irrelevant cases.

[dalesat, 2015-10-13 22:29:59]

On 2015/10/13 21:35:36, brettw wrote:
> https://codereview.chromium.org/1399363002/diff/1/build/config/BUILDCONFIG.gn
> File build/config/BUILDCONFIG.gn (right):
> 
>
https://codereview.chromium.org/1399363002/diff/1/build/config/BUILDCONFIG.gn...
> build/config/BUILDCONFIG.gn:429: # //third_party/ffmpeg/BUILD.gn wants to
remove
> this, add it. Does nothing.
> You should migrate to Chrome's configuration for this, where the default
> optimization config is added unconditionally and this expands to the current
> default behavior. We do the same for symbols, etc now since it makes it much
> easier to do what ffmpeg is doing. If you don't migrate to this, you'll lose
> optimization for the targets that opt into max-optimization.

New bug: https://github.com/domokit/mojo/issues/472

[jamesr, 2015-10-13 22:33:56]

Does this actually produce the result desired by ffmpeg, i.e. debug code is
still compiled with -O2? you can check by running 'gn desc out/Debug
//third_party/ffmpeg/...' and checking the flags.  the rationale for the
optimization level futzing is in
https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/master/BUILD....

https://codereview.chromium.org/1399363002/diff/20001/build/config/compiler/B...
File build/config/compiler/BUILD.gn (right):

https://codereview.chromium.org/1399363002/diff/20001/build/config/compiler/B...
build/config/compiler/BUILD.gn:1076: # the gcc 4.9 toolchains.
the comment still doesn't make sense, at least to me. nothing in the body here
is about gcc 4.9 and i don't think we will be using gcc 4.9 for this code. 
maybe we only want the first 2 lines of this comment?

[dalesat, 2015-10-14 00:40:42]

On 2015/10/13 22:33:56, jamesr wrote:
> Does this actually produce the result desired by ffmpeg, i.e. debug code is
> still compiled with -O2? you can check by running 'gn desc out/Debug
> //third_party/ffmpeg/...' and checking the flags.  the rationale for the
> optimization level futzing is in
>
https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/master/BUILD....
> 
>
https://codereview.chromium.org/1399363002/diff/20001/build/config/compiler/B...
> File build/config/compiler/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1399363002/diff/20001/build/config/compiler/B...
> build/config/compiler/BUILD.gn:1076: # the gcc 4.9 toolchains.
> the comment still doesn't make sense, at least to me. nothing in the body here
> is about gcc 4.9 and i don't think we will be using gcc 4.9 for this code. 
> maybe we only want the first 2 lines of this comment?

Re -O2: Yes, because the build file at the link you gave adds optimize_max and
that gives us -O2
(https://cs.corp.google.com/#github/domokit/mojo/build/config/compiler/BUILD.g...).

Re comment: Done

[brettw, 2015-10-14 03:12:50]

On 2015/10/14 00:40:42, dalesat wrote:
> On 2015/10/13 22:33:56, jamesr wrote:
> > Does this actually produce the result desired by ffmpeg, i.e. debug code is
> > still compiled with -O2? you can check by running 'gn desc out/Debug
> > //third_party/ffmpeg/...' and checking the flags.  the rationale for the
> > optimization level futzing is in
> >
>
https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/master/BUILD....
> > 
> >
>
https://codereview.chromium.org/1399363002/diff/20001/build/config/compiler/B...
> > File build/config/compiler/BUILD.gn (right):
> > 
> >
>
https://codereview.chromium.org/1399363002/diff/20001/build/config/compiler/B...
> > build/config/compiler/BUILD.gn:1076: # the gcc 4.9 toolchains.
> > the comment still doesn't make sense, at least to me. nothing in the body
here
> > is about gcc 4.9 and i don't think we will be using gcc 4.9 for this code. 
> > maybe we only want the first 2 lines of this comment?
> 
> Re -O2: Yes, because the build file at the link you gave adds optimize_max and
> that gives us -O2
>
(https://cs.corp.google.com/#github/domokit/mojo/build/config/compiler/BUILD.g...).
> 
> Re comment: Done

You'll also get -O1 or -Os or something and some duplicated flags because you're
also applying the regular optimize config. Please see my above response.

[dalesat, 2015-10-14 16:09:02]

On 2015/10/14 03:12:50, brettw wrote:
> On 2015/10/14 00:40:42, dalesat wrote:
> > On 2015/10/13 22:33:56, jamesr wrote:
> > > Does this actually produce the result desired by ffmpeg, i.e. debug code
is
> > > still compiled with -O2? you can check by running 'gn desc out/Debug
> > > //third_party/ffmpeg/...' and checking the flags.  the rationale for the
> > > optimization level futzing is in
> > >
> >
>
https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/master/BUILD....
> > > 
> > >
> >
>
https://codereview.chromium.org/1399363002/diff/20001/build/config/compiler/B...
> > > File build/config/compiler/BUILD.gn (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1399363002/diff/20001/build/config/compiler/B...
> > > build/config/compiler/BUILD.gn:1076: # the gcc 4.9 toolchains.
> > > the comment still doesn't make sense, at least to me. nothing in the body
> here
> > > is about gcc 4.9 and i don't think we will be using gcc 4.9 for this code.

> > > maybe we only want the first 2 lines of this comment?
> > 
> > Re -O2: Yes, because the build file at the link you gave adds optimize_max
and
> > that gives us -O2
> >
>
(https://cs.corp.google.com/#github/domokit/mojo/build/config/compiler/BUILD.g...).
> > 
> > Re comment: Done
> 
> You'll also get -O1 or -Os or something and some duplicated flags because
you're
> also applying the regular optimize config. Please see my above response.

Agreed. I'd like to address this in a separate cl, hence the bug.

[jamesr, 2015-10-14 16:13:06]

I think you should sort that out before adding ffmpeg.

[dalesat, 2015-10-20 17:33:28]

On 2015/10/14 16:13:06, jamesr wrote:
> I think you should sort that out before adding ffmpeg.

Please take another look. I've switched to an older version of ffmpeg to avoid
the gn compatibility issues.

[jamesr, 2015-10-20 23:06:31]

lgtm

[jamesr, 2015-10-20 23:06:54]

https://codereview.chromium.org/1399363002/diff/60001/build/config/compiler/B...
File build/config/compiler/BUILD.gn (right):

https://codereview.chromium.org/1399363002/diff/60001/build/config/compiler/B...
build/config/compiler/BUILD.gn:1077: cflags += [
"-Wno-incompatible-pointer-types" ]
we should open a bug on this, this is not a good warning to disable (i.e. the
ffmpeg code depending on this is wrong)

[dalesat, 2015-10-21 19:33:47]

Committed patchset #4 (id:60001) manually as
150a0d8130245f42b9834411f182b3086c6c9a9f (presubmit successful).