815883 - fix WebRTC builds for ARM chips with neon FPUs

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P2)

Description

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Patch 1, courtesy of gcp

FWIW, I haven't yet come across build breakage that this patch would fix.  Or is this something that would be a problem at runtime?

Comment 1

[Timothy B. Terriberry (:derf)]

Comment on attachment 685895 [details] [diff] [review]
Patch 1, courtesy of gcp

> -    ['target_arch=="arm" and armv7==1', {
> +    ['target_arch=="arm" and (armv7==1 or arm_neon==1)', {

When do we have armv7!=1 but arm_neon==1?

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

FWIW, I see what this patch fixes now:

/home/morbo/hg/mozilla-central/media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:631: error: undefined reference to 'WebRtcAecm_WindowAndFFTNeon'
/home/morbo/hg/mozilla-central/media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:631: error: undefined reference to 'WebRtcAecm_InverseFFTAndWindowNeon'
/home/morbo/hg/mozilla-central/media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:631: error: undefined reference to 'WebRtcAecm_CalcLinearEnergiesNeon'
/home/morbo/hg/mozilla-central/media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:631: error: undefined reference to 'WebRtcAecm_StoreAdaptiveChannelNeon'
/home/morbo/hg/mozilla-central/media/webrtc/trunk/src/modules/audio_processing/aecm/aecm_core.c:631: error: undefined reference to 'WebRtcAecm_ResetAdaptiveChannelNeon'

I'll leave derf's question for gcp, as I have no idea...

Comment 3

[Gian-Carlo Pascutto [:gcp]]

>When do we have armv7!=1 but arm_neon==1?

Always, it's the default - that's why this patch was needed.

You're right that it makes no sense (because ARMv6 can never have NEON, right?). I guess common.gypi isn't pickup up our ARMv6 vs ARMv7 configuration correctly.

Comment 4

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. v2 Fix mozilla cflags, pass down arch.

Cleaner fix taking into account derf's remarks.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 688267 [details] [diff] [review]
Patch 1. v2 Fix mozilla cflags, pass down arch.

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

r=dmose

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Landed on mozilla-inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/5f9c909db022

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Setting in-testsuite-: since this is a build fix, the compiler acts an automated test.

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/5f9c909db022