Crash in WebRtcVoiceEngine::Init() on Windows if audio sampling rate unsupported by webrtc

[drejx]

Hi,

I wanted to give you guys some information about a nasty bug I encountered, which is is more of a PSA as it's in the Windows m71 WebRtc implementation and not MixedReality.

Describe the bug
When creating a peer connection for a local (outgoing) track, specifically when initializing the audio with PeerConnection.InitializeAsync(), you can crash in native webrtc if your speaker or mic device is set to an unsupported sampling frequency (unsupported in the webrtc lib).

Crash Callstack

> Microsoft.MixedReality.WebRTC.Native.dll!abort() Line 77 C++
  Microsoft.MixedReality.WebRTC.Native.dll!rtc::webrtc_checks_impl::FatalLog(char const *,int,char const *,enum rtc::webrtc_checks_impl::CheckArgType const *,...) Unknown
  Microsoft.MixedReality.WebRTC.Native.dll!cricket::WebRtcVoiceEngine::Init(void) Unknown
  Microsoft.MixedReality.WebRTC.Native.dll!cricket::CompositeMediaEngine<class cricket::WebRtcVoiceEngine,class cricket::WebRtcVideoEngine>::Init(void) Unknown
  Microsoft.MixedReality.WebRTC.Native.dll!cricket::ChannelManager::Init(void) Unknown
  Microsoft.MixedReality.WebRTC.Native.dll!rtc::MessageQueue::Dispatch(struct rtc::Message *) Unknown
  Microsoft.MixedReality.WebRTC.Native.dll!rtc::Thread::ReceiveSendsFromThread(class rtc::Thread const *) Unknown
  Microsoft.MixedReality.WebRTC.Native.dll!rtc::MessageQueue::Get(struct rtc::Message *,int,bool) Unknown
  Microsoft.MixedReality.WebRTC.Native.dll!rtc::Thread::ProcessMessages(int) Unknown
  Microsoft.MixedReality.WebRTC.Native.dll!rtc::Thread::PreRun(void *) Unknown

Root Cause
The Windows implementation of the m71 version of webrtc has a hardcoded array of sampling frequencies that it supports for speakers and mics:

{48000, 44100, 16000, 96000, 32000, 8000, 24000}

If your speaker or mic is set to something other than one of those above then the webrtc code does a forced crash, yup really. I was able to reproduce this by going into Windows 10 sound settings and choosing a 192000Hz sampling rate on the speakers then running my app resulting in a crash. Then I switched it to 48000Hz and it didn't crash anymore. Same for a mic at 22050Hz (crash), then switched to 44100Hz (no crash).

The crash occurs in WebRtcVoiceEngine::Init(). Here is the path the logic goes through to result in the crash:

WebRtcVoiceEngine::Init()
    ->  webrtc::AudioDeviceModule::Create(webrtc::AudioDeviceModule::kPlatformDefaultAudio)
        -> AudioDeviceModule::CreateForTest(audio_layer)
            -> AudioDeviceModuleImpl::CreatePlatformSpecificObjects()
                -> AudioDeviceWindowsCore::CoreAudioIsSupported()

                    ===> Here if *any* of the recording or playback devices fail an availability check
                         this method returns false, which bubbles back up to WebRtcVoiceEngine::Init()
                         where a RTC_CHECK macro on the failed result produces a forced crash

So AudioDeviceWindowsCore::InitPlayout() in webrtc\modules\audio_device\win\audio_device_core_win.cc for the hardcoded array of sampling
frequencies -> {48000, 44100, 16000, 96000, 32000, 8000, 24000}.

Environment

• Platform: Windows 10, Unity game engine

Additional context
This totally suuuuuucks!
The unfortunate part is this problem is not some edge case because there are plenty of people that have speaker/mic sampling rates set which would fail. Also from digging a bit in the webrtc Google group it appears the webrtc team is focusing on a new Windows Audio Module (ADM Core Audio 2) so they're not fixing bugs in the m71 release.

I think there is a way one can write your own ADM and pass it to libwebrtc so that your custom ADM overrides the webrtc default, without having to compile libwebrtc yourself, but I'm not entirely sure. Anyway I just wanted to let you and the community know so if anyone runs into this they won't be banging their head on a solid surface...much ;)

Andrej

[djee-ms]

Hi Andrej,

Yes I already know all of that, and I agree it's bad. I share your pain!

• Audio init happens at startup whether you need audio or not; this means you need to add the UWP microphone capability otherwise the app crashes on UWP, even for using data channels only (!).
• Anything that makes audio fail to initialize triggers an assert in the voice engine startup code and crash the application.
• This is very confusing to many users, rightfully so as you pointed
• This is the number one blocker to supporting Azure Kinect DK which has a 7-channel microphone array, which fails because ADM1 supports only 1/2/(4) channels and crash in the same way.

I already argued several times with the Google WebRTC team about it. They have a weird position where ADM1 is the default but is deprecated (won't fix bugs), while ADM2 is marked as experimental and you can't build it without modifying the source code (I have an open bug for that). I have currently a local modification on my machine in experimental testing for using ADM2 and we might push it soon, but need to make some code change in Google's code which is annoying to maintain.

[drejx]

Hi Jerome,

Oh wow that is definitely the definition of stuck between a rock and a hard place! I'm not sure about what the webrtc team's intent was, but I feel like ADM1 is almost like sample/inspiration code and they expect users to build out their own ADM implementation. But as far as you know is ADM2 more robust with handling audio devices?

The change you have for ADM2 on your local machine would be pulling in a newer version of webrtc > m71, or would it be sorta patched over m71? I don't quite recall, but the reason for MixedReality sticking with m71 is because of the dependency on webrtc-uwp-sdk, which uses m71? I recall some issue also being support for the visual studio build chain.

Cheers,
Andrej

[djee-ms]

I feel like ADM1 is almost like sample/inspiration code and they expect users to build out their own ADM implementation

I reached the same conclusion. Chromium has its own A/V stack. The one in the WebRTC repo is only used by Google for testing as far as I know.

The change you have for ADM2 on your local machine would be pulling in a newer version of webrtc > m71, or would it be sorta patched over m71?

Patch over WebRTC UWP m71. It's a simple commit. The burden is maintenance, more than the change itself. I also have to make sure audio works as intended on all platforms/configs; I didn't do that yet.

the reason for MixedReality sticking with m71 is because of the dependency on webrtc-uwp-sdk, which uses m71

Yes. We are working on undocking from that project and consuming directly the Google code which will allow us to upgrade, but this takes time and we're not a parity yet so cannot flip the switch (no video on UWP so far).

I recall some issue also being support for the visual studio build chain.

The VS toolchain kind of work on Google's master. UWP is half-broken, but WinRTC has patches to help us. Also there were some fixes recently in VS adding support for missing ARM64 NEON instructions (I think from 16.5) so that one issue went away.

[drejx]

Thanks for that info. You definitely have a full buffet of tasks ahead of you :)

I just found #242 which you created that details upgrading to ADM2, so I'll close this this issue. It can be considered a reference if someone happens to search for that offending crash callstack ;)

In my case I'm actually going full steam ahead on mobile and would love to use MixedReality, but I'm unsure about Android/iOS status. Would you mind responding to my comment here when you have a chance: #28 (comment)

Cheers,
Andrej