-
Notifications
You must be signed in to change notification settings - Fork 501
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
Added AAudio support for Android #484
Conversation
This can be selected by compiling with SAUDIO_AAUDIO defined. Requires Android API >= 26.
Thanks! Do you think it make sense to drop the OpenSLES backend in favour of AAudio? (e.g. is AAudio supported by the majority of Android devices, or does it make sense to keep OpenSLES as fallback option?) I need to update the Android SDK/NDK platform used by fips anyway, when running Android samples on my Galaxy A40 I'm getting warnings about an outdated platform). Can platform version 26 be considered a viable "base platform" or is this still too early? |
I would say keep them both for now but perhaps if left undefined you could have it automatically select AAudio if available? API 26 is Android 8, and there are probably a fair number of devices still on Android 7 or below. Here are some real time stats: https://gs.statcounter.com/os-version-market-share/android/mobile-tablet/worldwide EDIT: according to the above 86.59% of devices in January 2021 would run Android 8 or above. EDIT2: by making it select AAudio if available you would ensure it get good test coverage from the people using your library, as otherwise maybe nobody would bother enabling it. |
sokol_audio.h
Outdated
AAudioStreamBuilder_setChannelCount(_saudio.backend.builder, _saudio.num_channels); | ||
AAudioStreamBuilder_setBufferCapacityInFrames(_saudio.backend.builder, _saudio.buffer_frames * 2); | ||
AAudioStreamBuilder_setDataCallback(_saudio.backend.builder, _saudio_aaudio_data_callback, NULL); | ||
AAudioStreamBuilder_setFramesPerDataCallback(_saudio.backend.builder, _saudio.buffer_frames); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor indentation typo here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...looks like it's a tab character. Do you have an editor that honors the .editorconfig file?
(see: https://github.com/floooh/sokol/blob/master/.editorconfig)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep I am editing this in Xcode and I use tabs so I manually change them before submitting a PR.
I'll try and figure out a better workflow for next time.
FYI, I'll first try to get Android CI builds working again, then I'll try to bump the default platform version in fips to something more recent, then I'll look into the PR. So it might be a few days. I also have some sokol_audio.h changes queued up in the back of my head to address this topic: ...I'll probably batch all sokol_audio.h related stuff. Just as a heads up :) |
Yeah that's fine For reference, I used these resources for the implementation: https://developer.android.com/ndk/guides/audio/aaudio/aaudio |
So just on this, be aware there is a problem with AAudio on Android 8.0, apparently fixed in 8.1 so if you end up making this automatically selected then best aim for API 27, not 26. This would mirror the behaviour of Google's "Oboe" library which selects OpenSLES for API 26 or below, and AAudio for API 27 or above. So maybe you could do the same, but of course have it manually overridable too! https://github.com/google/oboe/blob/master/docs/AndroidAudioHistory.md |
0569b2d
to
39bad15
Compare
Wow, time flies :/ I'll try to look into this soon-ish. I'll take care of a couple other smaller 'drive-by' PRs first, but then I'll try to test and merge this PR. It's amazing how little code is needed with AAudio vs OpenSLES, would be nice if we could get rid of OpenSLES alltogether, but I guess the responsible thing to do is to implement a fallback (I'll take care of this when merging). |
Nice one. I actually migrated away from sokol_audio because I needed hardware decoding/offlloading in addition to raw PCM playback, but the PR was working perfectly fine when I stopped using it and perhaps someone will find it useful in future! |
As for getting rid of OpenSLES....what Android API level do you support for your other libraries, does it vary, or have you got some kind of consistent baseline? Seems like API 26 and below is an ever-decreasing % of Android devices... |
Out of interest, what library are you using now, miniaudio maybe? Reason I'm asking is because I'd like to keep the sokol_audio.h feature set small, but want to recommend people something useful if they 'outgrow' the sokol-audio features :) PS: I think I've switched everything to android-28 (see here: https://github.com/floooh/fips/blob/a12c2db77270087e7a7d8ebd9a85689a78989724/mod/android.py#L120) |
No I ended up just writing my own code directly using the APIs for the various platforms. For Apple platforms CoreAudio also allows passing packets to be decoded by configuring things slightly differently. Android requires some JNI interface code to get access to AudioTrack, which is quite similar to CoreAudio, and Windows I haven't properly figured out yet so I'm using decoding in software and accessing WASAPI much like sokol_audio does. Since my project is AV-related I wanted to be able to do things like pass EAC3 stream data so that it can be directly decoded by the TV. I was actually using miniaudio before I stumbled across your libraries, and since at that point I was only dealing with software decoding and PCM playback I found sokol_audio a more compact/simpler fit. I think the beauty of your libraries is they don't do too much! I am using sokol_gfx and it's perfect. PS: If you're on android-28 then it looks like you can just ditch OpenSLES completely. In case someone is relying on it though, perhaps you could leave it in for a period of time as an option but deprecate it? |
Starting to tinker with this now. I'll probably kick out OpenSLES support alltogether while I'm at it... |
FYI, I'm closing the PR instead of merging. I had started a cleanup branch already and it was a bit of a hassle to cleanly merge the PR into it, so I copied your code over and applied the structural 'cleanup changes'. I'll make sure to mention you in the changelog though (hope that's ok). |
Perfect - nice one! |
This can be selected by compiling with SOKOL_AAUDIO defined.
Requires Android API >= 26.