Skip to content
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

Increase resolution of patch cable strength in the sound editor #17

Merged
merged 1 commit into from Jun 20, 2023

Conversation

bfredl
Copy link
Contributor

@bfredl bfredl commented Jun 7, 2023

This is not nearly done yet but it would make sense to use draft PR:s to indicate what people are exploring/working on.

The Sound engine (and synth XML files) support much higher resolution for patch cable strength internally, than the -50 to +50 steps available in soundeditor when you make modulation mappings.
It is already possible to adjust the strength by editing the XML manually (or by some third party editor which could have implemented this), and the resolution could be increased in the sound editor only, while keeping full backward/forward compat of synth XML files.

to be done:

  • rework the conversion code to use floats instead of magic integer constants, and allow even higher resolution on demand.
  • rework the OLED UI layout (the additional digits might overlap parameter name, need to use smaller font for digits)
  • update the UI code for 7seg display this should already be handled by MenuItemDecimal but a 7seg owner verifying this would be good

@litui
Copy link
Collaborator

litui commented Jun 7, 2023

Interesting idea of having draft PRs as works-in-progress. I tend to see that as a use for "enhancement" issues rather than PRs, but I'm curious what @jamiefaye thinks.

@bfredl
Copy link
Contributor Author

bfredl commented Jun 7, 2023

the benefit of draft PR:s is that you can comment/give suggestions directly on the code diffs and have it tracked as part of the PR discussion even as code is updated/rebased.

@litui litui added the enhancement New feature or request label Jun 8, 2023
@bfredl bfredl mentioned this pull request Jun 9, 2023
14 tasks
@bfredl bfredl marked this pull request as ready for review June 11, 2023 16:10
@bfredl
Copy link
Contributor Author

bfredl commented Jun 11, 2023

I opted to document the meaning of the integer constants instead of changing to float, which seemed a more conservative change and works well enough.

Updated the UI for OLED. I think the 7seg UI should work as it is as this is implemented by the MenuItemDecimal parent class and look the same as for (unmodulated) Osc1/osc2 transpose which also has two decimals and max two digits before the decimal point.

@jamiefaye
Copy link
Collaborator

jamiefaye commented Jun 13, 2023

On 7LED, this fails with a visit to the abort_handler. Repro: Enter a few notes into the clip, press Select to edit parameters. Freezes immediately.

@PaulFreund
Copy link
Collaborator

I didn't look at the code yet but there seem to be different implementations of numeric values for different synth parameters and cable patches. Some of these are not supported as MIDI CC and for some negative values didn't work with any gear I tested. Would it make sense to touch this problem at the same time? Also would it make sense to restrict the range to what MIDI can represent?

@bfredl
Copy link
Contributor Author

bfredl commented Jun 15, 2023

@PaulFreund not sure I follow, are you talking about generating midi CC from the mod matrix? Or more like editing synth params over CC?

@PaulFreund
Copy link
Collaborator

I have an external MIDI controller connected (in this case via DIN) open a parameter like Transpose and hold LEARN/INPUT, then I move the CC knob and the display shows LEARNED. I can now control the parameter with the CC. This does not work with Detune Number (Can't Learn) and it also does not allow negative numbers for some reason. For patch cables (For example mapping transpose to LFO2) I can learn a CC to the cable strength but only in positive direction, so if we want to keep compatibility we might need to limit the range to what MIDI supports

@bfredl
Copy link
Contributor Author

bfredl commented Jun 15, 2023

That sounds to me like we should rather improve the midi learn implementation, I e have more CC values as a modulation source so you can specify a range and offset for CC controlled parameters (this will also apply to patch cables as these can be modulated in turn)

There is no compatibility issue with this PR tho as it only improves the resolution and doesn't change the range, so I'd rather address the MIDI issue in a follow up.

@PaulFreund
Copy link
Collaborator

Perfect, just wanted to make sure :) Thanks!

…- 50.00

The Sound engine and its XML files already support much higher resolution internally.
Thus this change keeps FULL backward/forward compat of synth XML files.
It is already possible to adjust the strength by editing the XML manually and then
loading it into stock firmware.

This feature is very useful i e when patching note value to osc1/osc2
pitch, to create subtle tuning variations or even faking high-EDO
tunings (although we plan supporting the latter natively as part of the
alternate tuning work).

Use MenuItemDecimal with two decimal places to keep the same effect of
the encoder by default. Then use <|> encoder to select decimals,
just like for the (unmodulated) osc1/2 transpose.
@bfredl
Copy link
Contributor Author

bfredl commented Jun 17, 2023

@jamiefaye before the 7SEG emulation is in place I'm fumbling around a bit in the blind, but I pushed a change which might (or might not) fix the 7SEG issue. would you mind try again?

@jamiefaye
Copy link
Collaborator

jamiefaye commented Jun 17, 2023 via email

@jamiefaye jamiefaye merged commit 2c2140c into SynthstromAudible:community Jun 20, 2023
@ok-reza
Copy link
Collaborator

ok-reza commented Jul 5, 2023

Would love to see this feature be toggleable in the Community Settings.

@sapphire-arches
Copy link
Collaborator

Would love to see this feature be toggleable in the Community Settings.

that should be relatively straightforward to implement. Can you file a bug? Easier to track that way than a comment in a closed PR =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants