move compressor to TG menu and add parameters#973
move compressor to TG menu and add parameters#973soyersoyer wants to merge 4 commits intoprobonopd:mainfrom
Conversation
Reviewer's GuideThis PR refactors the compressor from a single global toggle to fully configurable per‐tone-generator, introducing six new TG parameters (enable, pre-gain, attack, release, threshold, ratio) and propagating them through the config loader/saver, the synth engine, and the UI menu, with backward compatibility for existing global settings. Class diagram for updated compressor configuration in CPerformanceConfig and CMiniDexedclassDiagram
class CPerformanceConfig {
+bool GetCompressorEnable(unsigned nTG) const
+int GetCompressorPreGain(unsigned nTG) const
+unsigned GetCompressorAttack(unsigned nTG) const
+unsigned GetCompressorRelease(unsigned nTG) const
+int GetCompressorThresh(unsigned nTG) const
+unsigned GetCompressorRatio(unsigned nTG) const
+void SetCompressorEnable(bool, unsigned nTG)
+void SetCompressorPreGain(int, unsigned nTG)
+void SetCompressorAttack(unsigned, unsigned nTG)
+void SetCompressorRelease(unsigned, unsigned nTG)
+void SetCompressorThresh(int, unsigned nTG)
+void SetCompressorRatio(unsigned, unsigned nTG)
-bool m_bCompressorEnable[CConfig::AllToneGenerators]
-int m_nCompressorPreGain[CConfig::AllToneGenerators]
-unsigned m_nCompressorAttack[CConfig::AllToneGenerators]
-unsigned m_nCompressorRelease[CConfig::AllToneGenerators]
-int m_nCompressorThresh[CConfig::AllToneGenerators]
-unsigned m_nCompressorRatio[CConfig::AllToneGenerators]
}
class CMiniDexed {
+void SetCompressorEnable(bool, unsigned nTG)
+void SetCompressorPreGain(int, unsigned nTG)
+void SetCompressorAttack(unsigned, unsigned nTG)
+void SetCompressorRelease(unsigned, unsigned nTG)
+void SetCompressorThresh(int, unsigned nTG)
+void SetCompressorRatio(unsigned, unsigned nTG)
-bool m_bCompressorEnable[CConfig::AllToneGenerators]
-int m_nCompressorPreGain[CConfig::AllToneGenerators]
-unsigned m_nCompressorAttack[CConfig::AllToneGenerators]
-unsigned m_nCompressorRelease[CConfig::AllToneGenerators]
-int m_nCompressorThresh[CConfig::AllToneGenerators]
-unsigned m_nCompressorRatio[CConfig::AllToneGenerators]
}
CPerformanceConfig <.. CMiniDexed : uses
Class diagram for updated CUIMenu compressor menu and parameter handlingclassDiagram
class CUIMenu {
+static const TMenuItem s_TGMenu[]
+static const TMenuItem s_EditCompressorMenu[]
+static const TParameter s_TGParameter[]
+static std::string TodB(int nValue)
+static std::string ToMillisec(int nValue)
+static std::string ToRatio(int nValue)
}
class TMenuItem {
}
class TParameter {
}
CUIMenu o-- TMenuItem
CUIMenu o-- TParameter
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @soyersoyer - I've reviewed your changes - here's some feedback:
- The repeated loops of PropertyName.Format/GetNumber and SetNumber for each compressor parameter suggest a helper or data‐driven approach would reduce boilerplate and risk of mismatches.
- Maintaining six parallel arrays for compressor state across CPerformanceConfig and CMiniDexed is error‐prone—consider bundling them into a struct or small class to improve cohesion.
- The new TodB/ToMillisec/ToRatio formatters in CUIMenu could be moved into a shared formatting utility to keep the menu class focused solely on navigation and rendering logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The repeated loops of PropertyName.Format/GetNumber and SetNumber for each compressor parameter suggest a helper or data‐driven approach would reduce boilerplate and risk of mismatches.
- Maintaining six parallel arrays for compressor state across CPerformanceConfig and CMiniDexed is error‐prone—consider bundling them into a struct or small class to improve cohesion.
- The new TodB/ToMillisec/ToRatio formatters in CUIMenu could be moved into a shared formatting utility to keep the menu class focused solely on navigation and rendering logic.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Build for testing: |
|
Build for testing: |
|
I'm impressed. 👍 It's only possible to set the compressor for TG1. I can't access the compressor for TG2 - TG8. When you try to control the compressor via the TG2 - TG8 UI, it always jumps directly to TG1. The shortcut (pressing and turning the encoder) doesn't work in the compressor menu either. The compressor itself works really well :-) |
|
I still have a few questions:
|
|
As we still have a comprehensive FX patch waiting to become ready, I'm hesitant to merge this. Also I guess this would break the performance format? |
|
Build for testing: |
This surprised me, I fixed it. Thanks.
Just like before, from RPi1 as well. Only the configurability of the compressors has changed.
Not necessarily like that. Part2, Part3 PR was more of a demo to see if it was worth doing something like that. It would be nice to somehow link the TGs together, to find a solution that would be good for larger Performances, Unison, Part things.
This is also a good idea. We'll keep that in mind when linking to TGs.
This can open the current format, thus disabling all compressors if CompressorEnable is set and its value is 0. But this can be improved, CompressorEnable=0 can be written for older MiniDexeds if all compressors are disabled. But I don't know if it's worth the hassle.
Will jnonis still work on it? The compressor is also used by the fx-full code, this change will be basically compatible with it. |
|
Build for testing: |
|
Build for testing: |
|
Build for testing: |
|
Changed the preGrain range to -20 .. 20 dB and threshold to -60 .. 0 dBFS. |
|
Build for testing: |
|
Build for testing: |
|
Hey, hey, hey, the compressor is fun. Crying over nothing serious: A ratio of 5:1 is more of a limiter than a musical compressor. I think a starting value of 2:1 is more suitable. Just a thought: Since I like to overdo it sometimes, a warning about clipping would be helpful. Is it possible to evaluate the volume and, if the volume exceeds, say, -3 dB, set a digital output for a warning LED? Peter |
|
Good to hear! :)
This site said that 5:1 is the 'medium' ratio, so I think that's fine.
I'll look for a limiter at the end of the chain, and I think that can solve it. But the warning may also appear on the display for a short time. |
|
The compressor in Synth_Dexed could be placed after MasterVolume and parameterized to act as a limiter. Is this a good idea? Could it be parameterized in the Performance files also? |
|
The parameterizable compressor that you have integrated into the TGs, is it technically the same as the one we have always known as a limiter under effects?
Yes and yes, absolutely agree. The best way to protect your ears from unwanted happy little accidents :-) 🙉 |
|
The compressor is well implemented, and I wish it were adopted for this project. Then I could bring all the performances to the same volume level. The performances would work on older versions without a compressor. Peter |
Yes, it's the same. Look at this current implementation: if you enable the compressor in the Effects menu, it will enable all TG compressors.
That's sounds good! With 32 TGs, layering of performances is also possible. I'm going to add a limiter at the end of the chain. |
If it were possible for you, a fully parameterizable compressor would be ideal (with the default parameters as a limiter) |
af64cbe to
a9052ce
Compare
|
I added the limiter before MasterVolume. Parameters:
|
|
Build for testing: |
|
Thank you @soyersoyer , Peter |
|
Then we just have to wait for the integration :) |
|
@probonopd What do you think? Do you need else in this? |
|
I could offer to extend the wiki after the merge.🤖 |
|
I tested something else: Performance saving and loading works great. Peter |
Summary by Sourcery
Introduce per-tone generator compressor controls by adding enable, pregain, attack, release, threshold, and ratio parameters to performance configuration, synthesizer engine, and the UI, and move compressor settings from global effects to the TG menu.
New Features:
Enhancements: