Import updated libyuv for webrtc

RESOLVED FIXED in mozilla30

Status

()

Core
WebRTC: Audio/Video
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: jesup, Assigned: jesup)

Tracking

(Blocks: 1 bug)

22 Branch
mozilla30
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [WebRTC] [blocking-webrtc-])

Attachments

(5 attachments, 5 obsolete attachments)

(Assignee)

Description

5 years ago
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

5 years ago
Created attachment 759375 [details] [diff] [review]
libyuv import of rev 719 June 06 2013 15:49:00
(Assignee)

Comment 2

5 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

5 years ago
Created attachment 759447 [details] [diff] [review]
rollup of modifications to libyuv
(Assignee)

Comment 4

5 years ago
builds and runs with these mods of changes to the old rev of libyuv
Attachment #759375 - Flags: feedback?(tterribe) → feedback+
Don't forget to take into account bug 878446 in the rollup mods patch.
(Assignee)

Updated

5 years ago
Depends on: 880879
(Assignee)

Updated

5 years ago
Depends on: 959773
(Assignee)

Comment 6

5 years ago
Created attachment 8359977 [details] [diff] [review]
libyuv import of rev 971 Tue Jan 14 14:04:08 EST 2014
(Assignee)

Comment 7

5 years ago
Created attachment 8359979 [details] [diff] [review]
rollup of modifications to libyuv
(Assignee)

Updated

5 years ago
Attachment #759375 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #759447 - Attachment is obsolete: true
(Assignee)

Comment 8

5 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

5 years ago
Created attachment 8360527 [details] [diff] [review]
disable AVX2 on GCC until bug 959773 lands
(Assignee)

Comment 10

5 years ago
Created attachment 8360907 [details] [diff] [review]
rollup of modifications to libyuv

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

5 years ago
Attachment #8359979 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Blocks: 813645
(Assignee)

Comment 11

5 years ago
Created attachment 8367169 [details] [diff] [review]
disable AVX2 asm if the compiler/assembler don't support it
(Assignee)

Updated

5 years ago
Attachment #8360527 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #8367169 - Flags: review?(mh+mozilla)
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+
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

5 years ago
Created attachment 8368600 [details] [diff] [review]
make sure that NEON detection is on for B2G libyuv

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

5 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

5 years ago
Created attachment 8368848 [details] [diff] [review]
make sure that NEON detection is on for B2G libyuv, make sure NEON ifdefs match
(Assignee)

Updated

5 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

5 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

5 years ago
Created attachment 8369290 [details] [diff] [review]
minor tweaks for building Android ARMv6 (we define HAVE_ARM_NEON...)

https://tbpl.mozilla.org/?tree=Try&rev=efe53ce2c5d1 is green on ARMv6 (and others too)
(Assignee)

Updated

5 years ago
Attachment #8369290 - Flags: review?(mh+mozilla)
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 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)

Updated

3 years ago
Depends on: 1151628
(Assignee)

Updated

3 years ago
Depends on: 1152016

Updated

2 years ago
See Also: → bug 1282711

Updated

2 years ago
Blocks: 1284800

Updated

2 years ago
See Also: bug 1282711

Updated

2 years ago
See Also: → bug 1284803
You need to log in before you can comment on or make changes to this bug.