Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ao/aaudio: implement aaudio backend for android #12261

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jambonmcyeah
Copy link

No description provided.

@jambonmcyeah jambonmcyeah force-pushed the master branch 3 times, most recently from a6c9f63 to 74f7f0c Compare August 27, 2023 09:02
@sfan5
Copy link
Member

sfan5 commented Aug 27, 2023

Are the advantages over ao_audiotrack significant enough that it warrants merging this and having yet another AO to maintain?

@jambonmcyeah
Copy link
Author

jambonmcyeah commented Aug 27, 2023

well, using JNI is infamously slow and is more resource intensive.
Also this is way cleaner compared to the JNI code and should be easier to maintain.

@jambonmcyeah
Copy link
Author

@sfan5 Another reason is the audiotrack doesn't support exclusive mode audio, which bypasses android's resampling improving audio quality.

@jambonmcyeah
Copy link
Author

opensles does support exlusive mode audio, but only supports 2 channes, while AAudio supports multichannels.

@jambonmcyeah
Copy link
Author

switching to push seems to fix the underrun issues, this is ready for review

@romain8991
Copy link

Hello @jambonmcyeah, I was just wondering, is the aaudio output set on push exclusive mode by default in mpv? Or should I enable it somehow? Thanks in advance!

@jambonmcyeah
Copy link
Author

jambonmcyeah commented Dec 23, 2023

well right now this PR not merged by the MPV developers yet. So if you want to use it you have to compile it yourself

@jambonmcyeah
Copy link
Author

and it's now push only, since there were a lot of underruns with pull which I don't know how to fix

@romain8991
Copy link

romain8991 commented Dec 23, 2023

and it's now push only, since there were a lot of underruns with pull which I don't know how to fix

Thanks for the response and involvement. My bad, I thought it was already implemented, that's weird, since I don't know what audio driver output my Android device is using right now, especially since I can't get any logs. Anyway, I'll wait for it to be included then, considering my knowledge in programming is rather limited, though.

@romain8991
Copy link

romain8991 commented Feb 14, 2024

well right now this PR not merged by the MPV developers yet. So if you want to use it you have to compile it yourself

Hello again,

I actually ended up trying to compile it myself, using Android Studio, but I don't know where to add the ao_aaudio.c file inside the source code of the Android port of mpv. Could you please help me somehow/give some informations? Thanks in advance, that would be greatly appreciated: https://github.com/mpv-android/mpv-android/tree/master

Copy link
Contributor

@kasper93 kasper93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the aaudio point of view code looks ok. But please fix all style issues. It doesn't adhere to mpv coding style. You can read https://github.com/mpv-player/mpv/blob/master/DOCS/contribute.md and look at other files in the repository or ask questions if something is unclear. For starters we use 4 spaces, not 2 and many more tiny issues. I'm doing archeology here, but it popped out after recent bump.

audio/out/ao_aaudio.c Show resolved Hide resolved
audio/out/ao_aaudio.c Outdated Show resolved Hide resolved
audio/out/ao_aaudio.c Outdated Show resolved Hide resolved
audio/out/ao_aaudio.c Outdated Show resolved Hide resolved
audio/out/ao_aaudio.c Outdated Show resolved Hide resolved
audio/out/ao_aaudio.c Outdated Show resolved Hide resolved
audio/out/ao_aaudio.c Outdated Show resolved Hide resolved
audio/out/ao_aaudio.c Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
audio/out/ao_aaudio.c Outdated Show resolved Hide resolved
@gamernoob24
Copy link

I tried Building it from the mpv-android buildscripts but it had missing libraries
here is my partial log https://0x0.st/H536.txt

@sfan5
Copy link
Member

sfan5 commented Feb 24, 2024

mpv-android builds with API 21, AAudio was introduced with API 26.
I don't have plans to raise the API currently, especially not for a new audio driver when audio already works perfectly fine.

If you want to test you can edit the API level in buildscripts/buildall.sh and app/build.gradle.

@kasper93
Copy link
Contributor

So addition of this AO should be behind meson option and auto-detected.

@zhengqwe
Copy link

mpv-android builds with API 21, AAudio was introduced with API 26.
I don't have plans to raise the API currently, especially not for a new audio driver when audio already works perfectly fine.

Consider adding an option to toggle, and this option only appears on API26+ devices

@sfan5
Copy link
Member

sfan5 commented Feb 25, 2024

Consider adding an option to toggle, and this option only appears on API26+ devices

UI isn't the main problem.
If I build the native code with api level 26 it will not work on 25 and lower. A possible workaround on the mpv side is to load methods dynamically:

static bool load_lib_functions(struct priv_owner *p, struct mp_log *log)

@jambonmcyeah
Copy link
Author

jambonmcyeah commented Dec 12, 2024

implemented the requested fixes, and used the method described in https://developer.android.com/ndk/guides/using-newer-apis to load the symbols dynamically.

@jambonmcyeah jambonmcyeah force-pushed the master branch 4 times, most recently from d6cbe04 to d187e66 Compare December 12, 2024 14:55
@jambonmcyeah jambonmcyeah requested a review from kasper93 February 9, 2025 04:34
@zhengqwe
Copy link

zhengqwe commented Feb 9, 2025

Also need to bump the API version to 26 for mpv-android for this to work

No need to bump the API version to 26 for mpv-android, this results in all devices with API lower than 26 no longer being able to install mpv-android. Only need to add a detection mechanism. If the API does not meet the conditions, the relevant options will not be displayed or prompts unavailable.

meson.build Outdated Show resolved Hide resolved
audio/out/ao_aaudio.c Outdated Show resolved Hide resolved
@sfan5
Copy link
Member

sfan5 commented Feb 9, 2025

