Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(82)

Issue 300013: Add yasm and ffmpeg into the build tree for linux. (Closed)

Created:
11 years, 2 months ago by awong
Modified:
9 years, 7 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Add yasm and ffmpeg into the build tree for linux. BUG=22307 TEST=builds correctly, and an html5 video runs.

Patch Set 1 #

Total comments: 12

Patch Set 2 : Add in stuff needed to build. #

Patch Set 3 : Better version of the script. #

Patch Set 4 : Add TODOs marking examples of issues #

Total comments: 25

Patch Set 5 : More change #

Patch Set 6 : ffmpeg stable. #

Patch Set 7 : Almost there #

Total comments: 100

Patch Set 8 : Address Mark's comments. #

Patch Set 9 : EVerything works except -O0. #

Patch Set 10 : Fixes for scons. #

Total comments: 1

Patch Set 11 : Move the libffmpegsumo.so into the right spot and fix the installer. #

Patch Set 12 : Remove silly newline. #

Patch Set 13 : Make things buildable w/o -fomit-frame-pointer #

Patch Set 14 : The I HATE SCONS patch. #

Patch Set 15 : The I HATE SCONS - 2: "typo hell" patch. #

Patch Set 16 : Clean up wording. #

Patch Set 17 : Try again #

Patch Set 18 : use STATIC_LIB_PREFIX #

Patch Set 19 : Use a brand-new-gyp feature to make code scons agnostic. I repeat. Scons needs to die. #

Total comments: 20

Patch Set 20 : Address Andrew's comments. #

Patch Set 21 : gold can't do -l:libz.so. Rename zlib and bzip2 targets. #

Total comments: 20

