Update ffmpeg binaries directories to support variants based off the target architecture.

Update ffmpeg binaries directories to support variants based off the target architecture.

BUG=20948
TEST=none

[awong, 2009-09-18 00:08:15]

Ran this through the try servers.  They're marked failed on a check_deps for
zlib, which is unrelated to this.  The rest of the compile + unittests ran
though.

Mark, I needed to modify common.gypi because target_arch was not available for
conditional evaluation otherwise.

[Nicolas Sylvain, 2009-09-18 00:18:16]

last time i checked it look good, but don't have time to look right now, so I'm
asking maruel to do it (if he is online).

[M-A Ruel, 2009-09-18 00:38:31]

lgtm but I still think mutating the content of a directory depending on the
platform shouldn't be done unless necessary. I don't think it is necessary in
this case.

http://codereview.chromium.org/215016/diff/2001/3001
File DEPS (right):

http://codereview.chromium.org/215016/diff/2001/3001#newcode114
Line 114: "src/third_party/ffmpeg/binaries/chromium/ia32":
I must say I would have preference to not have this directory "mutable". I would
heartedly prefer to have one directory per platform to make it less confusing.

[scherkus (not reviewing), 2009-09-18 01:09:16]

hmm I've actually forgotten the reason for mapping everything to the
same folder. Believe it might be a relic from times past.

Now would be a good time to simply mirror the structure inside of DEPS

ajwong, thoughts?

On Thursday, September 17, 2009,  <maruel@chromium.org> wrote:
> lgtm but I still think mutating the content of a directory depending on the
> platform shouldn't be done unless necessary. I don't think it is necessary in
> this case.
>
>
> http://codereview.chromium.org/215016/diff/2001/3001
> File DEPS (right):
>
> http://codereview.chromium.org/215016/diff/2001/3001#newcode114
> Line 114: "src/third_party/ffmpeg/binaries/chromium/ia32":
> I must say I would have preference to not have this directory "mutable".
> I would heartedly prefer to have one directory per platform to make it
> less confusing.
>
> http://codereview.chromium.org/215016
>

[awong, 2009-09-18 01:13:40]

On Thu, Sep 17, 2009 at 6:08 PM, Andrew Scherkus <scherkus@chromium.org>wrote:

> hmm I've actually forgotten the reason for mapping everything to the
> same folder. Believe it might be a relic from times past.
>
> Now would be a good time to simply mirror the structure inside of DEPS
>

Yeah, I'm pretty sure it's a relic of the past.   How's this structure:

third_party/ffmpeg/binaries/{chrome,chromium}/{mac,linux,win}/{variant}

and only map the platform you're on.

-Albert



>
> ajwong, thoughts?
>
> On Thursday, September 17, 2009,  <maruel@chromium.org> wrote:
> > lgtm but I still think mutating the content of a directory depending on
> the
> > platform shouldn't be done unless necessary. I don't think it is
> necessary in
> > this case.
> >
> >
> > http://codereview.chromium.org/215016/diff/2001/3001
> > File DEPS (right):
> >
> > http://codereview.chromium.org/215016/diff/2001/3001#newcode114
> > Line 114: "src/third_party/ffmpeg/binaries/chromium/ia32":
> > I must say I would have preference to not have this directory "mutable".
> > I would heartedly prefer to have one directory per platform to make it
> > less confusing.
> >
> > http://codereview.chromium.org/215016
> >
>

[scherkus (not reviewing), 2009-09-18 01:19:47]

Sounds reasonable!

Would the mapping reduce down to:
{mac/win/linux} -> {chrome/chromium}/{mac/win/linux}

..and we'd pick up all variants?

On Thursday, September 17, 2009, Albert J. Wong (=E7=8E=8B=E9=87=8D=E5=82=
=91)
<ajwong@chromium.org> wrote:
> On Thu, Sep 17, 2009 at 6:08 PM, Andrew Scherkus <scherkus@chromium.org> =
wrote:
>
> hmm I've actually forgotten the reason for mapping everything to the
> same folder. Believe it might be a relic from times past.
>
> Now would be a good time to simply mirror the structure inside of DEPS
>
> Yeah, I'm pretty sure it's a relic of the past. =C2=A0 How's this structu=
re:
> third_party/ffmpeg/binaries/{chrome,chromium}/{mac,linux,win}/{variant}
>
> and only map the platform you're on.
> -Albert
>
>
>
> ajwong, thoughts?
>
> On Thursday, September 17, 2009, =C2=A0<maruel@chromium.org> wrote:
>> lgtm but I still think mutating the content of a directory depending on =
the
>> platform shouldn't be done unless necessary. I don't think it is necessa=
ry in
>> this case.
>>
>>
>> http://codereview.chromium.org/215016/diff/2001/3001
>> File DEPS (right):
>>
>> http://codereview.chromium.org/215016/diff/2001/3001#newcode114
>> Line 114: "src/third_party/ffmpeg/binaries/chromium/ia32":
>> I must say I would have preference to not have this directory "mutable".
>> I would heartedly prefer to have one directory per platform to make it
>> less confusing.
>>
>> http://codereview.chromium.org/215016
>>
>
>