So the conditional thing actually seems pointless, because meson just doesn't find aaudio with an api21 toolchain.
From meson-logs.txt:

Running compile:
Working directory:  .../mpv/_build-arm64/meson-private/tmpegknbsp7
Code:
int main(void) { return 0; }

-----------
Command line: `aarch64-linux-android21-clang .../mpv/_build-arm64/meson-private/tmpegknbsp7/testfile.c -o .../mpv/_build-arm64/meson-private/tmpegknbsp7/output.exe -D_FILE_OFFSET_BITS=64 -O0 -Werror=implicit-function-declaration -laaudio -Wl,--allow-shlib-undefined -Wl,-O1,--icf=safe -Wl,-z,max-page-size=16384` -> 1
stderr:
ld.lld: error: unable to find library -laaudio
clang: error: linker command failed with exit code 1 (use -v to see invocation)
-----------
Library aaudio found: NO
Running compile:
Working directory:  .../mpv/_build-arm64/meson-private/tmpmenk502w
Code:
int main(void) { return 0; }

-----------
Command line: `aarch64-linux-android21-clang .../mpv/_build-arm64/meson-private/tmpmenk502w/testfile.c -o .../mpv/_build-arm64/meson-private/tmpmenk502w/output.exe -D_FILE_OFFSET_BITS=64 -O0 -Werror=implicit-function-declaration -lOpenSLES -Wl,--allow-shlib-undefined -Wl,-O1,--icf=safe -Wl,-z,max-page-size=16384` -> 0
Library OpenSLES found: YES

Also need to bump the API version to 26 for mpv-android for this to work

Then why use the __builtin_available and the weak symbol stuff?

@jambonmcyeah jambonmcyeah force-pushed the master branch 4 times, most recently from 429fceb to 498f9b2 Compare February 9, 2025 16:28
@jambonmcyeah
Copy link
Author

@sfan5 yeah, I searched up some more. The weak symbol thing doesn't work if the library doesn't exist, so I'm changing it to just dlopening the library just like what oboe does

@jambonmcyeah
Copy link
Author

Now it builds fine using API level 21

@sfan5 sfan5 self-requested a review February 9, 2025 22:01
@jambonmcyeah jambonmcyeah force-pushed the master branch 3 times, most recently from a5d1705 to a357fdb Compare February 10, 2025 04:13
@jambonmcyeah
Copy link
Author

jambonmcyeah commented Feb 10, 2025

Did some minor cleanups, should be ready for review

@obscenelyvague
Copy link

Wearing earphones I hear some crackling/popping occasionally, is that just me? They might be subtle enough to not be audible when using speakers.

@zhengqwe
Copy link

Wearing earphones I hear some crackling/popping occasionally, is that just me? They might be subtle enough to not be audible when using speakers.

When crackling/popping occurs, does mpv have a prompt similar to device underrun?
Will the reason for this issue be similar, although the API is different? Need an option like --aaudio-exclusive-buffer?

@obscenelyvague
Copy link

Without specifying a buffer capacity:

[  97.783][v][ao/aaudio] device buffer: 1900 samples.
[  97.783][v][ao/aaudio] using soft-buffer of 9600 samples.

The default buffer capacity or specifically setting the one apparently reported by device in config both lead to crackling.
Setting a higher capacity like 96000 make the crackling much more rarer but occasionally leads to A/V desync detectable by mpv.

[ 576.570][w][cplayer] 
[ 576.570][w][cplayer] Audio/Video desynchronisation detected! Possible reasons include too slow
[ 576.570][w][cplayer] hardware, temporary CPU spikes, broken drivers, and broken files. Audio
[ 576.570][w][cplayer] position will not match to the video (see A-V status field).
[ 576.570][w][cplayer] Consider trying `--profile=fast` and/or `--hwdec=auto-safe` as they may help.
[ 576.570][w][cplayer] 

Need an option like --aaudio-exclusive-buffer?

Acquiring exclusive device access from Android app is probably not trivial. I'm not using that mode.
audio-exclusive doesn't seem to do anything at the moment.

@jambonmcyeah
Copy link
Author

jambonmcyeah commented Feb 11, 2025

@obscenelyvague I reproduced on my own device as well. Should be fixed now. Also fixed a bug where the player would get stuck on device disconnection.
Thanks a lot for testing.

@obscenelyvague
Copy link

Seems ok for the most part. What's a little strange is that I can't seem to set buffer capacity to a value lower than 9600 anymore. Higher values tend to make player startup/exit a little jankier.
On a slightly unrelated note, would it be possible to have reported device buffer as a property? I found that speaker and DAC report slightly different values. Would be cool to be able differentiate between devices like this. Maybe there's some other property that changes that I don't know of.

@jambonmcyeah
Copy link
Author

jambonmcyeah commented Feb 11, 2025

@obscenelyvague I'm tested setting buffer-capacity to 1 and even that works lol.
Also if you want to see the actual buffer capacity aaudio gave you, you can add MP_INFO(ao, "buffer-capacity = %d\n", ao->device_buffer) in static void start(struct ao *ao) and you can see it in logcat. Don't really know how to implement a property.
Finally audio-exclusive does work from my testing.

@jambonmcyeah
Copy link
Author

Additionally, Android won't give you a buffer capacity that's too low, the lowest I can get is 5772 even with aaudio-performance-mode=low-latency

@jambonmcyeah
Copy link
Author

@obscenelyvague Figured out the reason for the jank with large buffer capacities, should be fixed now as well.

@jambonmcyeah
Copy link
Author

jambonmcyeah commented Feb 11, 2025

Alright seems there's a bug with AAudio where it doesn't returning timing or returning garbage timing after a flush.
Seems like VLC has encountered the same issue as well.
I guess the solution is just to close the stream and reopen it every reset.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants