ValdemarOrn / CloudSeed

Algorithmic Reverb VST Plugin
MIT License
427 stars 42 forks source link

The reverb sounds like static/white noise #12

Open invcolors opened 3 years ago

invcolors commented 3 years ago

Whenever I use the plugin, the reverb just sounds like white noise no matter how much I mess with it.

https://user-images.githubusercontent.com/83488691/116772530-55799a80-aa15-11eb-9795-0833284e91f0.mp4

Is this kind of sound the expected result?

Thanks!

VincyZed commented 3 years ago

@invcolors Hey there, sorry for the very late answer but might as well answer: You can solve this by enabling "Use fixed size buffers" in the FL plugin wrapper. :)

martinfinke commented 3 years ago

@ValdemarOrn Is there a way to change the code to support variable buffer sizes? Or could you roughly point out which parts of the code rely on a fixed buffer size? Thank you for the reverb, it sounds very nice 👍

Related issue: #1

MIDIculous commented 3 years ago

@VincyZed Same here....A fixed buffer size doesn't always work with different DAWs and in our case causes static sounds. Please point us in the right direction as to where the code is located to update this or update the code to allow multiple buffers.

Much appreciated and thanks so much for your great work on this!

martinfinke commented 3 years ago

@ValdemarOrn The only problem seems to be in this line, where it uses the filterOutputBuffer from the previous Process() call. If I remove the + filterOutputBuffer[i] * feedback, the clicking goes away. Just posting this here, in case it's helpful in fixing the issue.

By the way, looking at that section of code, it seems the reverb would sound different, depending on the buffer size?

basdp commented 2 years ago

I am doing some research on this. It looks like what @martinfinke is saying is not entirely true. Removing that code disables the feedback loop altogether and therefor there is no late reverberation happening at all. So the bug is somewhere else (apart from the assumptions in the DelayLine::Process() function that the sampleCount never changes, I hacked a fix for that but that doesn't make the noise go away), and it seems to be somewhere in the ModulatedDelay. I am doing some more research but help is surely appreciated!

basdp commented 2 years ago

Sorry for spamming, but I've found another clue. It's not in ModulatedDelay. For some reason, the DelayLine has an internal feedback loop, that is 1 sampleCount samples long. This means, like @martinfinke was saying, that depending on the sampleCount, the sound of the reverb is different! The best approach here would be to fix the internal sample delay and make the filterOutputBuffer a circular running buffer. That would 99% sure fix this issue. Preliminary code review of the other classes suggests that the DelayLine is the only place that has an assumption on a constant sampleCount.

basdp commented 2 years ago

Again, sorry for spamming, but I've fixed it. Kind of. I am prepared to make a Pull Request, but there is a fundamental design decision we need to make. I feel we need @ValdemarOrn involved first.

The code, as it is now, has a feedback loop the size of the internal audio driver's buffer. In FL Studio (and others?), this buffer is not fixed by default (it depends on how fast the computer can actually process, so it's less than or equal to the drivers' buffer), which is the reason for this bug.

Now I have fixed this, passing the sample length down from the plugin all the way to the DelayLine, and fixing it to what the DAW tells the plugin the sample length is at maximum and using a circular buffer for filterOutputBuffer in DelayLine.h. Which solves this bug, and makes the plugin behave in FL Studio exactly like it does in other DAWs where the length is not dynamic.

But the question is, is it intentional that the sound of the delay line is coupled to the sample length of the audio driver? If not, should we take a step back and rethink a better strategy? If so, what length should we make the internal buffer? Should we make it configurable? A parameter? In the current code, the internal buffer works as long as it's larger than the audio driver's buffer. This is fixable by buffering in the DelayLine::Process() and looping essentially cutting up the large block of samples into smaller blocks that is at maximum the size of the internal buffer.

martinfinke commented 2 years ago

It looks like what @martinfinke is saying is not entirely true. Removing that code disables the feedback loop altogether and therefor there is no late reverberation happening at all.

Your ring buffer approach sounds good 👍 Which part of what I said was not entirely true? I just said that the clicking goes away, I didn't mean that simply removing that piece of code should be the fix 🙂

GhostNoteAudio commented 2 years ago

Hey guys - Valdemar here. First of all, awesome to see people working on fixing this long-standing issue! When I initially wrote the plugin I did not have any DAWs that used a dynamic buffer size, or at least so I thought (reaper gives you a non-standard blocksize at loop points), and the fact that the buffer size can change only became known to me well after I released version 1. Changing the plugin to work with dynamic block sizes is obviously possible but quite tricky. The fact that the delay line is coupled with the buffer size is entirely accidental and a bug! :)

If you can submit a pull request I'll do everything I can to get it merged and produce a new binary version!

basdp commented 2 years ago

It looks like what @martinfinke is saying is not entirely true. Removing that code disables the feedback loop altogether and therefor there is no late reverberation happening at all.

Which part of what I said was not entirely true? I just said that the clicking goes away, I didn't mean that simply removing that piece of code should be the fix 🙂

You're right, apologies, I was under the impression that the issue was not with that part of the code, but in the end, it was! Also, probably not the right wording because English is not my native language :-)

@GhostNoteAudio, thanks for your comments! I will go ahead and create a PR for this.