Convolution effect and arbitrary data storage in the patch #8219
Conversation
mx
force-pushed
the
convolution
branch
2 times, most recently
from
e756c2d to
f4641aa
Compare
Still need to figure out undo.
baconpaul
left a comment
baconpaul
left a comment
There was a problem hiding this comment.
Looks good! I think theres only one thing (the continue) which might be an actual bug but otherwise just tiny comments and questions. Happy for you to merge once you've read them.
| } | ||
|
|
||
| // TODO: Just change this entire thing to a single copy operation? | ||
| storage.getPatch().fx[s].user_data = fxsync[s].user_data; |
There was a problem hiding this comment.
Yeah I agree this is probably fine but the copy could take a long time - any worry about two changes in a row? I'm almost wondering if we want something like an atomic pointer swap instead.
I don't think we change it now but let's remember this. Perhaps this is just saying that I agree with your comment.
There was a problem hiding this comment.
Added note about maybe an atomic pointer swap or copy-on-write instead.
It's tough to see a memcpy() of tens of megabytes taking a long time but this is a good point. At least we're not in an active audio path here.
There was a problem hiding this comment.
Couldn't we store just the path to the IR sample instead of the whole sample in memory? That would be a lot faster and would take up less RAM in the undo stack. Then on redo we repeat the load from disk etc.
There was a problem hiding this comment.
we need the data in memory to convolve it. this point is how it gets loaded. so definitely the path here wouldn't be useful.
3 participants
Closes #7817