Closed
Bug 981780
Opened 11 years ago
Closed 11 years ago
Build error in --disable-webrtc builds: "VP8TrackEncoder.cpp:319: error: undefined reference to 'NV12ToI420'" and more
Categories
(Core :: WebRTC, defect)
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: dholbert, Assigned: jesup)
References
Details
(Keywords: regression, Whiteboard: [qa-] )
Attachments
(5 files, 7 obsolete files)
1.68 KB,
patch
|
Details | Diff | Splinter Review | |
13.37 KB,
patch
|
Details | Diff | Splinter Review | |
4.97 KB,
patch
|
jesup
:
review-
|
Details | Diff | Splinter Review |
2.81 KB,
patch
|
drno
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
4.60 KB,
patch
|
glandium
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Since bug 970787's landing, I'm getting this build error:
{
/usr/bin/ld.gold.real: error: $OBJDIR/toolkit/library/../../content/media/encoder/Unified_cpp_content_media_encoder0.o: requires dynamic R_X86_64_PC32 reloc against 'NV12ToI420' which may overflow at runtime; recompile with -fPIC
/usr/bin/ld.gold.real: error: read-only segment has dynamic relocations
$SRCDIR/content/media/encoder/VP8TrackEncoder.cpp:319: error: undefined reference to 'NV12ToI420'
$SRCDIR/content/media/encoder/VP8TrackEncoder.cpp:326: error: undefined reference to 'NV21ToI420'
$SRCDIR/content/media/encoder/VP8TrackEncoder.cpp:334: error: undefined reference to 'I444ToI420'
$SRCDIR/content/media/encoder/VP8TrackEncoder.cpp:342: error: undefined reference to 'I422ToI420'
}
Seems to happen reliably in clobber builds with --disable-webrtc in mozconfig.
Seems to happen if I have --disable-webrtc in my .mozconfig. At least, it fixed itself when I removed that option.
Reporter | ||
Comment 1•11 years ago
|
||
jesup points out that libyuv doesn't live in /webrtc source tree.
However, it looks like webrtc does control whether or not it's built, as shown by this MXR search:
http://mxr.mozilla.org/mozilla-central/search?string=build_libyuv
===========
/build/gyp.mozbuild (View Hg log or Hg annotations)
line 26 -- 'build_libyuv': 0,
/media/webrtc/trunk/webrtc/build/common.gypi (View Hg log or Hg annotations)
line 97 -- 'build_libyuv%': 1,
/media/webrtc/trunk/webrtc/common_video/common_video.gyp (View Hg log or Hg annotations)
line 30 -- ['build_libyuv==1', {
===========
So: looks like libyuv isn't built by default, and webrtc turns it on.
Reporter | ||
Comment 2•11 years ago
|
||
> /build/gyp.mozbuild (View Hg log or Hg annotations)
>
> line 26 -- 'build_libyuv': 0,
jesup suggested switching this from 0 to 1. I tried that (and clobbered), but unfortunately it doesn't fix this -- I still get the same build error.
Comment 3•11 years ago
|
||
Hi Benjamin,
Please help for this bug. thanks.
Assignee: nobody → bechen
Component: Video/Audio → Video/Audio: Recording
Comment 4•11 years ago
|
||
Perhaps this file needs to have a separate section that checks MOZ_WEBM_ENCODER
rather than MOZ_WEBRTC for 'media/libyuv'?
http://mxr.mozilla.org/mozilla-central/source/toolkit/toolkit.mozbuild#62
Keywords: regression
Reporter | ||
Comment 5•11 years ago
|
||
Good find -- though it looks like that's also not sufficient.
If I make this change, then a clobber build with --disable-webrtc fails during the configure step, with:
{
python: can't open file './build/dir_exists.py': [Errno 2] No such file or directory
[...]
ERROR PROCESSING MOZBUILD FILE
The error occurred while processing the following file:
$SRCDIR/mozilla/media/libyuv/moz.build
}
Reporter | ||
Comment 6•11 years ago
|
||
(er, I guess it fails just *after* the configure step.)
I filed bug 982185 as a helper-bug on that failure, with more details about the failure message, BTW, since it looks like there might be a build system bug involved. (Though even with that resolved, I'm betting it'd just mean we'll have a cleaner error here. Probably still more fixing required.)
Assignee | ||
Comment 7•11 years ago
|
||
Comment on attachment 8389283 [details]
strawman #1 (busted in --disable-webrtc builds)
Aha, yes, that's probably the problem However, media/libyuv should now be outside of all configs I think; we want it to be available for all targets so we can leverage in other spots (gfx, compositor, etc). And note: webm encoding isn't available on B2G yet, which is why the above patch may not be sufficient.
Reporter | ||
Comment 8•11 years ago
|
||
OK, here's a patch with media/libyuv added regardless of config, and with the gyp tweak from comment 2.
This still hits the same issue noted in comment 5, in --disable-webrtc builds.
(Nether of these patches have problems in builds with webrtc enabled, but that's not surprising since those have always been fine.)
Attachment #8389283 -
Attachment is obsolete: true
Reporter | ||
Updated•11 years ago
|
Attachment #8389283 -
Attachment description: fix-toolkit.mozbuild → strawman #1 (busted in --disable-webrtc builds)
Reporter | ||
Updated•11 years ago
|
Attachment #8389335 -
Attachment filename: fix-toolkit.mozbuild → fix-gyp-and-toolkit.patch
Attachment #8389335 -
Attachment is patch: true
Comment 9•11 years ago
|
||
Still occurs. Can someone please take a look?
Comment 10•11 years ago
|
||
Comment 11•11 years ago
|
||
This patch only works on Linux for enable/disable webrtc. The reason why we have "can't open file './build/dir_exists.py'" error is that when building gyp projects, we always include "media/webrtc/trunk/build/common.gypi". It runs some script to get environment settings and the relative path is only correct when we are building webrtc. Should we eliminate the dependency between gyp projects and common.gypi that is remove the code in [1]?
[1] http://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/frontend/gyp_reader.py#100
Flags: needinfo?(rjesup)
Comment 12•11 years ago
|
||
Hi Randell,
Do you have comments on slee's patch?
Comment 13•11 years ago
|
||
Why not convert media/libyuv to moz.build ? gfx/angle and gfx/skia are examples of doing so outside of webrtc build. skia also has generate_mozbuild.py to facilitate future updates.
Comment 15•11 years ago
|
||
All future branches affected.. i was hitting it on aurora/sparc64 when it was still 30, and now i've hit it again on central/amd64.
status-firefox29:
--- → unaffected
status-firefox30:
--- → affected
status-firefox31:
--- → affected
status-firefox32:
--- → affected
Comment 16•11 years ago
|
||
Even if not an acceptable solution, att #8394229 allows me to pass configure on OpenBSD using --disable-webrtc with m-c. Testing Ffx 30.0b1.
Comment 17•11 years ago
|
||
Fwiw, with att #8394229 i can build m-c fully. Ffx 30.0b1 being broken with disable-webrtc, and needing to touch windows-related stuff in media/webrtc/trunk/build/common.gypi (which is supposed to be disabled, ffs!) to work around it shows that the build system has deep issues. I always hated gyp, now i know why.
Comment 18•11 years ago
|
||
The same patch allows me to build fx 30.0b1 with disable-webrtc, but this is still gross.
Updated•11 years ago
|
Assignee: bechen → nobody
Status: ASSIGNED → NEW
Comment 19•11 years ago
|
||
I know --disable-webrtc is NPOTB thus a configuration not tested by supported platforms, but this is still in use by lots of configurations/architectures out there, so having it fixed properly would be nice.
Bug 970787 made libyuv mandatory, not taking into account at all the configurations that dont build it (ie non-webrtc ones). Benjamin, instead of dropping ownership of the bug, i really think you should help us here......
Comment 20•11 years ago
|
||
gps, since you're the moz.build owner, any hint on how to solve this ?
Flags: needinfo?(gps)
Assignee | ||
Updated•11 years ago
|
Component: Video/Audio: Recording → WebRTC
Flags: needinfo?(rjesup)
Assignee | ||
Comment 22•11 years ago
|
||
Reporter | ||
Comment 23•11 years ago
|
||
Comment on attachment 8418822 [details] [diff] [review]
fix disable-webrtc on linux/etc
>diff --git a/tools/quitter/chrome.manifest b/tools/quitter/chrome.manifest
>--- a/tools/quitter/chrome.manifest
>+++ b/tools/quitter/chrome.manifest
>@@ -1,4 +1,4 @@
>+category profile-after-change @mozilla.org/quitter-observer;1 @mozilla.org/quitter-observer;1
>+component {c235a986-5ac1-4f28-ad73-825dae9bad90} components/QuitterObserver.js
> content quitter chrome/quitter/content/
>-component {c235a986-5ac1-4f28-ad73-825dae9bad90} components/QuitterObserver.js
> contract @mozilla.org/quitter-observer;1 {c235a986-5ac1-4f28-ad73-825dae9bad90}
>-category profile-after-change @mozilla.org/quitter-observer;1 @mozilla.org/quitter-observer;1
You don't want these changes in your patch. (See bug 1006541 for what happened here. Shouldn't affect m-c anymore.)
Comment 24•11 years ago
|
||
I added one more flag to check whether we enable webrtc or not and use this flag to prevent running some unnecessary commands.
Attachment #8394229 -
Attachment is obsolete: true
Assignee | ||
Comment 25•11 years ago
|
||
Comment on attachment 8418855 [details] [diff] [review]
disable webrtc
Review of attachment 8418855 [details] [diff] [review]:
-----------------------------------------------------------------
I think I found a simpler way to deal with it in my patch. And I don't want to modify common.gypi any more than I have to, and my patch avoids any mods there. (Thanks for the patches though; I build on what you found)
Attachment #8418855 -
Flags: review-
Assignee | ||
Comment 26•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #8418822 -
Attachment is obsolete: true
Comment 27•11 years ago
|
||
< jesup > gaston: bonus points if you make fabrice happy and write a patch to disable webrtc tests if we have --disable-webrtc (dom/media/tests/crashtests & mochitests)
< RyanVM > lol
< RyanVM > jesup: well played ;)
I took you up on this, and here's a shot at it, only tested in the --disable-webrtc case but seems to do the trick. Since i'm not so used to running tests ,that might of course be totally wrong, and i didnt run the whole test suite, only the webrtc relevant parts you mentioned - there might be others :)
$mach mochitest-plain dom/media/tests/mochitest/
....
1:07.70 10 INFO TEST-START | Shutdown
1:07.70 11 INFO Passed: 1
1:07.70 12 INFO Failed: 0
1:07.70 13 INFO Todo: 0
for reference, without it:
2:19.67 604 INFO TEST-START | Shutdown
2:19.67 605 INFO Passed: 300
2:19.67 606 INFO Failed: 31
2:19.67 607 INFO Todo: 0
$mach crashtest --filter dom/media
...
0:29.13 REFTEST INFO | Discovered 14 tests, after filtering SKIP tests, we have 0
for reference, without it:
0:09.73 REFTEST TEST-UNEXPECTED-FAIL | file:///src/mozilla-central/dom/media/tests/crashtests/780790.html | boolean preference 'media.peerconnection.enabled' not known or wrong type
0:09.73 REFTEST TEST-UNEXPECTED-FAIL | file:///src/mozilla-central/dom/media/tests/crashtests/791270.html | boolean preference 'media.peerconnection.enabled' not known or wrong type
etc etc etc
Attachment #8419219 -
Flags: feedback?(rjesup)
Comment 28•11 years ago
|
||
Comment on attachment 8418872 [details] [diff] [review]
fix disable-webrtc on linux/etc
Fixes the build issue for me on m-i tip, and also backported on top of 30.0b2.
Attachment #8418872 -
Flags: feedback+
Assignee | ||
Updated•11 years ago
|
Attachment #8418872 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•11 years ago
|
Attachment #8419219 -
Flags: review?(drno)
Attachment #8419219 -
Flags: feedback?(rjesup)
Attachment #8419219 -
Flags: feedback+
Assignee | ||
Comment 29•11 years ago
|
||
Comment on attachment 8419219 [details] [diff] [review]
disable webrtc tests when webrtc is not built
Review of attachment 8419219 [details] [diff] [review]:
-----------------------------------------------------------------
This might need disabling steeplechase as well, as per https://pastebin.mozilla.org/5095025 (though that misses disabling the crashtests). I'd be good with the pastebin patch instead, with disabling crashtests added to it.
Comment 30•11 years ago
|
||
Yeah but from what i understand, to disable crashtests id' still need the layout/tools/reftest/reftest.js chunk adding webrtc status to the sandbox, no ?
Assignee | ||
Comment 31•11 years ago
|
||
(In reply to Landry Breuil (:gaston) from comment #30)
> Yeah but from what i understand, to disable crashtests id' still need the
> layout/tools/reftest/reftest.js chunk adding webrtc status to the sandbox,
> no ?
Yes, I believe so.
Comment 32•11 years ago
|
||
integrating jesup's feedback
Attachment #8419219 -
Attachment is obsolete: true
Attachment #8419219 -
Flags: review?(drno)
Attachment #8419245 -
Flags: review?(drno)
Comment 33•11 years ago
|
||
Comment on attachment 8418872 [details] [diff] [review]
fix disable-webrtc on linux/etc
Review of attachment 8418872 [details] [diff] [review]:
-----------------------------------------------------------------
::: build/gyp.mozbuild
@@ +6,5 @@
>
> gyp_vars = {
> 'build_with_mozilla': 1,
> 'build_with_chromium': 0,
> + 'use_official_google_api_keys': 0,
Sounds like this should go in its own bug?
::: media/libyuv/build/dir_exists.py
@@ +2,5 @@
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +# exists to keep common.gypi happy and avoid modifying it
Shouldn't this be a copy of media/webrtc/trunk/build/dir_exists.py ?
Attachment #8418872 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 34•11 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #33)
> > gyp_vars = {
> > 'build_with_mozilla': 1,
> > 'build_with_chromium': 0,
> > + 'use_official_google_api_keys': 0,
>
> Sounds like this should go in its own bug?
a) we should never have left it in (though it didn't hurt other than running a script, b) if it defaults, common.gypi needs that script and it's hard to get it to find it.
> ::: media/libyuv/build/dir_exists.py
> @@ +2,5 @@
> > +# This Source Code Form is subject to the terms of the Mozilla Public
> > +# License, v. 2.0. If a copy of the MPL was not distributed with this
> > +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> > +
> > +# exists to keep common.gypi happy and avoid modifying it
>
> Shouldn't this be a copy of media/webrtc/trunk/build/dir_exists.py ?
It could - but then I have to get a license patch approval from gerv, and all I want here is to make this fail to find Win32 SDK files.
Comment 35•11 years ago
|
||
Comment on attachment 8419245 [details] [diff] [review]
disable webrtc tests when webrtc is not built
Review of attachment 8419245 [details] [diff] [review]:
-----------------------------------------------------------------
lgtm
Attachment #8419245 -
Flags: review?(drno) → review+
Assignee | ||
Comment 36•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a3a67e1ded26
https://hg.mozilla.org/integration/mozilla-inbound/rev/33e91919b06b
Whiteboard: [webrtc-uplift]
Target Milestone: --- → mozilla32
Comment 37•11 years ago
|
||
Backed out for OSX bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7a931fe4f40
https://tbpl.mozilla.org/php/getParsedLog.php?id=39378455&tree=Mozilla-Inbound
Assignee: nobody → rjesup
Assignee | ||
Comment 38•11 years ago
|
||
updated to fix mac and bounce the scripts to the ones in the right dirs
Assignee | ||
Updated•11 years ago
|
Attachment #8420376 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•11 years ago
|
Attachment #8418872 -
Attachment is obsolete: true
Assignee | ||
Comment 39•11 years ago
|
||
Comment 40•11 years ago
|
||
All green on try, can we move forward on this ? I'll need to get it backported to aurora/beta....
Comment 41•11 years ago
|
||
Comment on attachment 8420376 [details] [diff] [review]
fix disable-webrtc
Review of attachment 8420376 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/libyuv/build/dir_exists.py
@@ +4,5 @@
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +# exists to keep common.gypi happy and avoid modifying it
> +def main():
> + return subprocess.call("../../webrtc/trunk/build/dir_exists.py",sys.argv[1:])
I'd be surprised if that actually worked. subprocess.call expects a single list for executable + arguments. You're giving the executable and a list of arguments. BTW, it would also be better to use sys.executable as executable and pass it the python script and the arguments.
Attachment #8420376 -
Flags: review?(mh+mozilla) → review-
Assignee | ||
Comment 42•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #8420376 -
Attachment is obsolete: true
Assignee | ||
Comment 43•11 years ago
|
||
Comment on attachment 8421534 [details] [diff] [review]
fix disable-webrtc
Hopefully better...
Attachment #8421534 -
Flags: review?(mh+mozilla)
Comment 44•11 years ago
|
||
Comment on attachment 8421534 [details] [diff] [review]
fix disable-webrtc
Review of attachment 8421534 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/libyuv/build/dir_exists.py
@@ +4,5 @@
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +# exists to keep common.gypi happy and avoid modifying it
> +def main():
> + return subprocess.call([sys.executable, "../../webrtc/trunk/build/dir_exists.py", sys.argv[1:]])
[...] + sys.argv[1:], otherwise you're calling subprocess.call(['foo', 'bar', ['baz']])
Attachment #8421534 -
Flags: review?(mh+mozilla) → feedback+
Assignee | ||
Comment 45•11 years ago
|
||
tested both manually and in builds. Also https://tbpl.mozilla.org/?tree=Try&rev=f55daf727ee7
Assignee | ||
Updated•11 years ago
|
Attachment #8421534 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #8421944 -
Flags: review?(mh+mozilla)
Comment 46•11 years ago
|
||
Comment on attachment 8421944 [details] [diff] [review]
fix disable-webrtc
Review of attachment 8421944 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/libyuv/build/dir_exists.py
@@ +8,5 @@
> +
> +def main():
> + return subprocess.call([sys.executable, "../webrtc/trunk/build/dir_exists.py"] + sys.argv[1:])
> +
> +print os.getcwd()
Do you really want this?
Attachment #8421944 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 47•11 years ago
|
||
Comment 48•11 years ago
|
||
fwiw, i can build m-i with --disable-webrtc fine with what was commited. Yay!
Comment 49•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ca72332a6bf9
https://hg.mozilla.org/mozilla-central/rev/1d2bbffee6ea
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 50•11 years ago
|
||
Comment on attachment 8421944 [details] [diff] [review]
fix disable-webrtc
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 970787
User impact if declined: No user impact; inability to make --disable-webrtc builds which are used by some downstream maintainers (and 1.3T)
Testing completed (on m-c, etc.): On m-c soon. Tested by BSD maintainer
Risk to taking this patch (and alternatives if risky): none, build config only
String or IDL/UUID changes made by this patch: none
Attachment #8421944 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•11 years ago
|
Attachment #8419245 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•11 years ago
|
Attachment #8421944 -
Flags: approval-mozilla-beta?
Assignee | ||
Updated•11 years ago
|
Attachment #8419245 -
Flags: approval-mozilla-beta?
Updated•11 years ago
|
Attachment #8419245 -
Flags: approval-mozilla-beta?
Attachment #8419245 -
Flags: approval-mozilla-beta+
Attachment #8419245 -
Flags: approval-mozilla-aurora?
Attachment #8419245 -
Flags: approval-mozilla-aurora+
Updated•11 years ago
|
Attachment #8421944 -
Flags: approval-mozilla-beta?
Attachment #8421944 -
Flags: approval-mozilla-beta+
Attachment #8421944 -
Flags: approval-mozilla-aurora?
Attachment #8421944 -
Flags: approval-mozilla-aurora+
Comment 51•11 years ago
|
||
Comment 52•11 years ago
|
||
http://buildbot.rhaalovely.net/builders/mozilla-aurora-sparc64/builds/422
http://buildbot.rhaalovely.net/builders/mozilla-central-sparc64/builds/801
32 & 31 confirmed building fine on sparc64 using --disable-webrtc (and a fix for 1005449)
Will test beta/30 with the next b5..
Comment 53•11 years ago
|
||
Updated•11 years ago
|
status-b2g-v1.4:
--- → fixed
status-b2g-v2.0:
--- → fixed
Updated•11 years ago
|
Whiteboard: [webrtc-uplift] → [webrtc-uplift] [qa-]
Assignee | ||
Updated•11 years ago
|
Whiteboard: [webrtc-uplift] [qa-] → [qa-]
Reporter | ||
Comment 54•11 years ago
|
||
FWIW, it looks like the patches that landed here may have introduced a version of the error in comment 5 ("ERROR PROCESSING MOZBUILD FILE" for libyuv), in some build configurations. See bug 982185 comment 6 and beyond for more.
You need to log in
before you can comment on or make changes to this bug.
Description
•