1486038 - fix webrtc compilation errors on aarch64 windows

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P3)

Description

[Nathan Froyd [:froydnj]]

Things I've found so far once some of the basic problems in the headers are fixed:

 0:17.83 Unified_c_isac_fix_c_gn1.c
 0:17.84 c:\mozilla-central\VC_PATH\include\arm_neon.h(17): fatal error C1189: #error:  This header is specific to ARM targets
 0:17.84 mozmake.EXE[4]: *** [c:/mozilla-central/config/rules.mk:780: Unified_c_isac_fix_c_gn1.obj] Error 2
 0:17.84 mozmake.EXE[3]: *** [c:/mozilla-central/config/recurse.mk:74: media/webrtc/trunk/webrtc/modules/audio_coding/isac_fix_c_gn/target] Error 2

I suspect there are going to be a couple cases of this...though it's possible that this error is fixed with MSVC 15.8.  I'm just using the preview here.

 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/ns_core.c(219): error C2365: 'NoiseEstimation': redefinition; previous definition was 'typedef'
 0:18.53 c:\mozilla-central\media\webrtc\trunk\webrtc/modules/audio_processing/ns/nsx_core.h(175): note: see declaration of 'NoiseEstimation'
 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/ns_core.c(1108): error C2143: syntax error: missing ')' before ','
 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/ns_core.c(1108): error C2082: redefinition of formal parameter 'self'
 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/ns_core.c(1108): error C2040: 'magn': 'int' differs in levels of indirection from 'float [129]'
 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/ns_core.c(1108): error C2059: syntax error: ')'
 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/ns_core.c(1108): error C2040: 'noise': 'int' differs in levels of indirection from 'float [129]'
 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/nsx_core.c(303): error C2084: function 'void UpdateNoiseEstimate(NoiseSuppressionC *,const float *,const float *,const float *,float *)' already has a body
 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/ns_core.c(801): note: see previous definition of 'UpdateNoiseEstimate'
 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/nsx_core.c(437): warning C4133: 'function': incompatible types - from 'NoiseSuppressionFixedC *' to 'NoiseSuppressionC *'
 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/nsx_core.c(437): warning C4047: 'function': 'const float *' differs in levels of indirection from 'size_t'
 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/nsx_core.c(437): warning C4024: 'UpdateNoiseEstimate': different types for formal and actual parameter 2
 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/nsx_core.c(437): error C2198: 'UpdateNoiseEstimate': too few arguments for call
 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/nsx_core.c(446): warning C4133: 'function': incompatible types - from 'NoiseSuppressionFixedC *' to 'NoiseSuppressionC *'
 0:18.53 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/nsx_core.c(446): warning C4047: 'function': 'const float *' differs in levels of indirection from 'size_t'
 0:18.55 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/nsx_core.c(446): warning C4024: 'UpdateNoiseEstimate': different types for formal and actual parameter 2
 0:18.55 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_processing/ns/nsx_core.c(446): error C2198: 'UpdateNoiseEstimate': too few arguments for call
 0:18.55 mozmake.EXE[4]: *** [c:/mozilla-central/config/rules.mk:779: Unified_c_udio_processing_c_gn0.obj] Error 2
 0:18.55 mozmake.EXE[3]: *** [c:/mozilla-central/config/recurse.mk:74: media/webrtc/trunk/webrtc/modules/audio_processing/audio_processing_c_gn/target] Error 2

I'm unclear how we're not running into this error on other platforms.

 0:21.66 Unified_cpp_audio_device_gn0.cpp
 0:21.66 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_device/opensl/single_rw_fifo.cc(35): error C2182: '__dmb': illegal use of type 'void'
 0:21.66 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_device/opensl/single_rw_fifo.cc(35): error C7525: inline variables require at least '/std:c++17'
 0:21.66 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_device/opensl/single_rw_fifo.cc(35): error C2448: 'webrtc::subtle::__dmb': function-style initializer appears to be a function definition
 0:21.66 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_device/opensl/single_rw_fifo.cc(102): error C2064: term does not evaluate to a function taking 1 arguments
 0:21.66 c:/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_device/opensl/single_rw_fifo.cc(117): error C2064: term does not evaluate to a function taking 1 arguments
 0:21.66 mozmake.EXE[4]: *** [c:/mozilla-central/config/rules.mk:1108: Unified_cpp_audio_device_gn0.obj] Error 2

This stems from the code trying to do:

void MemoryBarrier() { ::MemoryBarrier(); }

and running into problems because MemoryBarrier is a #define on AArch64 Windows.

Comment 1

[(Away)]

> ns_core.c(219): error C2365: 'NoiseEstimation': redefinition; previous
> definition was 'typedef'

> c:\mozilla-central\media\webrtc\trunk\webrtc/modules/audio_processing/ns/
> nsx_core.h(175): note: see declaration of 'NoiseEstimation'

