jamoma / jamoma2

A header-only C++ library for building dynamic and reflexive systems with an emphasis on audio and media.
MIT License
30 stars 6 forks source link

Delay Object Implementation #28

Closed tap closed 8 years ago

tap commented 9 years ago

Creating this to track work that @nwolek has already started.

A couple of thoughts...

  1. We should do this work on a branch until it is fully baked (I should also do the same with the Limiter work -- my bad)
  2. For the design we don't want branching in the perform loop (e.g. for different interpolation types) because it's slow. But we don't want multiple Delay objects for Linear, Cosine, Cubic, etc. delays because that is not dry.

The approach I'm envisioning is that the Delay is a template class -- similar to what we did with the CircularStorage class, except the type that specializes it is an interpolation class (also implemented as a template). This would allows to create a single Delay class and a set of general Interpolation classes and have it all compile down to some very efficient code.

It might be a bit more complicated, and I can help. If the template stuff is too much to think about right now then first implementing as regular C++ classes on the branch as a prototype would still be a good start.

tap commented 9 years ago

Moved to the nw/delay branch in git

tap commented 9 years ago

Relevant food for thought coming from the music-dsp list this weekend:

Is it possible to use a filter to compensate for high frequency signal loss due to interpolation? 
For example linear or hermite interpolation.

And an answer:

Look at Vesa Välimäki's work, and his students'. 
They did fractional delay delay lines, which had just this problem in the high end. 
Also, Julius O. Smith's work with waveguides bumped into this very same thing, 
because they're implemented as (fractional) delay lines as well. 
[snip]

The usual thing you do is to go for higher order interpolation, 
with the interpolating polynomial being designed for flatter performance 
over the utility band than the linear spline. 
It's already very much better at 3rd order, and if you do something like 
4th to 5th order with 2x oversampling, it's essentially perfect.
-- 
Sampo Syreeni, aka decoy - decoy@iki.fi, http://decoy.iki.fi/front

And another:

As far as compensation: Taking linear as an example, 
we know that the response rolls off (“sinc^2"). 
Would you compensate by boosting the highs? 
Consider that for a linearly interpolated delay line, 
a delay of an integer number of samples, i, 
has no high frequency loss at all. 
But that the error is max it you need a delay of i + 0.5 samples. 
More difficult to compensate for, would be such a 
delay line where the delay time is modulated.

A well-published way of getting around the fractional problem is 
allpass compensation. But a lot of people seem to miss that this 
method doesn’t lend itself to modulation—it’s ideally suited for 
a fixed fractional delay. Here’s a paper that shows one possible solution, 
crossfading two allpass filters:

http://scandalis.com/jarrah/Documents/DelayLine.pdf

Obviously, the most straight-forward way to avoid the problem is to 
convert to a higher sample rate going into the delay line 
(using windowed sinc, etc.), then use linear, hermite, etc.
tap commented 8 years ago

The nw/delay branch was just a stub and I ended-up starting over. The initial Delay class and unit test is now implemented in a working form on the master branch. But there are some caveats:

To use this as a building block for filters it is fine to have a version without interpolation. The minimum delay time on the other hand is a serious problem. There are a couple of ways we could approach this:

Note: we already resize the history when we call adapt() in the process method. So it makes perfect sense to add the block size at that point if we go that route.

nwolek commented 8 years ago

Been doing some thinking today about how LinearInterpolation would work using CircularStorage as a the foundation. I am going to be working and testing on a non-DRY implementation in branch to see what this thinking yields.

nwolek commented 8 years ago

@tap I have the Sample operator working of the linear interpolation delay (and passing tests), but running into some challenges with the SampleBundle operator. Hitting the thread safety assertion. Wondering if you can take a look and see if something jumps out at you. If you need me to explain, we should set up a time to Skype, but that might not happen until next week.

Operator is here: https://github.com/jamoma/jamoma2/blob/nw/delay/includes/library/JamomaDelay.h#L165

Test throws assertion here: https://github.com/jamoma/jamoma2/blob/nw/delay/tests/Delay/Delay.cpp#L363

tap commented 8 years ago

Hi @nwolek -- what I see in the debugger is this:

  1. In JamomaDelay.h, line 173, we crash at the point you mention, when channel == 1
  2. At this point I can see that the size of mHistory == 1, meaning only channel 0 is legit.
  3. In dereferencing the uninitialized memory that assertion fails

A little higher up, at line 167, you call adapt. This will adapt the number of channels to the stereo input, but because the observer is commented out (line 124) the history is left alone as a mono buffer.

Much less important, at line 168, there is no need to call adapt a 2nd time. You can assign mOutput directly to tailOut.

Hope this helps!

nwolek commented 8 years ago

OK, I commented out the Observer because I did not exactly understand what it was doing and wanted to eliminate variables. Your explanation above helped me see what it is doing.

Adding the Observer back in resolves the thread safety issue. Glad to know that thread check is working! Now I just need to get it passing the test.

My immediate goal for this issue is to get the linear interpolating delay to pass several tests with a non-DRY solution. When I get there I will issue a pull request to @tap. This should allow us to work on more DRY solutions and ensure they produce the right results.

nwolek commented 8 years ago

@tap - My tests are showing that the adapt(x) operation zaps the memory in mHistory. It doesn't happen in your version without interpolation (line 86), but does in my version with interpolation (line 171). Not sure what the difference is. Might require us to Skype and discuss. Message me if you think you might have time before the weekend.