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

New resampler: audible aliasing when upsampling 11k or 22k audio #2663

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 9 comments
Closed

New resampler: audible aliasing when upsampling 11k or 22k audio #2663

SDLBugzilla opened this issue Feb 11, 2021 · 9 comments
Assignees

Comments

@SDLBugzilla
Copy link
Collaborator

SDLBugzilla commented Feb 11, 2021

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.0
Reported for operating system, platform: Other, x86

Comments on the original bug report:

On 2017-10-12 19:27:12 +0000, Eric Wasylishen wrote:

Created attachment 2981
11025Hz wav with 5kHz sine wave

The new resampler seems to be letting too much aliasing through.
Here is a test case to see the problem:

testresample sine11025.wav sine44100.wav 44100 1
sox sine44100.wav -n spectrogram -o sine44100.png
open sine44100.png

You can hear the aliasing (warning, it's a loud / high pitched sine tone) - sine11025.wav contains a pure sine at 5kHz, but the resampled sine44100.wav will have an additional tone at 6kHz that's almost the same volume as the 5kHz one.

(There are other test cases you can use - e.g. generate 1 second of white noise at 11025Hz in Audacity and upsample it with testresample, or upsample the 22.05kHz test/sample.wav from SDL to 44.1kHz)

On 2017-11-11 18:20:55 +0000, Eric Wasylishen wrote:

Created attachment 3083
Another test case

Here is another test case I'm using, it's a sine sweep from 0 to 5.5kHz, at a 11025Hz sample rate. Cases to test are upsampling to e.g. 44100 or 48000.

If the resampler is performing correctly, the spectrogram of the resampled file should show nothing much above 5.5kHz.

I'm working on a patch for this that lowers the cutoff frequency (currently it is at 0.5*inrate which is too high).

On 2018-02-17 23:10:47 +0000, Ryan C. Gordon wrote:

(In reply to Eric Wasylishen from comment # 1)

I'm working on a patch for this that lowers the cutoff frequency (currently
it is at 0.5*inrate which is too high).

Did you ever make any progress on a patch? I'd like to get it into revision control.

Thanks,
--ryan.

On 2018-02-21 21:03:49 +0000, Eric Wasylishen wrote:

Sorry about neglecting this!
Will try to dust off my patch later today or tomorrow and get back to you.

On 2018-02-21 21:12:25 +0000, Sam Lantinga wrote:

Thanks, we're trying to get a 2.0.8 RC tomorrow and would like to include this.

On 2018-02-22 11:08:10 +0000, Eric Wasylishen wrote:

Created attachment 3178
Resampler w.i.p.

Here's what I have as of tonight..

On the plus side, it fixes this bug,
https://bugzilla.libsdl.org/show_bug.cgi?id=3955
https://bugzilla.libsdl.org/show_bug.cgi?id=3956
and the quality seems good when resampling whole files using "testresample". Testing sine sweeps, upsampling and downsampling them, and looking at the spectrogram with sox.

On the downside, this is more of a rewrite than a small patch.
I broke streaming (clicking artifact when doing:
SDL_AUDIODRIVER=disk SDL_DISKAUDIOFILE=44100.raw FREQ=44100 ./test/loopwave some-96k-input.wav
using the patch to the disk driver from https://bugzilla.libsdl.org/show_bug.cgi?id=3848#c10
), so the patch is not usable as-is. It's also ~2x as slow as what's currently in hg

Aside from fixing streaming, it could use some more documentation / tests / assertions against buffer overflows.
I can have a look on the weekend at fixing the streaming clicking problem and tidying it a bit more.

On 2018-02-22 18:43:20 +0000, Sam Lantinga wrote:

Okay, we'll hold off on this for 2.0.8.

Thanks!

@higaski
Copy link

higaski commented Mar 31, 2022

Any updates on this? I think I just ran into this using release-2.0.20

@slouken slouken added this to the 2.0.22 milestone Mar 31, 2022
@icculus
Copy link
Collaborator

icculus commented Mar 31, 2022

I mean, it's been a few years, it's probably time to rewrite the resampling code from scratch again, right? :)

@icculus
Copy link
Collaborator

icculus commented Mar 31, 2022

This looks like it fizzled out without usable patches. I'd probably encourage you to use the libsamplerate support for now if you're getting bit by this badly, and we can look at this more after this milestone.

@higaski
Copy link

higaski commented Apr 3, 2022

I've managed to statically link libsamplerate into my project and I can see how it gets loaded when stepping through the code during initialization (LoadLibSampleRate). However the problem is still here. Still very audible noise and crackling.

/edit
ok, looks like SDL_sound doesn't use libsamplerate, no matter if its loaded or not?

@icculus
Copy link
Collaborator

icculus commented Apr 4, 2022

Oh, I see what's happening now.

SDL_sound is still using the SDL_AudioCVT API for resampling, and this API does not work to resample data in chunks (it was all we had at the time)...I haven't heard your results, but it's not uncommon for it to produce audio artifacts on the edges of the data that you feed it, because it doesn't know about samples before and after the chunk to interpolate them properly.

Nowadays SDL has SDL_AudioStream, which can resample data as it passes through a queue. SDL_sound will be moving to this soon, but hasn't yet.

My suspicion is this is the exact problem and it's clear what the fix needs to be.

(SDL_AudioCVT doesn't use libsamplerate, just SDL_AudioStream, but your problem will likely go away with or without libsamplerate, once we start using a proper stream. Sorry for the confusion and extra work I caused you here.)

--ryan.

@icculus icculus removed this from the 2.0.22 milestone Apr 4, 2022
@higaski
Copy link

higaski commented Apr 4, 2022

No worries, thank you for clarification. That also explains why the artifacts appear less often when increasing the buffer size. At first I've falsely interpreted that as a performance issue...

Guess I'll wait for a SDL_sound update then. 😄

@icculus
Copy link
Collaborator

icculus commented Apr 9, 2022

Guess I'll wait for a SDL_sound update then.

I just pushed this change to SDL_sound; it works the same way but uses SDL_AudioStream internally. See if it fixes your problem!

@higaski
Copy link

higaski commented Apr 10, 2022

Yes it does. Thank you!

@icculus
Copy link
Collaborator

icculus commented Apr 11, 2022

Hooray! I'm going to close this for now, because I think we're good here. Let me know if this needs to be reopened, though!

@icculus icculus closed this as completed Apr 11, 2022
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

4 participants