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

Rubberband R3 time stretching algorithm adds a fade in/eats the transient on playback start #11093

Closed
robbert-vdh opened this issue Nov 25, 2022 · 5 comments · Fixed by #11120

Comments

@robbert-vdh
Copy link
Contributor

Bug Description

I just noticed the new time stretching option on the main branch added in #4853. It sounds much better than the existing two options, but I'm finding it not suitable for most applications as it seems to kill the initial transient on playback for me by adding a quite lengthy fade in. I searched through the issue tracker but I couldn't find anything referencing this. Here's a demonstration using all three time stretching algorithms:

2022-11-25_14-27-21-reencode.mp4

Out of the three the v2 Rubber Band one sounds the closest to what I'd expect. The SoundTouch one is also fine, but the Rubber Band v3 kills the transient completely.

Version

The branch from #11090, or 8affbbb on the main branch

OS

Arch Linux

@MarcPlace
Copy link

MarcPlace commented Nov 25, 2022

Can confirm this behaviour here with Rubberband R3 when using time stretching, that it cuts off (or eats) the first beat on playback start. If you listen closely, even R2 cuts it off a bit. But the impact is very small and it's still OK to work with (compared to R3).
I'm using commit 9965d55 (main branch) and Gentoo Linux.

@daschuer
Copy link
Member

A workaround would be to "settle" Rubberband with the samples before the cue point to move the ramp up out of the hear-able range. We have such a code for SoundTouch and a workaround in Mixxx:

m_pSoundTouch->putSamples(buffer_back.data(), kSeekOffsetFrames);

@robbert-vdh
Copy link
Contributor Author

I just started looking into this when I thought to check what other rubberband time stretching implementations do. I created a quick .wav file containing a train of DC pulses to test with (the sample, if anyone needs it). I noticed in Mixxx this starting playback at the edge of the DC block adds a relatively lengthy fade in from what looks like a linear phase filter or (more likely) a window function used in an STFT process. I then found out that I already had a rubberband-r3 CLI tool installed. Running that with rubberband-r3 --fine --realtime --tempo 1 dc-train.wav dc-train-resampled.wav should behave the same as in Mixxx, but the rubberband-r3 version doesn't have the long fade in. I then dug op the source code of that command line tool and spotted this:

https://github.com/breakfastquay/rubberband/blob/8edf1723c4f8a8f3b558607b2b1055c23aed48cb/main/main.cpp#L776-L800

As I already expected at this point, in realtime mode you need to pad the input with a certain number of samples and subsequently discard the outputs from processing those samples after resetting rubberband. With these settings that's 2048 samples, which corresponds nicely with the fade in I'm seeing. The docs and faq also mention that you need to do this. I'll implement this tomorrow, here's hoping it's actually as simple as it seems ha.

@uklotzde
Copy link
Contributor

uklotzde commented Dec 7, 2022

As I already expected at this point, in realtime mode you need to pad the input with a certain number of samples and subsequently discard the outputs from processing those samples after resetting rubberband.

A comment in the SoundTouch code confirms this wrong assumption:

// (Rubberband does not suffer this issue)

@robbert-vdh
Copy link
Contributor Author

robbert-vdh commented Dec 7, 2022

It was indeed as simple as I hoped it would be. The initial transient plays back just fine now, even with this pulse train sample. Did some more minor clean up while I was at it. I'll submit a PR soon:

(warning: contains a short DC burst near the end, only an issue if you have monitors connected to DC-coupled outputs)

simplescreenrecorder-2022-12-07_22.28.47.mp4

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

Successfully merging a pull request may close this issue.

5 participants