Why does getNextAudioBlock hold the lock for the entire function call? Is it because it accesses the ‘ratio’ class member more than once? Assignments of floats are atomic (at least, on Intel, I think).
Regardless, why not just do this
void ResamplingAudioSource::getNextAudioBlock (const AudioSourceChannelInfo& info)
{
{
const ScopedLock sl (ratioLock);
float workingRatio = ratio;
}
// use workingRatio instead of ratio now
I’m not complaining…nor am I using this code. I was just studying it to see how it works - I was already a fairly knowledgable programmer but I have learned a lot from looking at your designs.
So I’m not saying its a “bug” I’m just wondering if I am missing out on something unknown.
I think the only thing you’re missing is how unlikely it is that this would cause anybody a problem.
You are right, and I will tidy it up because it could be done better. But for it to make any noticeable impact on performance, you’d have to be absolutely hammering that setResamplingRatio() call. And since the class wasn’t designed to have its speed continuously varied, that would probably create quite an ugly noise, so isn’t something that people are likely to ever do!
I’ve been wondering about this as well. In another recent post you mentioned that CriticalSections are a sure way to cause mayhem in ProTools.
I know locks in the audio-thread should be avoided at all costs, but there are two or three locations in our plugin where I still use them, as it’s impossible (or at least ridicously complex) to solve it with atomics or other lock-free patterns.
So is a CriticalSection in the PT audio-thread still a critical issue if you know
it will only be held for a short time
the chance it is already locked by another thread is very low
not having a lock is likely to result in a crash or an ugly race-condition
You underestimate what people will do if they can. We are actually using the resampling in the Elastik plugin and allow to control the ratio in real-time. It’s a fun effect, and while there might be small glitches as the class wasn’t designed for that, they are not audible.
The locking could indeed be a problem in PT, so using this class in a plugin could be a bad idea. Annoyingly, although it’d be very easy to change the ratio to an Atomic and avoid locks, I think that’d probably stop it compiling on some platforms that don’t have support for 64-bit atomics.
Let me tell you that you couldn’t be more wrong. Not only does changing the speed continuously work GREAT (like, a new speed every 16 samples), its fast as hell, the sound quality is perfect, and it has almost no latency! This is just what the doctor ordered for simulation of vinyl scratching effects.
I’ve gone through 4 other sets of resampling code (secret rabbit, libresample, others) for doing continuously variable time-stretching of audio material and the juce ResamplingAudioSource is the best by a clear margin!
Excellent! It’s probably good this that situation because it’s not trying to do anything fancy - the others probably use all kinds of clever tricks that sound great at a continuous speed, but don’t handle changes in pitch very smoothly.
It could also be done using a ReferenceCountedObjectPtr to pass it… But probably the easiest lock-free method would just be to convert it to an integer and pass it as an Atomic
Just my opinion but a class like ResamplingAudioSource shouldn’t be responsible for the lock at all. It should be handled by the caller, as I do in my app. The details of locking and data sharing between threads are so application domain-specific that trying to distribute the responsbility to each leaf class is likely to be inapplicable for all but the simplest programs.
Excellent! It’s probably good this that situation because it’s not trying to do anything fancy - the others probably use all kinds of clever tricks that sound great at a continuous speed, but don’t handle changes in pitch very smoothly.[/quote]
You can play with real time changes of the resampling rate of juce::ResamplingAudioSource (I’m using it as a time stretcher) in the DspFilters demo:
Did you do that test with normal resampling too ? (i.e. : playback a 44.1kHz file on a 96 kHz sound card)
When doing this, I got much better results in audacity (libresample) than with ResamplingAudioSource. I got some audible noise with ResamplingAudioSource (a bit like the one induced by the downsampler/bitcrusher effect in Ableton, not as dramatic of course, but still verty audible) , and no real quality loss in audacity but there could be several reasons for that :
that’s my fault (wrong buffer size in prepare to play for example)
ResamplingAudioSource was designed with speed instead of quality in mind
libresample is just better when the frequency is constant
So I wondered if you, Winn, or anyone else, had done the same test and had the same results.
Yes. There is some aliasing when changing the sample rate dynamically, like with a pitch fader (you can hear this in SimpleDJ or DSPFiltersDemo). But if the rate is held constant it sounds pretty good, and uses next to nothing in terms of CPU.
Allright. I must be doing something wrong then. Thanks a lot
I don’t actually use the ResamplingAudioSource directly, but I use the AudioTransportSource, which internally uses a ResamplingAudioSource. Maybe that’s part of the problem ?
EDIT : When you say “pretty good”, do you mean it sound the same as the original, non resampled file, unless you’re realmly really splitting hairs, or do you mean the quality loss is acceptable for you ?
Could someone explain me (as i like to learn a bit more about the different kinds of locking) why you want to use a SpinLock and not a normal Lock (critical section) in this situation.
IMHO, just for performance reasons . From the doc :
The difference between a re-entrant and a non re-entrant lock is that in the former case, the same thread can aquire the lock multiple times, while in the later case, that situation would create a deadlock.