[awong, 2009-09-18 01:22:36]

...not sure I understand what you mean...
under binaries, we'd fork into chrome and chromium trees for the different
branding requirements of the binaries.

Under that, we'd have 3 platform directories which would have a conditional
inclusion in the deps.

For each platform, we always map all variants in.

The result is that for any given checkout, you should likely only see the
variants for the platform you are on.

-Albert


On Thu, Sep 17, 2009 at 6:19 PM, Andrew Scherkus <scherkus@chromium.org>wro=
te:

> Sounds reasonable!
>
> Would the mapping reduce down to:
> {mac/win/linux} -> {chrome/chromium}/{mac/win/linux}
>
> ..and we'd pick up all variants?
>
> On Thursday, September 17, 2009, Albert J. Wong (=E7=8E=8B=E9=87=8D=E5=82=
=91)
> <ajwong@chromium.org> wrote:
> > On Thu, Sep 17, 2009 at 6:08 PM, Andrew Scherkus <scherkus@chromium.org=
>
> wrote:
> >
> > hmm I've actually forgotten the reason for mapping everything to the
> > same folder. Believe it might be a relic from times past.
> >
> > Now would be a good time to simply mirror the structure inside of DEPS
> >
> > Yeah, I'm pretty sure it's a relic of the past.   How's this structure:
> > third_party/ffmpeg/binaries/{chrome,chromium}/{mac,linux,win}/{variant}
> >
> > and only map the platform you're on.
> > -Albert
> >
> >
> >
> > ajwong, thoughts?
> >
> > On Thursday, September 17, 2009,  <maruel@chromium.org> wrote:
> >> lgtm but I still think mutating the content of a directory depending o=
n
> the
> >> platform shouldn't be done unless necessary. I don't think it is
> necessary in
> >> this case.
> >>
> >>
> >> http://codereview.chromium.org/215016/diff/2001/3001
> >> File DEPS (right):
> >>
> >> http://codereview.chromium.org/215016/diff/2001/3001#newcode114
> >> Line 114: "src/third_party/ffmpeg/binaries/chromium/ia32":
> >> I must say I would have preference to not have this directory "mutable=
".
> >> I would heartedly prefer to have one directory per platform to make it
> >> less confusing.
> >>
> >> http://codereview.chromium.org/215016
> >>
> >
> >
>

[Evan Martin, 2009-09-18 18:22:44]

http://codereview.chromium.org/215016/diff/5002/8006
File build/common.gypi (right):

http://codereview.chromium.org/215016/diff/5002/8006#newcode50
Line 50: 'target_arch%': '<(target_arch)',
wow, that was the fix?  gnarly.

[Mark Mentovai, 2009-09-18 18:27:09]

http://codereview.chromium.org/215016/diff/5002/8006
File build/common.gypi (right):

http://codereview.chromium.org/215016/diff/5002/8006#newcode50
Line 50: 'target_arch%': '<(target_arch)',
Evan Martin wrote:
> wow, that was the fix?  gnarly.

Yeah, this is a sucky limitation.  I'm glad you found this.  The whole way that
conditions and variables and defaults work when they're inside one another is
kind of weird in corner cases.  I think we got the behavior right for the
majority of cases, but I'm not satisfied that we have to fall back on this. 
It's on my mental list of things that we could improve on.

[Mark Mentovai, 2009-09-18 18:30:31]

Oh, by the way: LGTM.

http://codereview.chromium.org/215016/diff/5002/8007
File third_party/ffmpeg/ffmpeg.gyp (right):

http://codereview.chromium.org/215016/diff/5002/8007#newcode194
Line 194: 'ffmpeg_bin_dir': 'chrome/<(OS)/<(ffmpeg_variant)',
Nit: too much indent!  (It was there already.)

[Michael Moss, 2009-09-18 20:02:45]

Could you also update chrome/installer/installer.gyp to remove the 64-bit
input_files exclusion (around line 512)?

[awong, 2009-09-18 20:15:46]

Made changes to the installer....not really sure how to test though since i've
never build/run it before.  Are there instructions anywhere?

[Michael Moss, 2009-09-18 20:21:45]

It's not really an installer change, just changing the list of inputs for the
installer target, so it gets rebuilt when necessary. I thought it would be good
to have it all in one CL, but I can change it later if you prefer.

[Michael Moss, 2009-09-18 20:29:58]

lgtm