Closed Bug 837598 Opened 7 years ago Closed 7 years ago

cannot build WebRTC for Linux/arm

Categories

(Firefox Build System :: General, defect)

ARM
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla22

People

(Reporter: m_kato, Assigned: m_kato)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

We cannot build for Linux/arm target due to WebRTC build config.
What specific errors do you get? On Debian armhf, I'm getting:

media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699: undefined reference to `WebRtcAecm_WindowAndFFTNeon'
media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699: undefined reference to `WebRtcAecm_InverseFFTAndWindowNeon'
media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699: undefined reference to `WebRtcAecm_CalcLinearEnergiesNeon'
media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699: undefined reference to `WebRtcAecm_StoreAdaptiveChannelNeon'
media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699: undefined reference to `WebRtcAecm_ResetAdaptiveChannelNeon'
(In reply to Mike Hommey [:glandium] from comment #1)
> What specific errors do you get? On Debian armhf, I'm getting:
> 
> media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699:
> undefined reference to `WebRtcAecm_WindowAndFFTNeon'
> media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699:
> undefined reference to `WebRtcAecm_InverseFFTAndWindowNeon'
> media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699:
> undefined reference to `WebRtcAecm_CalcLinearEnergiesNeon'
> media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699:
> undefined reference to `WebRtcAecm_StoreAdaptiveChannelNeon'
> media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699:
> undefined reference to `WebRtcAecm_ResetAdaptiveChannelNeon'

