WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
167207
[WebRTC] Add libwebrtc build infrastructure
https://bugs.webkit.org/show_bug.cgi?id=167207
Summary
[WebRTC] Add libwebrtc build infrastructure
youenn fablet
Reported
2017-01-19 11:12:48 PST
Add a way to build libwebrtc from source code inside WebKit
Attachments
Patch
(4.45 MB, patch)
2017-01-19 13:00 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Libwebrtc source code
(
deleted
)
2017-01-19 13:24 PST
,
youenn fablet
no flags
Details
Patch for landing
(3.81 MB, patch)
2017-01-20 10:06 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
youenn fablet
Comment 1
2017-01-19 13:00:22 PST
Created
attachment 299263
[details]
Patch
youenn fablet
Comment 2
2017-01-19 13:24:13 PST
Created
attachment 299264
[details]
Libwebrtc source code
Alexey Proskuryakov
Comment 3
2017-01-19 17:18:04 PST
This repeatedly hardcodes /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk - is that right?
Alex Christensen
Comment 4
2017-01-19 17:22:59 PST
That is not right.
youenn fablet
Comment 5
2017-01-19 17:59:18 PST
This flag is given by libwebrtc gn build system. We can filter this out as part of the script. Which value should be passed instead?
Alex. Gouaillard
Comment 6
2017-01-19 21:24:02 PST
As we were discussing this week on the discuss-webrtc mailing list, GN is going to generate configurations depending on input parameters, so you will dependent on: - the args you pass - all the .gn and .gni files - the host_arch and host_os - your local configuration What we really would like is to import the .gn[i] files instead, but it s almost intractable. In that specific case, "-mmacosx-version-min=10.9" should be enough, and is hardcoded in one of the gn[i] file. But somewhere during the process src/build/mac/find_sdk.py is called, which takes the highest sdk available on the host machine that matches the idk_min condition. That's where you are getting this long line, that should only be a local variable and not part of a global build. Guys, I'm maintaining several cmake wrappers and tool for libwebrtc, as well as a CTests, CDash (
http://my.cdash.org/index.php?project=libwebRTC
) and so on, and I have bumped into a lot of stones on the way. I'm hanging out every day in irc webkit under the nick "Alex_SG". Do not hesitate to ask me anything.
youenn fablet
Comment 7
2017-01-20 08:04:11 PST
Removing this flag seems to work on my setup. I'll update the patch accordingly and commit it if there is no objection.
youenn fablet
Comment 8
2017-01-20 10:06:32 PST
Created
attachment 299351
[details]
Patch for landing
WebKit Commit Bot
Comment 9
2017-01-20 10:46:12 PST
Comment on
attachment 299351
[details]
Patch for landing Clearing flags on attachment: 299351 Committed
r210973
: <
http://trac.webkit.org/changeset/210973
>
WebKit Commit Bot
Comment 10
2017-01-20 10:46:17 PST
All reviewed patches have been landed. Closing bug.
Alex. Gouaillard
Comment 11
2017-01-21 16:15:55 PST
I took a look at the python script, I see the gn command line including these: - rtc_include_tests=true - rtc_use_h264=true - rtc_libvpx_build_vp9=false - rtc_libvpx_build_vp8=false 1. Tests. libwebrtc does not contain all the arguments and infos to run the tests, even though they are compiled and the input files are downloaded. One need to get a copy of the builedbot scripts, from their "infra" repository to import arguments. Since the download of the files is taking a lot of time, since building the tests take some time, since you seem to be ignoring the tests target later on, and since you are using a release branch which is less likely to fail, you might want to skip the tests all together. Henrik is working on making the download dependent on the etc_include_tests flag. If you want to import the test for your unit test needs, you can open a bug, and I ll give you some of my cmake scripts to import them from the infra repo to get that:
http://my.cdash.org/viewTest.php?onlypassed&buildid=1120561
2. H.264 I had this arguments to add support for H.264: gn gen out/Release -args='ffmpeg_branding="Chrome" proprietary_codecs=true' What did I miss? 3. Is the VP8/9 support missing a temporary thing? The is here again (as in G.711) the question of the mandatory to implement codec to be spec compliant.
youenn fablet
Comment 12
2017-01-21 18:03:32 PST
(In reply to
comment #11
)
> I took a look at the python script, > I see the gn command line including these: > - rtc_include_tests=true > - rtc_use_h264=true > - rtc_libvpx_build_vp9=false > - rtc_libvpx_build_vp8=false
Yes, it is no longer in sync. rtc_use_h264 should be removed I believe since it uses software codecs.
> 1. Tests. > libwebrtc does not contain all the arguments and infos to run the tests, > even though they are compiled and the input files are downloaded. One need > to get a copy of the builedbot scripts, from their "infra" repository to > import arguments. Since the download of the files is taking a lot of time, > since building the tests take some time, since you seem to be ignoring the > tests target later on, and since you are using a release branch which is > less likely to fail, you might want to skip the tests all together. Henrik > is working on making the download dependent on the etc_include_tests flag. > If you want to import the test for your unit test needs, you can open a bug, > and I ll give you some of my cmake scripts to import them from the infra > repo to get that:
http://my.cdash.org/viewTest.php?onlypassed&buildid=1120561
The script that goes from project.json (generated by libwebrtc build system) can enable/disable test targets.
Alex. Gouaillard
Comment 13
2017-01-21 18:20:58 PST
I don't think the h264 flag is functional at all. The firefox version of webrtc uses openH264, and tries to load the pre-compiled binary as often as possible, while chrome relies on a mix of ffmpeg for decoder and openH264 for encoder. Firefox did not upstream that part. About the tests, what I meant is : if you want to run them, you will need more than enabling their configuration and building them, and if you don't want to run them, it's better to use the gn flag than your flag, before some of the hooks will download the resources depending on the gn flag, independently of what you do with the targets later.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug