-
Notifications
You must be signed in to change notification settings - Fork 19
lv2-custom/RipplerX.lv2: Update dsp.ttl to v1.5.18 #26
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
base: vangelis
Are you sure you want to change the base?
Conversation
From v1.3.4, Zynthian's original snapshot.
This commit applies changes to existing LV2 parameters,
and adds the new input audio port group ("Sidechain").
|
Sorry about the bad initial title! I was so sure I had pasted a more relevant one into the PR submission form. |
|
Hi @jpetso, since you CC me I'll share some thoughts, even though the decision about if, how and when it'll be updated is to the project owners and I have nothing to do about that. I think this will mainly work, here some questions about it anyway. When comparing the your ttl with the current "official" one from the upstream release, I see:
Tell me what you think. |
|
Hi @johannesmenzel, thanks for having a look! As the original designer of these groups, your feedback is important to take into consideration. Let's hop right into your questions.
This came directly from the original .ttl file that was generated by the RipplerX build system, and I decided not to change it. The RipplerX source code does indeed specify it as My understanding is that the build system (powered by JUCE) then compiles this code into a helper binary, which reads out the min/max/default values of each parameter and generates the actual .ttl file from it. I don't have enough insight into why a constant 0.0f would turn out as not-exactly-zero, but I'm sure there's an explanation that makes a lot of sense. Thankfully, the Zynthian interface also shows it as rounded down to 0.
"Noise Release" (a.k.a. In "Noteworthy features that I'm aware of":
In "Challenges and questions":
I agree that we should understand and answer this before merging my change. I don't have a good understanding of what Zynthian or JUCE would do to ensure that a previously stored value (e.g. from a Zynthian snapshot) gets clamped to the valid range. And in the current state, there definitely isn't anything that would transform the old range 20 to 20000 linearly to the new range -1 to 1.
That's a good point, perhaps we should allow this option. I think I saw in a parameter list somewhere that Zynthian also has a file selection control, not sure if this would be suitable here though?
Yes, and please note my remarks on this topic at the top of the "Challenges and questions" section in the top comment. I actually thought your "Material" and "Tone" groups for resonators A and B were rather nice, I think the same names/concepts work pretty well in the "Mallet" section as its number of parameters increased and groups had to be expanded. Splitting "Noise - Velocity Modulation" into two different "Velocity Modulation" groups for "Noise - Filter" and "Noise - Envelope", respectively, makes the names longer but I remains intuitive and logical. Personally, I think because the two "Velocity Modulation" groups are right next to their base parameter groups instead of the very bottom, the Noise section is actually more usable than before. Feel free to disagree and make suggestions. This leaves the resonator section for the most awkward grouping. The group names I used are definitely a trade-off between what logically makes the most sense, and what saves space. Here's a rough parameter grouping that represents the organization of the RipplerX codebase with regards to resonators:
There are a few challenges in there. One is that there are 5 per-partial parameters, or 9 if you include velocity modulation. Ratio does not get modulated. 10 if you include number of partials. Tube models only have a single parameter (radius), no modulation either. Low cut is a filter and I didn't feel comfortable grouping it together with three other per-partial parameters in "Tone", while "Material" seemed like a random assortment of number of partials, two per-partial parameters, and the tube radius for a different model type. So I wanted to move (low)cut out of the per-partials group, while putting as many per-partial parameters together as possible. The result is two exclusively per-partial groups, now also called "Material" and "Tone", both with velocity modulation. The "ratio" and "tube radius" parameters are indeed thrown in with "model type" and "number of partials" into a new "Model" group. Ideally they'd be separated out, but two extra groups per resonator seemed like a bad trade-off. This leaves decay, release and (low)cut, all of which apply to both non-tube and tube models. I called this "Attenuation", because all three take away a part of the resonator sound. Admittedly this is not a great grouping. Another approach would be something like this:
This would increase the number of groups for each resonator from 4 to 5, but might be preferable in terms of clarity? Happy to take suggestions for a different kind of grouping, if you can think of something good. I'm relatively happy with the three global resonator setup groups, "Setup", "Mix" and "Pitch". We could consider moving "resonator gain" into "Setup" analogously to "mallet mix" and "noise mix" parameters in their respective "Mallet - Setup" and "Noise - Setup" groups. Then "Resonator Mix" would be renamed to "Resonator Coupling". (Mallet and noise mix can be velocity-modulated so I'd rather leave them in these groups, a "Global Mix" group would have to span at least two groups anyway and I'm not sure it's really an improvement that way.) I wonder if we should add a dash to the "Resonator Setup/Mix/Pitch" groups, or perhaps remove dashes everywhere else so we'd have "Resonator A Model", "Mallet Material", "Noise Envelope - Velocity Modulation" and the likes. In summary, you're right that there's a bit of awkwardness here and there, but one cannot say that I didn't at least put some thought into all these groupings :) |
In the upstream dsp.ttl from their release page (v1.5.18) I read:
No, I think the grouping is quite reasonable. My thoughts were solely about the group names and their aestetics/readability. You know, it makes totally sense to have a group of the material parameters of resonator B, which excludes the tube models but I wouldn't necessarily call it "Resonator B - Material (excl. tube models)" Otherwise it's great, I think! |
|
Apologies for the very late response, I haven't forgotten about this but I'm not very consistent with my open source contributions right now. Thanks for being patient.
Yep, you're indeed right, the value changed from 5000 to 20000 and I missed this change in my three-way merge. Fixed in this latest upload. I also added the "User File" option in "Mallet Type" as you suggested. The group naming is still the same for now, still needs another go at the naming. If anyone else wants to take a shot, by all means give it a go, I don't have to hog this PR. |
This commit introduces new parameters that were added since v1.3.4. It also reorganizes the parameter groups somewhat, for compactness and consistency (hopefully) in light of the new parameter entries.
Okay, in the interest of aesthetics, I took out the "(excl. tube models)" suffix. Perhaps in the future, Zynthian can support a .ttl property to hide or show certain groups and parameters only when certain enum values of another parameter are selected. I also cut down on the number of dashes (or hyphens, really) in the group names. So "Mallet - Setup" is now "Mallet Setup" for example, and "Noise - Filter - Velocity Modulation" is now "Noise Filter - Velocity Modulation". I did not remove all of them, in the interest of readability ("Resonator A - Model": indicates more clearly that this is not "a model") and group continuation ("Noise Filter - Velocity Modulation" is kind of page We could also consider just going with a simpler |
RipplerX has had a few nice updates since it was originally added in zynthian/zynthian-sys@c8d4720 plus custom dsp.ttl file in #20. See past forum discussion here. (CC @johannesmenzel.)
I thought I'll see if I can update it from v1.3.4 to v1.5.8. It was a little bit of effort but it's working now. To do this, I built both v1.3.4 and v1.5.18 on my Zynthian. Then I went through the diff between both dsp.ttl files and applied the changes to Zynthian's modified dsp.ttl in classic three-way-merge fashion. Then I spent way too long pondering how the new parameters should be organized into 4-knob input groups.
This is the result. I split the change up into two commits:
Noteworthy features that I'm aware of:
Challenges and questions:
plug:a_cutandplug:b_cutparameters changed their min/max values to a completely different range: previously 20 to 20000, now -1 to 1.plug:mallet_type(new parameter) has a bunch of"Reserved"values as well as"User File". I left those enum values out from the custom dsp.ttl, because they're not really useful in the Zynthian interface anyway?"User File"enum option (i.e. use custom filter sample) could be useful, not sure how it would be integrated though.plug:a_partialsandplug:b_partialshave new enumeration values labelled "1" and "2", listed only after the existing ones "4", "8", "16", "32" and "64". The internalrdf:valuefor the new "1" and "2" values is larger than the original ones and presumably can't be listed first ifrdf:valuedetermines the order?Unrelated to the upgrade:
After the change, the list of parameters is now grouped like this:
Here is a Release build for ARM that works on the Zynthian, at least for my RPi 5. Needless to say, this goes hand in hand with the amended dsp.ttl from this PR.