Clownacy / clownaudio

Sound engine that is suitable for video games and other multimedia applications.
zlib License
24 stars 2 forks source link

Add oswrapper_audio decoder backend #10

Closed NeRdTheNed closed 5 months ago

NeRdTheNed commented 1 year ago

This PR adds a decoder backend which uses oswrapper_audio, from https://github.com/NeRdTheNed/OSWrapper. oswrapper_audio is a BSD0 licensed single-header file library which acts as a wrapper for OS provided audio decoding functionality on supported platforms. Currently, it only has an implementation for macOS, but I plan to add a Windows implementation once I figure out how to use the Microsoft Media Foundation APIs. I've left it off by default, as most people don't use macOS (it compiles, but it can't decode audio without a platform implementation). The macOS implementation can use system libraries to decode many formats, including MP3, M4A, WAV, AIFF, and FLAC on recent versions of macOS. A full list of supported file types can be found here. This can drastically reduce compiled binary size, as decoders for each of these formats don't have to be included in the binary.

I was originally working on a macOS specific AudioToolbox decoder backend, but I decided to spin it off into a platform-independent library, because I figured that adding a macOS specific decoder backend would be impossible to maintain for everyone who doesn't use macOS. As such, the API for oswrapper_audio is very similar to a clownaudio decoder.

ewancg commented 1 year ago

If this idea was better supported on all platforms, I'd be an advocate. Considering the current limited state of WMF though, and the fact that there is no obvious solution for non-Windows or macOS platforms, libavcodec & libavformat would likely be required at some point which is a licensing nightmare and a lot of extra overhead for something with questionable benefit (what can't you decode that you want to already)?

NeRdTheNed commented 1 year ago

If this idea was better supported on all platforms, I'd be an advocate. Considering the current limited state of WMF though, and the fact that there is no obvious solution for non-Windows or macOS platforms, libavcodec & libavformat would likely be required at some point which is a licensing nightmare and a lot of extra overhead for something with questionable benefit (what can't you decode that you want to already)?

The idea of the oswrapper_audio library (and the other OSWrapper libraries) is that you can re-use a OS decoder / feature on supported platforms, and not have to ship other libraries, which reduces code size and prevents licensing issues (such as with libavcodec & libavformat). All decoders that oswrapper_audio will use are built-in to the operating system, so they don't have any issues with licensing, but the formats they can decode are platform-dependent. For example, this means you don't have to ship an M4A decoder on macOS, as it has an inbuilt M4A decoder, which might avoid issues with licensing. I've just started writing the libraries, so if there is an issue or problematic feature, feel free to open an issue at https://github.com/NeRdTheNed/OSWrapper.

NeRdTheNed commented 1 year ago

Changing this to a draft for now, until I finish the Windows implementation for oswrapper_audio (I think I'll have to make some breaking API changes).

NeRdTheNed commented 1 year ago

I've now written the Windows implementation of oswrapper_audio, using the Media Foundation API. Supported formats include MP3, M4A, WAV, and WMA. For a more complete list of supported formats, see here. Additionally, any installed codecs are able to be used (for example, I'm able to play FLAC files).

NeRdTheNed commented 1 year ago

Hi @Clownacy, I'd be happy to close this PR if you feel that it's too redundant / you don't want to maintain this decoder backend / it's too hard to test cross platform / you want to re-write some code before any new decoder backends are added ect. I'd also be happy to re-work any problematic features and code, or split this into platform specific decoder backends. If the git submodule is a problem, I can remove it and just include the appropriate header file.

Clownacy commented 1 year ago

Oh no, I like this PR! I'm just trying to figure what to do about CoInitializeEx, as having it in clownaudio itself seems like a hack. I'm not very familiar with COM, so it's not clear to me where the ideal place to call it is. I've read that libraries shouldn't call it, and that it should be left to the main application, but that seems to defeat the point of encapsulation and would impact portability (platform-independent programs have to include platform-specific code just for an optional dependency of a dependency).

Cubeb forces the CoInitializeEx call onto the library's user, while miniaudio handles it automatically but allows the user to specify which value to initialise it with. What bothers me is what should be done if the user already called CoInitializeEx on their own, and needs it to be a certain value, which isn't the one that your library wants it setting to.

NeRdTheNed commented 1 year ago

Oh no, I like this PR! I'm just trying to figure what to do about CoInitializeEx, as having it in clownaudio itself seems like a hack. I'm not very familiar with COM, so it's not clear to me where the ideal place to call it is. I've read that libraries shouldn't call it, and that it should be left to the main application, but that seems to defeat the point of encapsulation and would impact portability (platform-independent programs have to include platform-specific code just for an optional dependency of a dependency).

Cubeb forced the CoInitializeEx call onto the library's user, while miniaudio handles it automatically but allows the user to specify which value to initialise it with. What bothers me is what should be done if the user already called CoInitializeEx on their own, and needs it to be a certain value, which isn't the one that your library wants it setting to.

I completely understand, I dislike that I ask users of the library to call it. I don't think I should always include calling it in the library, as calling CoInitialize / CoInitializeEx can apparently result in strange failures depending on the concurrency model used (OSWrapper should be fine with any concurrency model). I could add a define for calling it (along with the value for the call) in oswrapper_audio_init if that would help.

NeRdTheNed commented 1 year ago

@Clownacy I've added a define to oswrapper_audio that makes it responsible for managing calls to CoInitializeEx, which I'm now using in this PR.