> I'm unclear how we're not running into this error on other platforms.

We appear to be compiling both ns_core.c and nsx_core.c, which I suspect are mutually exclusive. Maybe it's from here? https://searchfox.org/mozilla-central/rev/3c85ea2f8700ab17e38b82d77cd44644b4dae703/media/webrtc/trunk/webrtc/modules/audio_processing/audio_processing_c_gn/moz.build#158,168

https://searchfox.org/mozilla-central/rev/3c85ea2f8700ab17e38b82d77cd44644b4dae703/media/webrtc/trunk/webrtc/build/webrtc.gni#97 and https://searchfox.org/mozilla-central/rev/3c85ea2f8700ab17e38b82d77cd44644b4dae703/media/webrtc/trunk/webrtc/modules/audio_processing/BUILD.gn#259 suggest that maybe the `x` one is the one we want.

Comment 2

[(Away)]

> > ns_core.c(219): error C2365: 'NoiseEstimation': redefinition; previous
> > definition was 'typedef'

> We appear to be compiling both ns_core.c and nsx_core.c, 

I split this out into bug 1503359.

Comment 3

[Dan Minor [:dminor]]

I think you will need run gn on aarch64 windows and generate new moz.build files according to these instructions [1]. It appears the errors are showing up in code generated moz.build files. If gn won't run on that platform, you can probably copy and modify one of the existing json files for a similar platform.

I'll be landing an update to the webrtc.org code very soon in Bug 1376873, everything is reviewed and I'm doing some final tests and fixes. There's probably not much point in regenerating moz.build files until after that lands.

[1] https://firefox-source-docs.mozilla.org/build/buildsystem/gn.html

Comment 4

[(Away)]

>  0:17.84 c:\mozilla-central\VC_PATH\include\arm_neon.h(17): fatal error
> C1189: #error:  This header is specific to ARM targets

Looks like we have to include arm64_neon.h instead.

Comment 5

[(Away)]

With some NEON hackery, I have a green webrtc build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=38f5a41f37a6

But if we build with clang-cl, we wouldn't need those hacks. I'm inclined not to land them (esp. because it's more work when the files are not ours) and instead just wait for the clang transition.

By the way, this issue has gone away:

> single_rw_fifo.cc(35): error C2448: 'webrtc::subtle::__dmb': function-style
> initializer appears to be a function definition
...
> and running into problems because MemoryBarrier is a #define on AArch64
> Windows.

single_rw_fifo.cc, while still in the tree, is no longer reachable in the build system as of the webrtc 64 update.

Comment 6

[Nathan Froyd [:froydnj]]

I know it's msvc hackery, but I think it might be worth landing, even if we have to back it out later, just so people can start banging on the webrtc support.  WDYT?

Comment 7

[(Away)]

(In reply to Nathan Froyd [:froydnj] from comment #6)
> I know it's msvc hackery, but I think it might be worth landing, even if we
> have to back it out later, just so people can start banging on the webrtc
> support.  WDYT?

It depends how easily we can modify these files, since they don't originate in our repo. If we're allowed to just stomp on the files, ok. If we have to carry local diff files and/or send patches upstream then I feel very much unmotivated to do it.

dminor, can you remind me what's the policy for webrtc changeS?

Comment 8

[Dan Minor [:dminor]]

Discussed this on IRC, as long as we file a bug to revert these changes once we are able to build with clang, I'm fine with taking them for now.

If we wanted to keep them around longer, we should attempt to get them upstreamed.

Comment 9

[(Away)]

Attachment: Bug 1486038: Use arm64_neon.h instead of arm_neon.h with MSVC.

Comment 10

[(Away)]

Attachment: Bug 1486038: Work around missing ARM64 NEON intrinsics in MSVC.

Most of the awkwardness is around making sure we capture MSVC only and not clang-cl (which defines both _MSC_VER and __clang__).

Also, I did these on an error-by-error basis, rather than e.g. replacing all uses of WEBRTC_ARCH_ARM64, because MSVC is only missing _some_ of the intrinsics.

Comment 11

[(Away)]

Attachment: Bug 1486038: Enable webrtc in aarch64-windows builds.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/06d49dc866e8
Use arm64_neon.h instead of arm_neon.h with MSVC. r=dminor
https://hg.mozilla.org/integration/mozilla-inbound/rev/304b5b25af77
Work around missing ARM64 NEON intrinsics in MSVC. r=dminor
https://hg.mozilla.org/integration/mozilla-inbound/rev/371ce9714a3e
Enable webrtc in aarch64-windows builds. r=dminor

Comment 13

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/06d49dc866e8
https://hg.mozilla.org/mozilla-central/rev/304b5b25af77
https://hg.mozilla.org/mozilla-central/rev/371ce9714a3e