Patch Set 22 : Fix mark's comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8347 lines, -50 lines) Patch
M DEPS View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +7 lines, -0 lines 0 comments Download
M build/all.gyp View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -1 line 0 comments Download
M chrome/installer/installer.gyp View 1 chunk +1 line, -3 lines 0 comments Download
M media/base/media_posix.cc View 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 2 chunks +10 lines, -0 lines 0 comments Download
M third_party/bzip2/bzip2.gyp View 1 chunk +1 line, -1 line 0 comments Download
M third_party/ffmpeg/README.chromium View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 2 chunks +186 lines, -5 lines 0 comments Download
A third_party/ffmpeg/dummy_nolink.cc View 14 1 chunk +10 lines, -0 lines 0 comments Download
M third_party/ffmpeg/ffmpeg.gyp View 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 4 chunks +378 lines, -39 lines 0 comments Download
A third_party/ffmpeg/munge_config_optimizations.sh View 13 14 15 16 17 18 19 20 21 1 chunk +30 lines, -0 lines 0 comments Download
A third_party/ffmpeg/source/config/Chrome/linux/ia32/config.h View 9 10 11 12 1 chunk +766 lines, -0 lines 0 comments Download
A third_party/ffmpeg/source/config/Chrome/linux/ia32/version.h View 1 chunk +1 line, -0 lines 0 comments Download
A third_party/ffmpeg/source/config/Chrome/linux/x64/config.h View 1 chunk +766 lines, -0 lines 0 comments Download
A third_party/ffmpeg/source/config/Chrome/linux/x64/version.h View 1 chunk +1 line, -0 lines 0 comments Download
A third_party/ffmpeg/source/config/Chromium/linux/ia32/config.h View 9 10 11 12 1 chunk +766 lines, -0 lines 0 comments Download
A third_party/ffmpeg/source/config/Chromium/linux/ia32/version.h View 1 chunk +1 line, -0 lines 0 comments Download
A third_party/ffmpeg/source/config/Chromium/linux/x64/config.h View 1 chunk +766 lines, -0 lines 0 comments Download
A third_party/ffmpeg/source/config/Chromium/linux/x64/version.h View 1 chunk +1 line, -0 lines 0 comments Download
A third_party/yasm/README.chromium View 1 2 3 4 5 6 7 8 1 chunk +118 lines, -0 lines 0 comments Download
A third_party/yasm/source/config/linux/Makefile View 1 chunk +3822 lines, -0 lines 0 comments Download
third_party/yasm/source/config/linux/config.h View 1 chunk +176 lines, -0 lines 0 comments Download
A third_party/yasm/source/config/linux/libyasm-stdint.h View 1 chunk +9 lines, -0 lines 0 comments Download
third_party/yasm/yasm.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +528 lines, -0 lines 0 comments Download
M third_party/zlib/zlib.gyp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 35 (0 generated)
scherkus (not reviewing)
I know you said it's not ready for review but here are my comments anyway ...
11 years, 2 months ago (2009-10-20 01:26:25 UTC) #1
awong
:-/ I'll fix those later. The instructions need an overhaul and style is...well...not fixed yet. ...
11 years, 2 months ago (2009-10-20 01:58:39 UTC) #2
Mark Mentovai
I went to build this to see if I could debug what you were seeing, ...
11 years, 2 months ago (2009-10-20 02:01:14 UTC) #3
Mark Mentovai
I think that what you've uploaded is incomplete because even when I bypass all of ...
11 years, 2 months ago (2009-10-20 02:10:16 UTC) #4
awong
:-/ Did you do a gclient sync? That file should be there.. The original patch ...
11 years, 2 months ago (2009-10-20 02:14:42 UTC) #5
awong
Hey Mark, Things work now, so I'm trying to clean things up. The yasm.gyp file ...
11 years, 2 months ago (2009-10-21 01:31:44 UTC) #6
awong
I also added TODO(ajwong) in the lastest patch for each of the 4 issues I ...
11 years, 2 months ago (2009-10-21 01:33:09 UTC) #7
Mark Mentovai
This is not a full pass, but I don't know which ffmpeg gyp you want ...
11 years, 2 months ago (2009-10-21 01:55:23 UTC) #8
awong
Thanks Mark! I've responded to some of your comments. If you look at this again, ...
11 years, 2 months ago (2009-10-21 02:24:01 UTC) #9
Mark Mentovai
OK, I've answered the rest of your TODO comments. Once you've worked through these, we'll ...
11 years, 2 months ago (2009-10-21 03:37:46 UTC) #10
awong
Okay...this time, *all* the code should be ready for a review. :) Thanks for dealing ...
11 years, 2 months ago (2009-10-22 02:00:02 UTC) #11
Mark Mentovai
OK then, here's some feedback for you! http://codereview.chromium.org/300013/diff/8003/8004 File DEPS (right): http://codereview.chromium.org/300013/diff/8003/8004#newcode138 Line 138: "http://www.tortall.net/svn/yasm/trunk/yasm@2192", ...
11 years, 2 months ago (2009-10-22 02:49:35 UTC) #12
scherkus (not reviewing)
my eyes are bleeding :( http://codereview.chromium.org/300013/diff/8003/8004 File DEPS (right): http://codereview.chromium.org/300013/diff/8003/8004#newcode138 Line 138: "http://www.tortall.net/svn/yasm/trunk/yasm@2192", On 2009/10/22 ...
11 years, 2 months ago (2009-10-22 06:27:49 UTC) #13
awong
I made a pass through and addressed most of your comments, minus unrolling a copy ...
11 years, 2 months ago (2009-10-22 06:47:50 UTC) #14
awong
This makes everythign work *if* I can make it build > -O0. However, I can't ...
11 years, 2 months ago (2009-10-23 23:53:40 UTC) #15
awong
This build now works correctly for linux Make and scons. The last hiccup is the ...
11 years, 2 months ago (2009-10-24 02:08:29 UTC) #16
Mark Mentovai
I'm not sure why it wouldn't show up in <(PRODUCT_DIR), unless there's something specific to ...
11 years, 2 months ago (2009-10-24 04:00:13 UTC) #17
awong
Okay! I think this is finally ready! For r3@Lz!!!! Mark, I think the most useful ...
11 years, 1 month ago (2009-10-28 02:11:35 UTC) #18
awong
Oh, and I tests a release, branded build locally using make. Seems to work. Scons ...
11 years, 1 month ago (2009-10-28 02:12:22 UTC) #19
awong
The branded build on scons actually did break. Go figure. For some reasons, scons decides ...
11 years, 1 month ago (2009-10-28 21:01:51 UTC) #20
awong
I'm pretty sure it's ready for another pass now. Changes since last time: 1) Added ...
11 years, 1 month ago (2009-10-29 21:33:10 UTC) #21
awong
And wow. I jinxed myself. try servers failed. On 2009/10/29 21:33:10, awong wrote: > I'm ...
11 years, 1 month ago (2009-10-29 21:48:01 UTC) #22
Mark Mentovai
I'm optimistic. If you can figure out the try server problem, I hope to have ...
11 years, 1 month ago (2009-10-29 22:06:36 UTC) #23
awong
Ahah! The try servers haven't picked up the rolled gyp deps! That means scons is ...
11 years, 1 month ago (2009-10-29 22:25:11 UTC) #24
awong
I pushed a new gyp change (r736) that fixes the breakage for library targets. Rolled ...
11 years, 1 month ago (2009-10-30 00:00:53 UTC) #25
scherkus (not reviewing)
thoughts on upper-case Chromium versus lowercase chromium? everything else in chrome tree is lowercase all ...
11 years, 1 month ago (2009-10-30 00:25:11 UTC) #26
awong
Thanks for the review. Addressed the comments. As for the capital versus lower case, I'd ...
11 years, 1 month ago (2009-10-30 00:32:02 UTC) #27
awong
Oh, and it turns out gold doesn't like the -l:libz.so syntax. Go figure. I'm going ...
11 years, 1 month ago (2009-10-30 00:33:09 UTC) #28
awong
Got it up on the try servers again. Tony make sure that libzip2.so and libz.so ...
11 years, 1 month ago (2009-10-30 01:11:29 UTC) #29
awong
I see 3 greens on the try servers! linux_view is being annoying...but the compile failure ...
11 years, 1 month ago (2009-10-30 01:39:14 UTC) #30
scherkus (not reviewing)
LG Take care, young (gyp) padawan. As a final sanity check, does anyone need to ...
11 years, 1 month ago (2009-10-30 02:01:32 UTC) #31
awong
No issues with that yet. Everything is still mapped in. We just stopped copying the ...
11 years, 1 month ago (2009-10-30 02:48:38 UTC) #32
Mark Mentovai
OK, at long last, you're the recipient of the coveted LGTM award. The remaining comments ...
11 years, 1 month ago (2009-10-30 03:38:02 UTC) #33
Mark Mentovai
I take back what I said about not doing chrome_bz2 and chrome_zlib on the Mac. ...
11 years, 1 month ago (2009-10-30 03:39:34 UTC) #34
awong
11 years, 1 month ago (2009-10-30 07:34:11 UTC) #35
Comments fixed.  Will submit when it feels safe.

