Closed
Bug 880419
Opened 11 years ago
Closed 10 years ago
Import updated libyuv for webrtc
Categories
(Core :: WebRTC: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: jesup, Assigned: jesup)
References
Details
(Whiteboard: [WebRTC] [blocking-webrtc-])
Attachments
(5 files, 5 obsolete files)
2.34 MB,
patch
|
Details | Diff | Splinter Review | |
7.22 KB,
patch
|
Details | Diff | Splinter Review | |
5.00 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
5.41 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
2.86 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
Since we're updating webrtc, we should update the dependent projects This updates libyuv to rev719 Note that this will require a license update for src/x86inc.asm
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
Comment on attachment 759375 [details] [diff] [review] libyuv import of rev 719 June 06 2013 15:49:00 Note: this doesn't have a few make-related mods we've made applied yet f? to tim - any issues you can think of? I can easily switch to a different rev; I did this almost as a proof-of-concept of importing it separately.
Attachment #759375 -
Flags: feedback?(tterribe)
Assignee | ||
Comment 3•11 years ago
|
||
Assignee | ||
Comment 4•11 years ago
|
||
builds and runs with these mods of changes to the old rev of libyuv
Updated•11 years ago
|
Attachment #759375 -
Flags: feedback?(tterribe) → feedback+
Comment 5•11 years ago
|
||
Don't forget to take into account bug 878446 in the rollup mods patch.
Assignee | ||
Comment 6•10 years ago
|
||
Assignee | ||
Comment 7•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #759375 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #759447 -
Attachment is obsolete: true
Assignee | ||
Comment 8•10 years ago
|
||
Ok, needs an update to GCC (really bintools) from bug 938510 to work on the Linux builders. I'll disable AVX2 support until that lands; a better solution would be to predicate it on a configure.in check and add a #define to disable AVX2 support (overriding the compiler version checks in row.h). With AVX2 disabled, green on all Linux builds.
Depends on: 938510
Assignee | ||
Comment 9•10 years ago
|
||
Assignee | ||
Comment 10•10 years ago
|
||
updated to fix build issues with Android 2.x and B2G. B2G Desktop Linux still fails on configure (can't build executables), probably due to an issue with the GCC update patch (which hasn't landed yet due to a B2G problem). All other builds are clean.
Assignee | ||
Updated•10 years ago
|
Attachment #8359979 -
Attachment is obsolete: true
Assignee | ||
Comment 11•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8360527 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8367169 -
Flags: review?(mh+mozilla)
Comment 12•10 years ago
|
||
Comment on attachment 8367169 [details] [diff] [review] disable AVX2 asm if the compiler/assembler don't support it Review of attachment 8367169 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +1359,5 @@ > + result="yes", result="no") > + AC_MSG_RESULT("$result") > + if test "$result" = "yes"; then > + HAVE_X86_AVX2=1 > + AC_DEFINE(HAVE_X86_AVX2) You don't need the AC_DEFINE. @@ +1362,5 @@ > + HAVE_X86_AVX2=1 > + AC_DEFINE(HAVE_X86_AVX2) > + AC_SUBST(HAVE_X86_AVX2) > + fi > + esac As I mentioned on irc, it may be desirable to have a configure flag to make configure fail when the test fail, so that we ensure the avx assembly is not silently disabled on e.g. release linux builds (but then we should do the same for the other similar optional instruction sets, so it could be a followup).
Attachment #8367169 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 13•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a829f51aae56 https://hg.mozilla.org/integration/mozilla-inbound/rev/01334319137e https://hg.mozilla.org/integration/mozilla-inbound/rev/57d558b5d3df
Target Milestone: --- → mozilla29
Comment 14•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a829f51aae56 https://hg.mozilla.org/mozilla-central/rev/01334319137e https://hg.mozilla.org/mozilla-central/rev/57d558b5d3df
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 15•10 years ago
|
||
backed this out on central and integration branches as part of https://tbpl.mozilla.org/?rev=735a648bca0d because of the suspicion of causing bustages on device builds like https://tbpl.mozilla.org/php/getParsedLog.php?id=33874773&tree=Mozilla-Central
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 16•10 years ago
|
||
in the update, they added arm_neon_optional to the webrtc common.gypi, but our gyp.mozbuild doesn't define it, so when I moved libyuv out to a standalone, it broken B2G devicebuilds
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 8368600 [details] [diff] [review] make sure that NEON detection is on for B2G libyuv https://tbpl.mozilla.org/?tree=Try&rev=48a193ffab42 I'll take any build peer for review.... 1-liner fix. Try doesn't run device builds, but I ran a Helix local device build (failed without this, works with it)
Attachment #8368600 -
Flags: review?(ted)
Attachment #8368600 -
Flags: review?(mh+mozilla)
Attachment #8368600 -
Flags: review?(gps)
Assignee | ||
Comment 18•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8368600 -
Attachment is obsolete: true
Attachment #8368600 -
Flags: review?(ted)
Attachment #8368600 -
Flags: review?(mh+mozilla)
Attachment #8368600 -
Flags: review?(gps)
Assignee | ||
Comment 19•10 years ago
|
||
Comment on attachment 8368848 [details] [diff] [review] make sure that NEON detection is on for B2G libyuv, make sure NEON ifdefs match Upstream didn't have their ifdefs matching.... All the others use "if !disabled && (defined(__ARM_NEON__) || defined(LIBYUV_NEON))"; the *_neon.cc files left off LIBYUV_NEON
Attachment #8368848 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 20•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=efe53ce2c5d1 is green on ARMv6 (and others too)
Assignee | ||
Updated•10 years ago
|
Attachment #8369290 -
Flags: review?(mh+mozilla)
Comment 21•10 years ago
|
||
Comment on attachment 8369290 [details] [diff] [review] minor tweaks for building Android ARMv6 (we define HAVE_ARM_NEON...) Review of attachment 8369290 [details] [diff] [review]: ----------------------------------------------------------------- Please fold in the other one.
Attachment #8369290 -
Flags: review?(mh+mozilla) → review+
Comment 22•10 years ago
|
||
Comment on attachment 8368848 [details] [diff] [review] make sure that NEON detection is on for B2G libyuv, make sure NEON ifdefs match Review of attachment 8368848 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the code changes reverted. ::: media/libyuv/source/compare_neon.cc @@ +14,5 @@ > namespace libyuv { > extern "C" { > #endif > > +#if !defined(LIBYUV_DISABLE_NEON) && (defined(__ARM_NEON__) || defined(LIBYUV_NEON)) This is unnecessary. The reason the other parts have defined(LIBYUV_NEON) is because those other parts are *not* built with -mfpu=neon. ::: media/webrtc/signaling/test/Makefile.in @@ +17,5 @@ > $(DEPTH)/media/libyuv/libyuv_libyuv/$(LIB_PREFIX)yuv.$(LIB_SUFFIX) \ > $(DEPTH)/netwerk/srtp/src/$(LIB_PREFIX)nksrtp_s.$(LIB_SUFFIX) \ > $(NULL) > > +ifdef HAVE_ARM_NEON BUILD_ARM_NEON
Attachment #8368848 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 23•10 years ago
|
||
Green Try: https://tbpl.mozilla.org/?tree=Try&rev=859454ae37a3 https://hg.mozilla.org/integration/mozilla-inbound/rev/aaf73bfa3900 https://hg.mozilla.org/integration/mozilla-inbound/rev/ad4829ac770a https://hg.mozilla.org/integration/mozilla-inbound/rev/0930352a2b0e https://hg.mozilla.org/integration/mozilla-inbound/rev/326a283714a8
Target Milestone: mozilla29 → mozilla30
Comment 24•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/aaf73bfa3900 https://hg.mozilla.org/mozilla-central/rev/ad4829ac770a https://hg.mozilla.org/mozilla-central/rev/0930352a2b0e https://hg.mozilla.org/mozilla-central/rev/326a283714a8
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Updated•8 years ago
|
Blocks: libyuv-updates
You need to log in
before you can comment on or make changes to this bug.
Description
•