getHalSampleRate/getHalFormat/getHalChannelCount returns stale data if MTK Hi-Fi is enabled

[nift4]

trafficstars

Android version(s): Android 14 (not QPR) stock ROM Android device(s): Any MTK device with MTK Hi-Fi enabled (I used Unihertz Jelly Max) Oboe version: Any (reproduces on 1.9.2) App name used for testing: OboeTester

Short description

MediaTek edited the audio framework to add "MTK Hi-Fi" feature which changes USB DAC sample rate and AF mixer thread sample rate based on currently active clients (highest one from active clients will be chosen). However, both AAudio and libaudioclient's AudioTrack rely on that value never changing for reporting data to apps and hence cache the values. They have edited neither of them, which leads to wrong reported values.

Steps to reproduce

1. Plug in USB DAC with display (to verify sample rate changes) and enable "MTK Hi-Fi" in Settings > Audio > Audio enhancements (loose translation from German)
2. Open OboeTester with some ExoPlayer-based music player that does not use offload in split screen (to avoid OboeTester stopping when backgrounded)
3. Prepare 96kHz and 48kHz music files for playback in music player
4. Start playing 48kHz music file, notice USB DAC shows 48kHz sample rate
5. in OboeTester, start testing output in Legacy mode with 48kHz sample rate, play audio and let it run
6. in music player, start playing 96kHz music file (MTK Hi-Fi will switch USB DAC to 96kHz)
7. close and reopen + start stream (with same settings) in OboeTester

Expected behavior

OboeTester HW sample rate display always matches USB DAC display values. After step 5, it would display HW sample rate as 48000. After step 6, it would display HW sample rate as 96000. After step 7, it would display HW sample rate as 96000.

Actual behavior

OboeTester HW sample rate display does NOT always match USB DAC display values. After step 5, it displays HW sample rate as 48000. After step 6, it displays HW sample rate as 48000 due to caching in AAudio and AudioTrack (correct data point 96000 is already present at this point in dumpsys media.audio_flinger). After step 7, it displays HW sample rate as 96000.

Because closing and reopening creates a new AAudio stream and hence new AF track, even though we create 48kHz track, we get new hal sample rate information and hence correct values are displayed after step 7. But it actually already changed after step 6.

Device

ro.product.brand = Unihertz ro.product.manufacturer = Unihertz ro.product.model = Jelly Max ro.product.device = Jelly_Max ro.product.cpu.abi = arm64-v8a ro.build.description = sys_mssi_64_64only_ww_armv82_g78v78c2k_dfl_eea-user 14 UP1A.231005.007 V01.00.00 release-keys ro.hardware = mt6878 ro.hardware.chipname = ro.arch = | grep aaudio = [aaudio.mmap_exclusive_policy]: [2] [aaudio.mmap_policy]: [2]

Any additional context

In AOSP, hal sample rate / hal channel count / hal format is transmitted to AudioTrack.cpp on AF track (re)creation: https://cs.android.com/android/platform/superproject/main/+/main:frameworks/av/media/libaudioclient/AudioTrack.cpp;l=1956;drc=a0fe70939b7b3d5ca5dbba09b6f884017a61387c and it is assumpted that they never change during the lifetime of such an AF track. While it does not apply to the MTK scenario described above (as they invalidated the assumption that hal sample rate / hal channel count / hal format never changes during the lifetime of an AF track anyway - that can only be fixed by them), I believe there is a concrete reliabilty improvement that can be implemented from Google's side: If the AF track is recreated (onNewIAudioTrack callback called) and these values change, AAudio would report stale values because it neither closes the AAudio stream nor expose updates values on track recreation. I propose to close the AAudio stream if hal sample rate / hal channel count / hal format is different after track recreation. Right now, this only checks for the input sample rate / channel count / format: https://cs.android.com/android/platform/superproject/main/+/main:frameworks/av/media/libaaudio/src/legacy/AudioStreamTrack.cpp;l=353;drc=a0fe70939b7b3d5ca5dbba09b6f884017a61387c If this check was extended to compare hal sample rate / hal channel count / hal format, it would probably save some future vendors from this bug.

However, even this check does not always run because the callback is only registered for offload (where this concern is irrelevant) or callback transfer mode: https://cs.android.com/android/platform/superproject/main/+/main:frameworks/av/media/libaaudio/src/legacy/AudioStreamTrack.cpp;l=123;drc=a0fe70939b7b3d5ca5dbba09b6f884017a61387c This already leads to inconsistent AAudio stream recreation (because onNewIAudioTrack is only called in callback transfer mode, but not in sync transfer mode) and would cause issues with my proposed fix as well, so I additionally propose to always register the AudioStreamTrack as IAudioTrackCallback callback to be able to always handle onNewIAudioTrack.

[flamme]

@nift4, thanks for the report and great analysis!

I think it is true that vendor may call setParameters to change the lower layer configuration. @robertwu1 , please check if onNewIAudioTrack is called correctly when this happens. The hardware information should be updated if the stream is not disconnected.

However, even this check does not always run because the callback is only registered for offload (where this concern is irrelevant) or callback transfer mode:

That sounds a reasonable concerns to me. It should also register callback and only callback to client when client is using callback mode.

[robertwu1]

Tracked with internal bug b/417249179