LancePutnam / Gamma

Generic (Sound) Synthesis Library
Other
458 stars 54 forks source link

ReverbMS documentation and processing order mismatch #39

Open mhetrick opened 8 years ago

mhetrick commented 8 years ago

The description for ReverbMS states: "The network consists of a group of /// parallel comb filters followed by a series of allpass comb filters. /// The comb filters produce initial echoes and the allpass combs diffuse the /// echoes by smearing transients and further increasing echo density."

However, in the implementation (ReverbMS::operator()), it looks like the allpass filters are applied before the comb filters.

Am I reading that correctly?

For the JOS implementation diagrams, Freeverb (https://ccrma.stanford.edu/~jos/pasp/Freeverb.html) and SATREV (https://ccrma.stanford.edu/~jos/pasp/Example_Schroeder_Reverberators.html) are shown with allpass filtering after the comb filters. Only the two JCVERB examples follow the Gamma method of allpass before combs.

It might be worth adding a boolean switch to decide which stage happens first. That way, more Schroeder types can be implemented.

mhetrick commented 8 years ago

Two other notes (should I open these as separate issues?): 1) Spatial.h requires C++ 11 to compile, due to the use of initializer_list. I know that you are starting a C++ 11 branch, but it's worth noting to users Spatial.h requires C++ 11 before the rest of Gamma.

2) When using the ReverbFlavor constructors (with lengths in samples for AP and Combs instead of time)... Is there some way to specify the designed sample rate? For instance, the JCREVERB flavor in the code seems to be based directly off of this implementation (https://ccrma.stanford.edu/~jos/pasp/Example_Schroeder_Reverberators.html), but that implementation is designed for 25 kHz sampling rates. Perhaps I'm reading the code incorrectly, but I can't find a place where those sample lengths are scaled for the current sampling rate.

LancePutnam commented 8 years ago

The processing order won't affect the result as long as you don't modulate the delay times. That said, I'm not sure if one ordering would sound better if you did add modulation. I put the combs last so they could be tapped to generate more channels, so that's at least one advantage of the current ordering.

LancePutnam commented 8 years ago

Regarding the other notes: 1) Yes, it does require C++11. I suppose a note could be added in the header somewhere. 2) There is no way to specify sample rate. Lengths are generally in samples since they need to be coprime. The sample lengths could be scaled, but then you would also have to correct them afterwards to ensure they are coprime.