http://codereview.chromium.org/300013/diff/21050/28007
File third_party/bzip2/bzip2.gyp (right):

http://codereview.chromium.org/300013/diff/21050/28007#newcode37
Line 37: 'product_name': 'chrome_bz2',
On 2009/10/30 03:38:02, Mark Mentovai wrote:
> Don't do this on the Mac.  Make this Linux-only.

leaving as is per e-mail.

http://codereview.chromium.org/300013/diff/21050/28008
File third_party/ffmpeg/README.chromium (right):

http://codereview.chromium.org/300013/diff/21050/28008#newcode2
Line 2: crate the ffmpeg.gyp file + related configurations.
On 2009/10/30 03:38:02, Mark Mentovai wrote:
> say "and" aloud, not "+"

But...but...I speak punctuation.

?
!
...
.


.

http://codereview.chromium.org/300013/diff/21050/28008#newcode38
Line 38: The FFmpeg build is relatively straightforward.  All files are build
with
On 2009/10/30 03:38:02, Mark Mentovai wrote:
> are built with

Done.

http://codereview.chromium.org/300013/diff/21050/28010
File third_party/ffmpeg/ffmpeg.gyp (right):

http://codereview.chromium.org/300013/diff/21050/28010#newcode14
Line 14: # unacceptable w/o optimization, freeze the optimizations to -O2.
On 2009/10/30 03:38:02, Mark Mentovai wrote:
> without

Done.

http://codereview.chromium.org/300013/diff/21050/28010#newcode311
Line 311: 'action_name': 'make_library',
On 2009/10/30 03:38:02, Mark Mentovai wrote:
> We need a way to add .o files directly to sources in GYP.  If GYP doesn't have
a
> bug for that, we should file one.

Filed:
http://code.google.com/p/gyp/issues/detail?id=102

http://codereview.chromium.org/300013/diff/21050/28015
File third_party/ffmpeg/source/config/Chrome/linux/x64/version.h (right):

http://codereview.chromium.org/300013/diff/21050/28015#newcode1
Line 1: #define FFMPEG_VERSION "SVN-r28636"
On 2009/10/30 03:38:02, Mark Mentovai wrote:
> OK, well maybe I'll read some of them.
> 
> This is fine for now, but ultimately, we don't need so many copies of this. 
One
> copy will be much more than enough.

Actually, ultimately, I'd like to not have it checked in at all, and have it
auto-generate the string
"ffmpeg-<(ffmpeg_branding)-<(svn_revision)"  if I can figure out how to get
svn_revision.

http://codereview.chromium.org/300013/diff/21050/28024
File third_party/yasm/yasm.gyp (right):

http://codereview.chromium.org/300013/diff/21050/28024#newcode376
Line 376: 'gen_insn_path':
'source/patched-yasm/modules/arch/x86/gen_x86_insn.py',
On 2009/10/30 03:38:02, Mark Mentovai wrote:
> Pity that this is the only line that goes past 80 in the whole file.
> 
> Lately in .gyp files, I've been doing a hanging 4-space indent in cases like
> this to keep things a little tidier.

Done.

http://codereview.chromium.org/300013/diff/21050/28025
File third_party/zlib/zlib.gyp (right):

http://codereview.chromium.org/300013/diff/21050/28025#newcode61
Line 61: 'product_name': 'chrome_zlib',
On 2009/10/30 03:38:02, Mark Mentovai wrote:
> Again, don't do this on the Mac.

leaving as-is per e-mail.

Powered by Google App Engine
This is Rietveld 408576698