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

Should we use an external library for the resampler and/or mixer? #308

Open
zfigura opened this issue Mar 16, 2023 · 8 comments
Open

Should we use an external library for the resampler and/or mixer? #308

zfigura opened this issue Mar 16, 2023 · 8 comments

Comments

@zfigura
Copy link

zfigura commented Mar 16, 2023

While trying to fix an issue related to unaligned buffers, I ran into some complications related to the use of EXTRA_DECODE_PADDING. While I believe it should be possible to modify our resampler to avoid the need for that, it's not exactly trivial, and it seems like less effort to simply outsource that effort by making use of existing code, leading me to wonder if this is something that has been considered before.

Obviously any new external library is a burden for distribution. On the other hand—not to put too fine a point on it—the current code seems complex and brittle, and this has deteriorated over time (a770349, a756af4, eb94a81). The EXTRA_DECODE_PADDING also seems broken in a couple of ways, mostly by virtue of not handling end-of-buffer conditions.

Besides that, an external library would potentially offer better-quality resampling options (i.e. anything more sophisticated than the current simple linear interpolation), as well as SIMD support for the mixer, and more updated SIMD support for the resampler (e.g. GStreamer has a path for SSE 4.1). And of course there are the usual benefits of relying on an actively developed library—increased support for new features, more attention to bug fixes.

In summary, it seems well worth it to replace our resampler and mixer with an external library.

As for which external library to use—options that occur to me immediately are GStreamer and ffmpeg (aka libav). I haven't researched others.

GStreamer has a very simple synchronous API for resampling (libgstaudio) and a less simple callback-based API for mixing (i.e. it'd essentially require building a pipeline, though this isn't too hard, and I believe it can actually be done without callbacks if we care).

ffmpeg seems to be in about the same boat, although I'm not very familiar with its API. One downside is that it does not seem to commit to a stable or backwards-compatible API.

Is this something that would be acceptable? If so I'll start trying to implement this. If not I'd appreciate a link to discussion on the topic, if any.

@flibitijibibo
Copy link
Member

Those end up being pretty heavy-duty in terms of dependencies - FAudio is in the same boat as SDL where we really need everything to be either self-contained or something that's zlib-licensed (or stb-style public domain), since many targets can't bring something like ffmpeg into the mix. But, being in the same boat, I've kept a close eye on how SDL has dealt with resampling in particular, and the result is that we have 2 options:

  1. Add a path for libsamplerate, can be dynamically loaded: https://github.com/libsdl-org/SDL/blob/main/src/audio/SDL_audiocvt.c#L497
  2. Make resampling an FAudio_platform_* function, and add pitch shift support to SDL_AudioStream: https://github.com/libsdl-org/SDL/blob/main/include/SDL3/SDL_audio.h#L685

Way back in the day I attempted to use option 2 and the result was pretty limited; we have to be able to adjust the sample rate pretty much all the time, so most resampling libraries don't cut it in terms of all the fussy things XAudio needs. I did my best to take notes from Chris Robinson and OpenAL Soft (without actually using the code, for licensing reasons), but yeah, it's definitely got some sharp edges.

Mixing is a bit tougher because of the way XAudio2 processes channels; mixing isn't hard but writing handwritten SIMD can be sometimes... unlike resampling I actually don't think there's anything that does exactly what XAudio does, but at the same time it's much easier to get correct than resampling.

@zfigura
Copy link
Author

zfigura commented Mar 17, 2023

Assuming I understand correctly, in both cases we'll need to keep the current code path around, which means I'll need to fix the current resampling code anyway. Having to do that and spend effort on improving a less featureful, non-copyleft project does not sound particularly appealing.

I haven't touched the mixing code much but it does indeed seem like the resampler is in much worse shape than the mixer (and would also benefit more from being outsourced).

What prevents FAudio from linking to an LGPL library?

@flibitijibibo
Copy link
Member

FAudio targets consoles and mobile, both of which disallow LGPL binaries :/

I do agree that improving something like SDL's resampling would have a much bigger impact than trying to fix up this one, so if we can get a third party library to do this well it would make the possibility of nuking the old path a whole lot easier (and whatever library that comes in to fix it can presumably be reused for the Wine path as well, even if it has to statically link some SDL files manually or something).

@flibitijibibo
Copy link
Member

Remembered that we have an issue filed for this in SDL3:

libsdl-org/SDL#7378

@flibitijibibo
Copy link
Member

Looks like Ryan might be doing this for us, could be worth a review when he submits it?

icculus/SDL@f32a3bd

@flibitijibibo
Copy link
Member

Talked to Ryan last night and confirmed this should do what I had hoped we could do a few years ago - so, if you'd like, you can replace the resampler with whatever you think will work best for the win32 target, and as long as SDL3 can implement the same layer we should be good to go.

@flibitijibibo
Copy link
Member

libsdl-org/SDL#7571

@flibitijibibo
Copy link
Member

The new SDL audio converter is now in the SDL3 branch - you can find the test program here, demonstrating the pitch shifting capabilities in particular: https://github.com/libsdl-org/SDL/blob/main/test/testaudiostreamdynamicresample.c

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

No branches or pull requests

2 participants