Android rerouting fix

[znakeeye]

Fix for #913.

History

In our game engine, orx, we switched to miniaudio in late 2021. Ever since then, we observed a rare, intermittent problem on Android where audio would disappear (and never come back). Usually, it would happen when resuming a game after a few days. Then, in 2024, we found out that the issue is more likely to occur when taking a ride with the car 😆. It just happens that the car has a bluetooth audio device...

During these years, I have tried multiple devices, all running the latest Android version (API level 30 up to 35). Just to name a few:

• Pixel 5
• Pixel 6
• Pixel 9 Pro
• Lenovo P12 Pro
• Samsung S7 (if I recall correctly)

What is the common denominator here? Re-routing!

Symptoms

When re-routing, volume is usually lowered when returning back to the original device (this too is an important clue). Sometimes though, the audio is completely gone. In logcat we get a hint of what happened:

restartIfDisabled(7071): releaseBuffer() track 0xb40000750d625420 disabled due to previous underrun, restarting

And you get this error for both AAudio and OpenSL. Thus, these two APIs overlap somehow. As it turns out, AAudio has two paths; mmap and legacy, where the former is more performant. When re-routing, our AAudio integration seems to choose the legacy path. This indicates that it fails to pick the mmap path for some reason. That's odd!

Now, if we enforce the mmap path by calling AAudio_setMMapPolicy(AAUDIO_POLICY_ALWAYS), re-routing always fails at this line:

result = ma_device_init__aaudio(pDevice, &deviceConfig, &descriptorPlayback, &descriptorCapture);

Error is AAUDIO_ERROR_OUT_OF_RANGE due to the buffer size logic in AudioStreamInternal.cpp(295).

Clearly, re-routing does not work for mmap enabled devices. Makes no sense!

The fix

It turns out that there are fragments in the code that aim to handle this very issue.

if (!pConfig->aaudio.enableCompatibilityWorkarounds || ma_android_sdk_version() > 30)

Indeed, removing the workaround fixes the problem! Re-routing now maintains the mmap path, avoiding the less stable legacy path. Also, when reading information about the default device via ma_open_stream_basic__aaudio we now set AAUDIO_PERFORMANCE_MODE_LOW_LATENCY, allowing for the mmap path. This likely reduces the "path confusion" inside AAudio. Ideally, I would like to have the same logic as in ma_create_and_configure_AAudioStreamBuilder__aaudio but this is the best I can come up with when there is no ma_device_config to consider.

Final notes

You might want to re-iterate the removal of enableCompatibilityWorkarounds. However, since I have struggled with this issue since 2021 I'm quite confident that the "compatibility workarounds" were added at a time when AAudio still had some growing pains. My take is that the workarounds are no longer needed.

[znakeeye]

Commit b8cffea is interesting. You can try it yourself. I finally found a way to reproduce this rare case.

1. Put a breakpoint just before the call to ma_device_start__aaudio(pDevice)
2. Attach bluetooth device.
3. When breakpoint is hit, disable bluetooth in OS settings.
4. Continue. ma_device_start__aaudio fails with AAUDIO_ERROR_DISCONNECTED.

Now that the bluetooth device got disconnected, we have to re-route again. There is no need to post the job, so I intend to optimize this a bit.

[znakeeye]

Any chance you can merge this in a couple of days or so? We are eager to try it out in github/orx.

[mackron]

Thanks a lot for all your work on this. Just giving this a review now. A few things:

1. I'm still questioning if I want to fully disable that AAudioStreamBuilder_setBufferCapacityInFrames/setFramesPerDataCallback branch. What I'm fighting with is when I finally get this dev branch merged I don't want anyone to get a surprise when there's a change of behaviour with their buffer configuration. I'm wondering if in the current 0.11.x cycle I have a config option in ma_device_config to entirely disable it which you can make use of, and then later on maybe make that the default in the 0.12 release?

2. I don't like the use of ma_device__set_state(pDevice, ma_device_state_stopped); in ma_device_reinit__aaudio(). The intent was for ma_device__set_state() to only be used at the cross-platform layer of miniaudio. Have you by chance tried using ma_device_stop() at that part instead? Could we perhaps have ma_job_process__device__aaudio_reroute() call ma_device_stop() when ma_device_reinit__aaudio() returns an error? That should also trigger a stop notification in the result of an error.

[znakeeye]

Sure. As long as I can disable that buffer logic using some public API.

Can you perhaps add your suggestions to this PR, so I can try it out?

As for the buffer workaround, I'm quite sure it is broken. It fights with some weird "divide by 2" logic in AAudio internals and it is (mathematically) guaranteed to fail. Maybe I'll find time, this weekend, to explain this in detail.

Regarding 2, I added those for completeness. Didn't realize those functions had these constraints. Not needed for the PR at least.

[mackron]

Yeah what I was thinking was a variable like deviceConfig.aaudio.noSetBufferCapacity = MA_TRUE; which you just set in the device config. But having thought about it a bit more, it's sounding like eliminating the setting of the buffer capacity is the more stable option, so it makes sense that that'd be the default. Still, I'd like to have a way for people to invoke that branch if they still want it, so I'll still make an adjustment to your PR for that.

[mackron]

I've made some changes of my own. Summary of my changes:

1. I've added back the enableCompatibilityWorkarounds config variable because I don't want to break anyone's build. In the 0.12 release I'll probably remove that and instead use specific variables for each specific workaround and backdoor I might add in the future.

2. I've added a new device config variable called aaudio.allowSetBufferCapacity. It defaults to false. When set to true it will enter into that troublesome branch. This way if anyone wants to maintain that functionality they can do so. So basically, it defaults to the safer option, and your code should not require any changes to your device config.

3. I've removed those calls to ma_device__set_state() and replaced them with a call to ma_device_stop() in ma_job_process__device__aaudio_reroute() with a log message.

4. A very minor and pedantic coding style change for consistency with the rest of the code base.

This could probably do with some additional testing just to be double sure I haven't broken anything with these changes. I'm actually wondering if I should add another device state called ma_device_state_error or similar to indicate that the device is in an invalid and unrecoverable state, as would be the case if rerouting fails.

[znakeeye]

Cool!

Now I remember why I didn't call ma_device_stop in the first place. Stream will be NULL and crash when being closed.

See the assert I added. I guard for NULL when closing the stream. Maybe that guard should instead be inside the function itself?

[mackron]

I added the same guard to ma_device_start/stop_stream__aaudio(), so I guess maybe for consistency it would be better to do the same in ma_close_stream__aaudio().

[znakeeye]

I saw those guards now. Should work as it is.

Will verify that the new code works, later tonight.

[znakeeye]

I verified the latest tweaks in my two apps. No issues 👍
From my end, the PR is ready to be merged.

[mackron]

Thanks for testing. Merged.

[znakeeye]

Cool!

When is the next release planned?

[mackron]

Well I've been saying "soon" for about the last 9 months or so... But I am for real wanting to get this out within the next few weeks. Maybe next weekend, no promises.