(but that's on 18)
(In reply to Mike Hommey [:glandium] from comment #1)
> What specific errors do you get? On Debian armhf, I'm getting:
> 
> media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699:
> undefined reference to `WebRtcAecm_WindowAndFFTNeon'
> media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699:
> undefined reference to `WebRtcAecm_InverseFFTAndWindowNeon'
> media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699:
> undefined reference to `WebRtcAecm_CalcLinearEnergiesNeon'
> media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699:
> undefined reference to `WebRtcAecm_StoreAdaptiveChannelNeon'
> media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:699:
> undefined reference to `WebRtcAecm_ResetAdaptiveChannelNeon'

yes. 

- media/webrtc/trunk/src/build/arm_neon.gypi uses 'OS=="android".
- ARM_ARCH is set only when MOZ_LINKER is 1.  So if not Android, ARM_ARCH is never set.
Attached patch fixSplinter Review
Attachment #710071 - Flags: review?(mh+mozilla)
Comment on attachment 710071 [details] [diff] [review]
fix

Review of attachment 710071 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +3658,5 @@
>  
> +if test "$CPU_ARCH" = "arm"; then
> +    dnl Determine the target ARM architecture (5 for ARMv5, v5T, v5E, etc.; 6 for ARMv6, v6K, etc.)
> +    ARM_ARCH=`${CC-cc} ${CFLAGS} -dM -E - < /dev/null | sed -n 's/.*__ARM_ARCH_\([[0-9]]*\).*/\1/p'`
> +fi

Please move this in build/autoconf/arch.m4

@@ +8984,5 @@
>     else
> +      EXTRA_GYP_DEFINES="${EXTRA_GYP_DEFINES} -D armv7=1 "
> +      if $CC $CFLAGS -E -dM -</dev/null | grep -q __ARM_NEON__; then
> +          dnl default of compiler supports NEON
> +          EXTRA_GYP_DEFINES="${EXTRA_GYP_DEFINES} -D arm_neon=1 "

I'm not sure this is needed.

::: media/webrtc/trunk/src/build/arm_neon.gypi
@@ +20,5 @@
>  # }
>  
>  {
>    'conditions': [
> +    ['armv7==1', {

I'm not sure this is a good idea. vfpv3-d16 is a good common ground for armv7 chips. neon is not.
Attachment #710071 - Flags: review?(mh+mozilla) → review-
Attached patch fix v2Splinter Review
neon detection cannot compile on Linux/arm. (It works on Android only).  So we need armv7=0 on non-Android.
Attachment #711773 - Flags: review?(mh+mozilla)
Comment on attachment 711773 [details] [diff] [review]
fix v2

>Bug 837598. try: -b do -p all -u all -t none
>
>diff --git a/build/autoconf/arch.m4 b/build/autoconf/arch.m4
>--- a/build/autoconf/arch.m4
>+++ b/build/autoconf/arch.m4
>@@ -208,16 +208,21 @@ if test "$CPU_ARCH" = "arm"; then
>   AC_TRY_LINK([],
>                  [asm(".fpu neon\n vadd.i8 d0, d0, d0");],
>                  result="yes", result="no")
>   AC_MSG_RESULT("$result")
>   if test "$result" = "yes"; then
>       AC_DEFINE(HAVE_ARM_NEON)
>       HAVE_ARM_NEON=1
>   fi
>+
>+  AC_MSG_CHECKING(ARM version support in compiler)
>+  dnl Determine the target ARM architecture (5 for ARMv5, v5T, v5E, etc.; 6 for ARMv6, v6K, etc.)
>+  ARM_ARCH=`${CC-cc} ${CFLAGS} -dM -E - < /dev/null | sed -n 's/.*__ARM_ARCH_\([[0-9]]*\).*/\1/p'`
>+  AC_MSG_RESULT("$ARM_ARCH")
> fi # CPU_ARCH = arm
> 
> AC_SUBST(HAVE_ARM_SIMD)
> AC_SUBST(HAVE_ARM_NEON)
> 
> if test -n "$MOZ_ARCH"; then
>   NSPR_CONFIGURE_ARGS="$NSPR_CONFIGURE_ARGS --with-arch=$MOZ_ARCH"
>   NSPR_CONFIGURE_ARGS="$NSPR_CONFIGURE_ARGS --with-thumb=$MOZ_THUMB"
>diff --git a/configure.in b/configure.in
>--- a/configure.in
>+++ b/configure.in
>@@ -3654,18 +3654,16 @@ if test "$ac_cv_thread_keyword" = yes -a
>       MOZ_TLS=1
>       ;;
>   esac
> fi
> 
> dnl Using the custom linker on ARMv6 requires 16k alignment of ELF segments.
> if test -n "$MOZ_LINKER"; then
>   if test "$CPU_ARCH" = arm; then
>-    dnl Determine the target ARM architecture (5 for ARMv5, v5T, v5E, etc.; 6 for ARMv6, v6K, etc.)
>-    ARM_ARCH=`${CC-cc} ${CFLAGS} -dM -E - < /dev/null | sed -n 's/.*__ARM_ARCH_\([[0-9]]*\).*/\1/p'`
>     dnl When building for < ARMv7, we need to ensure 16k alignment of ELF segments
>     if test -n "$ARM_ARCH" && test "$ARM_ARCH" -lt 7; then
>       LDFLAGS="$LDFLAGS -Wl,-z,max-page-size=0x4000 -Wl,-z,common-page-size=0x4000"
>       _SUBDIR_LDFLAGS="$_SUBDIR_LDFLAGS -Wl,-z,max-page-size=0x4000 -Wl,-z,common-page-size=0x4000"
>     fi
>   fi
> fi
> 
>@@ -8968,21 +8966,31 @@ if test "${OS_TARGET}" = "WINNT"; then
>       OS_BITS=64
>    else
>       OS_BITS=32
>    fi
>    EXTRA_GYP_DEFINES="-D MSVS_VERSION=${_MSVS_VERSION} -D MSVS_OS_BITS=${OS_BITS}"
> 
> elif test "${OS_TARGET}" = "Android"; then
>    EXTRA_GYP_DEFINES="-D gtest_target_type=executable -D android_toolchain=${android_toolchain} -G os=android "
>-   if test -n "$ARM_ARCH" && test "$ARM_ARCH" -lt 7; then
>-      EXTRA_GYP_DEFINES+=" -D armv7=0 "
>-   else
>-      EXTRA_GYP_DEFINES+=" -D armv7=1 "
>-   fi
>+fi
>+
>+if test -n "$ARM_ARCH"; then
>+    if test "$ARM_ARCH" -lt 7; then
>+        EXTRA_GYP_DEFINES="${EXTRA_GYP_DEFINES} -D armv7=0 "
>+    else
>+        if test "${OS_TARGET}" = "Android"; then
>+            dnl To detection neon automatelly, set arm_neon=0
>+            EXTRA_GYP_DEFINES="${EXTRA_GYP_DEFINES} -D armv7=1 -D arm_neon=0 "
>+        else
>+            dnl CPU detection for ARM works on Android only.  armv7 always uses CPU detection, so
>+            dnl we have to set armv7=0 for non-Android target
>+            EXTRA_GYP_DEFINES="${EXTRA_GYP_DEFINES} -D armv7=0 "
>+        fi
>+    fi
> fi
> 
> if test -n "$MOZ_WEBRTC"; then
>    AC_MSG_RESULT("generating WebRTC Makefiles...")
> 
> dnl Any --include files must also appear in -D FORCED_INCLUDE_FILE= entries
> dnl so that regeneration via dependencies works correctly
>    WEBRTC_CONFIG="-D build_with_mozilla=1 --include ${srcdir}/media/webrtc/webrtc_config.gypi -D FORCED_INCLUDE_FILE=${srcdir}/media/webrtc/webrtc_config.gypi"
>diff --git a/js/src/build/autoconf/arch.m4 b/js/src/build/autoconf/arch.m4
>--- a/js/src/build/autoconf/arch.m4
>+++ b/js/src/build/autoconf/arch.m4
>@@ -208,16 +208,21 @@ if test "$CPU_ARCH" = "arm"; then
>   AC_TRY_LINK([],
>                  [asm(".fpu neon\n vadd.i8 d0, d0, d0");],
>                  result="yes", result="no")
>   AC_MSG_RESULT("$result")
>   if test "$result" = "yes"; then
>       AC_DEFINE(HAVE_ARM_NEON)
>       HAVE_ARM_NEON=1
>   fi
>+
>+  AC_MSG_CHECKING(ARM version support in compiler)
>+  dnl Determine the target ARM architecture (5 for ARMv5, v5T, v5E, etc.; 6 for ARMv6, v6K, etc.)
>+  ARM_ARCH=`${CC-cc} ${CFLAGS} -dM -E - < /dev/null | sed -n 's/.*__ARM_ARCH_\([[0-9]]*\).*/\1/p'`
>+  AC_MSG_RESULT("$ARM_ARCH")
> fi # CPU_ARCH = arm
> 
> AC_SUBST(HAVE_ARM_SIMD)
> AC_SUBST(HAVE_ARM_NEON)
> 
> if test -n "$MOZ_ARCH"; then
>   NSPR_CONFIGURE_ARGS="$NSPR_CONFIGURE_ARGS --with-arch=$MOZ_ARCH"
>   NSPR_CONFIGURE_ARGS="$NSPR_CONFIGURE_ARGS --with-thumb=$MOZ_THUMB"
>diff --git a/media/webrtc/shared_libs.mk b/media/webrtc/shared_libs.mk
>--- a/media/webrtc/shared_libs.mk
>+++ b/media/webrtc/shared_libs.mk
>@@ -47,21 +47,25 @@ WEBRTC_LIBS = \
> ifneq (,$(INTEL_ARCHITECTURE))
> WEBRTC_LIBS += \
>   $(call EXPAND_LIBNAME_PATH,video_processing_sse2,$(DEPTH)/media/webrtc/trunk/src/modules/modules_video_processing_sse2) \
>   $(call EXPAND_LIBNAME_PATH,aec_sse2,$(DEPTH)/media/webrtc/trunk/src/modules/modules_aec_sse2) \
>   $(NULL)
> endif
> 
> # neon for ARM
>+ifeq (Android,$(OS_TARGET))
>+# NEON detection on WebRTC is Android only. If WebRTC supports Linux/arm etc, we should remove
>+# OS check
> ifeq ($(HAVE_ARM_NEON),1)
> WEBRTC_LIBS += \
>   $(call EXPAND_LIBNAME_PATH,aecm_neon,$(DEPTH)/media/webrtc/trunk/src/modules/modules_aecm_neon) \
>   $(NULL)
> endif
>+endif
> 
> 
> # If you enable one of these codecs in webrtc_config.gypi, you'll need to re-add the
> # relevant library from this list:
> #
> #  $(call EXPAND_LIBNAME_PATH,G722,$(DEPTH)/media/webrtc/trunk/src/modules/modules_G722) \
> #  $(call EXPAND_LIBNAME_PATH,iLBC,$(DEPTH)/media/webrtc/trunk/src/modules/modules_iLBC) \
> #  $(call EXPAND_LIBNAME_PATH,iSAC,$(DEPTH)/media/webrtc/trunk/src/modules/modules_iSAC) \
>diff --git a/media/webrtc/trunk/src/build/arm_neon.gypi b/media/webrtc/trunk/src/build/arm_neon.gypi
>--- a/media/webrtc/trunk/src/build/arm_neon.gypi
>+++ b/media/webrtc/trunk/src/build/arm_neon.gypi
>@@ -15,29 +15,25 @@
> #   'sources': [
> #     'foo.c',
> #     'bar.cc',
> #   ],
> #   'includes': ['path/to/this/gypi/file'],
> # }
> 
> {
>-  'conditions': [
>-    ['OS=="android"', {
>-      'cflags!': [
>+  'cflags!': [
>+    '-mfpu=vfpv3-d16',
>+  ],
>+  'cflags_mozilla!': [
>         '-mfpu=vfpv3-d16',
>-      ],
>-      'cflags_mozilla!': [
>-        '-mfpu=vfpv3-d16',
>-      ],
>-      'cflags': [
>-        '-mfpu=neon',
>-        '-mfloat-abi=softfp',
>-        '-flax-vector-conversions',
>-      ],
>-      'cflags_mozilla': [
>-        '-mfpu=neon',
>-        '-mfloat-abi=softfp',
>-        '-flax-vector-conversions',
>-      ]
>-    }],
>+  ],
>+  'cflags': [
>+    '-mfpu=neon',
>+    '-mfloat-abi=softfp',
>+    '-flax-vector-conversions',
>+  ],
>+  'cflags_mozilla': [
>+    '-mfpu=neon',
>+    '-mfloat-abi=softfp',
>+    '-flax-vector-conversions',
>   ],
> }
Attachment #711773 - Attachment is patch: true
Also, 'OS=="android" is Mozilla's modification.  This is unnecessary.
Comment on attachment 711773 [details] [diff] [review]
fix v2

Review of attachment 711773 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +8977,5 @@
> +    if test "$ARM_ARCH" -lt 7; then
> +        EXTRA_GYP_DEFINES="${EXTRA_GYP_DEFINES} -D armv7=0 "
> +    else
> +        if test "${OS_TARGET}" = "Android"; then
> +            dnl To detection neon automatelly, set arm_neon=0

"arm_neon=0" enables NEON autodetection.

@@ +8982,5 @@
> +            EXTRA_GYP_DEFINES="${EXTRA_GYP_DEFINES} -D armv7=1 -D arm_neon=0 "
> +        else
> +            dnl CPU detection for ARM works on Android only.  armv7 always uses CPU detection, so
> +            dnl we have to set armv7=0 for non-Android target
> +            EXTRA_GYP_DEFINES="${EXTRA_GYP_DEFINES} -D armv7=0 "

If that's really the case, we ought to fix this in a followup.

::: media/webrtc/trunk/src/build/arm_neon.gypi
@@ +27,1 @@
>          '-mfpu=vfpv3-d16',

indentation
Attachment #711773 - Flags: review?(mh+mozilla) → review+
Not sure if my version would help here but it looks similar in some parts
is anyone going to address review comments and push it ?
ah, I forget landing this.  I will land this soon.
https://hg.mozilla.org/mozilla-central/rev/989966a6e245
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.