-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improvements. #19
Comments
Hey @copych, I don't find the time to actively extend the library but you're welcome to add code. My opinion:
Whenever you have something send a pull request and I will have a look at it and merge. Thanks, |
Hello! Thanks for the quick and detailed answer. I completely understand the lack of time for hobbies and the shift in focus to something else. This is something I struggle with myself. |
Yes you are right. One would want to choose the ESP32 with more RAM. I normally use bigger LUTs but when you have good experience, go for it! I can help with implementation. A header with the data points should do it. Something like |
Thanks! I'll try ) |
Yes, precalculated tables worked ok. I've made a pull request, cause basic functionality is there already. I'm planning to add some more effects, but now I'd like to ask you about the routing in this DSP.
so I commented it out, cause it seems that hardcoded 384 value won't work for different settings. |
I took a list for the sake of easiness. When this turns out to be a cpu hog we could change it to a fixed size array of pointers. Insertion and removal of nodes is probably more complicated then. |
Parallel chains are cool. Maybe it would be easier to add nodeLists which are only processed when size is > 0. E.g. ChainA, ChainB, ChainC... |
Yes, 3 or 4 or even just 2 chains are quite enough for the MPU. |
Some information gathered. I am not that fast on coding, so I had some reading prior to experimenting ) |
Calculations for chain forming are pretty seldom and definitely not per sample. So std::vector will do it. Anyway, I assume that most of the CPU time is used in node calculations. But can be a future improvement. |
Hello, @garygru , it's a great library! Do you still support it? I ask because I have some ideas, but I am not a real coder, and my manipulations will spoil the beauty of your code )
Ideas are:
First, most obviously, there should be more effects to choose from, say, reverb is a must-have, chorus, flanger, etc. are highly welcome, and there are a lot of them around github already implemented as classes or as sets of functions with permissive licenses, mostly sample-wise, so require quite a little changes. (I could make these contributions).
Second, I have tried to implement some optimizations, like faster floating asm routines, exchanging divisions to multiplications where possible, and using of lookup tables for some common cases (sin(), cos(), tanh()). But (in my understanding) for the time being the library provides separate classes for the i2s, dsp and for the nodes, so I couldn't share lookup tables and functions library-wide. The lack of coding skills makes me think it's not possible with just including updated dspHelpers.h, but either requires passing pointers to the lookup tables to the newly created instances, or making some host factory class which would do that and something else silently.
And the third, I liked very much that you can form the fx-chain dynamically, but for now there's no way to remove or change the set or the order of the chain. Probably this was a ToDo )
I understand that for one's particular needs one could just include your library files and edit them, but this library is worth developing as a library to my mind.
Please, what do you think on the matter? Cheers!
PS I forgot one important thing. WROVER modules include 4/8MB of PSRAM which is a great solution that will let the effects to use almost any settings in the matter of timing.
The text was updated successfully, but these errors were encountered: