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

Add Resampling Support #1180

Open
wants to merge 57 commits into
base: dev
Choose a base branch
from
Open

Conversation

JasonLG1979
Copy link
Contributor

@JasonLG1979 JasonLG1979 commented Jun 22, 2023

This PR adds resampling to 48kHz, 88.2kHz, and 96kHz with Windowed Sinc Interpolation.

It also moves everything except decoding out of player and a few other misc improvements like improving thread creation in player, using #[default] in the player config enums where we can, and includes the sample pipeline latency in calculating an accurate position.

@JasonLG1979 JasonLG1979 force-pushed the resampling branch 3 times, most recently from 4b454f6 to 013f63a Compare June 22, 2023 20:33
@JasonLG1979
Copy link
Contributor Author

JasonLG1979 commented Jun 22, 2023

@eladyn or @roderickvd or anyone else really, this could use a review. I know it's a lot of code, I tried my best to split it up into a few commits to make it comprehensible.

It would be so much easier to use elapsed but elapsed could potentially panic is rare cases.

See: https://doc.rust-lang.org/std/time/struct.Instant.html#monotonicity

Otherwise this is pretty straight forward.
If anything fails getting expected_position_ms it will return 0 which will trigger a notify if either stream_position_ms or decoder_position_ms is > 1000.

If all goes well it's simply a matter of calculating the max delta of expected_position_ms and stream_position_ms and expected_position_ms and decoder_position_ms.
So if the decoder or the sample pipeline are off by more than 1 sec we notify.
@JasonLG1979 JasonLG1979 force-pushed the resampling branch 3 times, most recently from f4013f0 to 3411c41 Compare June 23, 2023 08:06
@JasonLG1979
Copy link
Contributor Author

JasonLG1979 commented Jul 4, 2023

Ok @roderickvd I've managed to generate anti-alias filters that are @ -0.5dB at 20kHz and @ -96dB at 22.05kHz using https://github.com/chipmuenk/pyfda

image

The only catch is that there would end up being a 1200 line filter_coefficients.rs that contains all of coefficients as arrays.

It does sound damn good though.

@JasonLG1979
Copy link
Contributor Author

Oh, I'm using a Kaiser Window with a beta of 8.6 so the lobe approximates a Blackman Window.

All Windows calculated with pyfda (Python Filter Design Analysis Tool)
https://github.com/chipmuenk/pyfda
Window = Kaiser
beta = 8.6 (Similar to a Blackman Window)
fc = 22.5kHz
-86dB by 23kHz

This also gets rid of Linear Interpolation which leaves only Low and High both being Windowed Sinc.
@JasonLG1979
Copy link
Contributor Author

I changed the filters a bit. I moved the fc up so that it won't cutoff any of the source.

That will gives us -86dB before 23kHz.

@roderickvd
Copy link
Member

Do you have a graph? I think your previous one is more SOTA. Slight roll-off just around or before 20 kHz is very common in industry as you can see by the transition band figures I posted earlier.

@JasonLG1979
Copy link
Contributor Author

How's this? The fc (-6dB point) is 20.2 - 20.5kHz, depending on the sample rate to get roughly the same curve. It's flat to about 19.8kHz, -0.5 at 20kHz, and hits -87dB well before 22kHz. That's about the best I can get and still maintain a lobe width that's close to a Blackman Window, still get damn close to 20kHz and be at max attenuation well before 22.05kHz.
neg1
neg6
neg100

@JasonLG1979
Copy link
Contributor Author

JasonLG1979 commented Jul 5, 2023

neg1
neg6
neg100
Well scratch that. I've been messing around with different fc's and if you set them below 22.05kHz it's sounds "crunchy". If I had to guess it's probability because the filter is so steep that that it's distorting the very, very high end. I've found the best so far is a fc of 22.5kHz. That still gives you 0.46875 at 48kHz, 0.255102041 at 88.2kHz, and 0.234375 at 96kHz and -86dB well before 23kHz so it should provide acquitted anti-alias filtering for all sampling rates considering most of the time the target is -60dB.

"Crunchy" sounding has often been by complaint for some resamplers. Makes me wonder now if it's from overzealous anti-aliasing?

@JasonLG1979 JasonLG1979 force-pushed the resampling branch 4 times, most recently from 33225cc to 6176710 Compare July 5, 2023 17:55
@roderickvd
Copy link
Member

So is that back to the first one you posted? Like you said then, you thought it sounded great, and objectively I think it looks like the best of them.

@JasonLG1979
Copy link
Contributor Author

So is that back to the first one you posted? Like you said then, you thought it sounded great, and objectively I think it looks like the best of them.

No I have a better one. This one still hits your targets but it hits -135dB at around 23kHz. I also lengthened the window and narrowed the main lobe width to improve interpolation quality. It cost a tiny bit more CPU cycles by I think it's worth it. Sorry this is taking a lot of iterations to get right but it's really trial and error.

neg150
neg6
neg1

@JasonLG1979
Copy link
Contributor Author

I think -135dB is good enough. If I don't stop myself somewhere I'll be tweaking on this forever,lol!!!

@JasonLG1979
Copy link
Contributor Author

JasonLG1979 commented Jul 7, 2023

@roderickvd what I just pushed will get us flat until just over 20kHz and about -195dB before 22kHz. And since -195dB is below the noise floor of even S32 any aliasing artifacts should be buried in the noise floor/quantization noise.
neg220

@roderickvd
Copy link
Member

And it sounds good too? 😄
Let me me know if you are good to merge.

@JasonLG1979
Copy link
Contributor Author

JasonLG1979 commented Jul 13, 2023

I'm still fine tuning. Since I don't have any way to objectively test anything and I can't really can't find any solid advice online specifically for interpolation kernels I'm left to do it by ear. I can find things specific to FIR filters but that advice does not necessary apply. I've been doing my testing in another branch so as not to spam the CI here. I'll let you know when I'm sure it's ready and even then I'd like at least one other person to listen to it to get their impression before it's merged.

@roderickvd
Copy link
Member

Any updates?

@roderickvd
Copy link
Member

bump any updates?

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

Successfully merging this pull request may close these issues.

None yet

3 participants