New Reverb and Chorus API versus actual API

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

New Reverb and Chorus API versus actual API

Ceresa Jean-Jacques

Hi All,

 

For those amoung you using actual Reverb and Chorus API (or associated shell commands).

You have noticed that theses API (or commands) allow to change one parameter for all fx unit.

For example if fluidsynth has been started with synth.effects-groups set to 2, the synthesizer has 2 fx units.

That leads with MIDI channel 0 using fx unit 0, and MIDI channel 1 using fx unit 1.

The lack of actual API that it doesn't allow to change parameters of only one fx unit (all fx unit (0,1) are affected by this change).

 

To overcome this issue a new set of Revert and Chorus API will be proposed with one added parameter which is the fx unit index (0 or 1)

to which the change is applied. This new API set can also behave like actual API set if fx unit index is -1.

That means that the actual API set will become redundant as soon the new API set will be proposed.

So it seems that it should be preferable to deprecate the actual API set (that should be replaced by the new API set).

 

Now the question is: does the deprecation of actual Reverb and Chorus API could cause issues ?

 

Thanks for your feedback.

jjc

 

 


_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: New Reverb and Chorus API versus actual API

Marcus Weseloh
Hi JJC,

Am Do., 17. Sept. 2020 um 11:52 Uhr schrieb Ceresa Jean-Jacques
<[hidden email]>:
> To overcome this issue a new set of Revert and Chorus API will be proposed with one added parameter which is the fx unit index (0 or 1)
> to which the change is applied. This new API set can also behave like actual API set if fx unit index is -1.
> That means that the actual API set will become redundant as soon the new API set will be proposed.
> So it seems that it should be preferable to deprecate the actual API set (that should be replaced by the new API set).
> Now the question is: does the deprecation of actual Reverb and Chorus API could cause issues ?

My guess is that most users simply use a single stereo output from
FluidSynth. And that most users will not even be aware that FluidSynth
has support for multi-channel output, let alone support for multiple
fx units. So based on that assumption, I would vote against
deprecating the existing API and shell commands. Otherwise we would
force users who have no interest in multi-channel output or multiple
fx units to think about which fx unit they want to target. I know
there is the -1 catch-all, but that is still another "magic" parameter
to those functions that users need to think about.

The proposed new API functions simply add a "2" to all names. Maybe we
could name the new functions differently to make their intended use
more clear? So instead of "fluid_synth_set_reverb_roomsize2" we could
name it something like "fluid_synth_set_reverb_unit_roomsize". That
might make it clear that those functions target a particular unit. And
then remove the -1 catch-all, as we still have the old API around if
you want to target all existing fx units.

Same for the shell commands: keep the old commands like
"rev_setroomsize" unchanged, then add new commands like
"revunit_setroomsize".

TLDR: I am against complicating the common use-cases for a rarely-used
special use-case. Adding new API functions and shell commands for the
special case and keeping the existing API unchanged sounds better to
me.

Cheers
Marcus

_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: New Reverb and Chorus API versus actual API

fluid-dev mailing list
> My guess is that most users simply use a single stereo output from
FluidSynth.

Probably. And I must admit that I'm still not convinced that it makes
sense from a musically perspective to control all effects groups
independently. However, I understand the technical need for it, so I'll
buy it. [The other change about the buffer mapping we still have to
discuss and "design". I haven't forgotten.]

> The proposed new API functions simply add a "2" to all names.

That was my proposal. I find it hard to find a suitable naming
convention for those new functions. Initially we had
fluid_synth_set_fx_reverb_roomsize(). Now you're suggesting
fluid_synth_set_reverb_unit_roomsize(). However, the corresponding
setting is named effects-groups. And: do we add this "magic word" (unit,
fx, group,...) before or after the "reverb" word? ...Pretty confusing.
Simply adding a 2 is easier to write, easier to remember and consistent
with a few other places among our API (new_fluid_sequencer,
new_fluid_audio_driver), I think.

While I agree that the existing API functions should be kept, I would
still vote for their deprecation. I see this deprecation notice as an
advice to developers to use those new functions in new code instead. Yes
I get your point with the -1 catch all. I don't think it would hurt to
make developers think about that in new application. In any case, I'm
open which way to go here. Just let me know if you insist.

However, I vote against introducing new shell commands. Instead, the
existing commands, should become smart enough to detect whether they
have been called with one or two parameters. If only one: the old
behaviour takes place. If two, the user wants to apply the change only
for the given fx unit, e.g.:

rev_setroomsize [fxid] num

Side note: those existing shell commands have already been deprecated.
Ofc. this should be reverted once this feature will be implemented.

Tom

_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: New Reverb and Chorus API versus actual API

Marcus Weseloh
Am So., 11. Okt. 2020 um 14:29 Uhr schrieb Tom M. via fluid-dev
<[hidden email]>:
> > The proposed new API functions simply add a "2" to all names.
> That was my proposal. I find it hard to find a suitable naming
> convention for those new functions.

Yes, I completely understand. And I had forgotten (or overlooked) the
other naming proposals. I still think as a long time solution, having
meaningful names for API functions is way better than simply adding
"2" to the end. Especially if we plan on deprecating and eventually
removing the original functions. Because then we are left with an API
that contains some functions ending in "2", with no clear indication
why they are named like this. Unless you plan on another deprecation
round and then removing the "2" again at a later date, of course.

So I would still vote to go with a "future-proof" name for the new
functions, something like "fluid_synth_fx_set_..." or
"fluid_synth_set_fx_..." or "fluid_synth_set_reverb_unit_...".
I actually feel more strongly about the naming than about the
deprecation of the old API functions, to be honest. In my mind, API
functions and shell commands target quite different audiences. API
functions are always for programmers... and we can expect programmers
to read the API documentation and understand what an fx unit is and
how it relates to multi-channel vs. single-channel output. So forcing
them to always specify an fx unit index or -1 as a catch-all would be
ok, I guess.

> However, I vote against introducing new shell commands. Instead, the
> existing commands, should become smart enough to detect whether they
> have been called with one or two parameters.

Good idea! Not perfect in my opinion, as the documentation for those
commands would still need to show the optional fx unit param,
triggering "normal" users to wonder what they are... but a good
compromise. In any case, much better than removing the old commands
and forcing everybody to specify a magical -1 for those functions.

> Side note: those existing shell commands have already been deprecated.
> Ofc. this should be reverted once this feature will be implemented.

Ah, I wasn't aware of that. What was the reason for deprecating those
shell commands?

Cheers
Marcus

_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: New Reverb and Chorus API versus actual API

Ceresa Jean-Jacques
In reply to this post by Ceresa Jean-Jacques

>Tom: Side note: those existing shell commands have already been deprecated. Ofc. this should be reverted once this feature will be implemented.
>Marcus Ah, I wasn't aware of that. What was the reason for deprecating those shell commands?

The reason is that those actual shell fx commands are redundant with respective settings (synth.reverb.xxxx, synth.chorus.xxxx).

Instead, the user can use the commands: set synth.reverb.xxxx, set synth.chorus.xxxx which set the respective setting for all fx unit. Those

settings are real time and can be changed in a config file.

In the perpective of definitive deprecation of actual fx shell commands, the  new fx shell commands implementation (https://github.com/FluidSynth/fluidsynth/pull/673) is only for individual fx unit.

(not for all fx units).

 

Also, because of the redundancy with new fx API (through the unit index -1 as a "catch-all"), I am favorable to deprecate the actual fx API. Otherwise we will get a complex

set of fx API functions and future developers will not advised (or confused) to use those new functions. Indeed, the use of -1 parameter as a "catch-all" is used in other API functions, so it

would not hurt developers.

 

Also, in the perpective of actual fx API deprecation, suffixing the new fx API name with 2 becomes not approriate and explicit name should be preferable. For example.

fluid_synth_set_reverb_fx_xxxxx(synth, fxunit_idx,xxxxx), fluid_synth_set_chorus_fx_xxxxx(synth, fxunit_idx,xxxxx).

Thanks for opinions.

jjc


_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: New Reverb and Chorus API versus actual API

fluid-dev mailing list
> The reason is that those actual shell fx commands are redundant with
> respective settings (synth.reverb.xxxx, synth.chorus.xxxx).> Instead, the user can use the commands: set synth.reverb.xxxx, set
> synth.chorus.xxxx which set the respective setting for all fx unit. Those

JJC is correct.

> fluid_synth_set_reverb_fx_xxxxx(synth, fxunit_idx,xxxxx),
> fluid_synth_set_chorus_fx_xxxxx(synth, fxunit_idx,xxxxx).

I kept thinking about this. If we decide against the "2"-suffix my
current favorite is:

fluid_synth_set_reverb_group_xxxxx()
fluid_synth_set_chrous_group_xxxxx()

This would be consistent with the synth.effects-groups setting.
And, like JJC, I would still vote for deprecating the old API functions.


Tom

_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: New Reverb and Chorus API versus actual API

Ceresa Jean-Jacques

>my current favorite is:fluid_synth_set_reverb_group_xxxxx() fluid_synth_set_chrous_group_xxxxx()

>This would be consistent with the synth.effects-groups setting.

 

Indeed, replacing "fx" by "group" seems a better naming choice.

jjc

 

 


_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: New Reverb and Chorus API versus actual API

Marcus Weseloh
In reply to this post by fluid-dev mailing list
Hi,

Am So., 18. Okt. 2020 um 17:41 Uhr schrieb Tom M. via fluid-dev
<[hidden email]>:
> I kept thinking about this. If we decide against the "2"-suffix my
> current favorite is:
>
> fluid_synth_set_reverb_group_xxxxx()
> fluid_synth_set_chrous_group_xxxxx()
>
> This would be consistent with the synth.effects-groups setting.
> And, like JJC, I would still vote for deprecating the old API functions.

So for the shell we introduce the proposed new commands to set
specific fx group settings.  We keep the old fx shell commands
deprecated and document that "set synth.reverb.xxx"  should be used
instead (if not already done).
And for the API we use the function names suggested above and
deprecate the old API functions.

Sounds good to me!

Cheers,
Marcus

_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: New Reverb and Chorus API versus actual API

Ceresa Jean-Jacques

Hi,

 

>So for the shell we introduce the proposed new commands to set specific fx group settings.

>We keep the old fx shell commands deprecated and document that "set synth.reverb.xxx" should be used instead (if not already done).

>And for the API we use the function names suggested above and deprecate the old API functions. Sounds good to me!

 

Ok, I will update the PR according to the discussion.

jjc

 


_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: New Reverb and Chorus API versus actual API

fluid-dev mailing list
Marcus, wait - remember:

>> [Tom] However, I vote against introducing new shell commands. Instead, the
>> existing commands, should become smart enough to detect whether they
>> have been called with one or two parameters.
> [Marcus] Good idea! [...]

Pls. what has changed your mind to "introduce the proposed new [shell]
commands"?

With your suggestion, we would have three different ways of setting
reverb+chorus. When changing the existing command and un-deprecate
them, we'll only have two. While still retaining backward
compatibility.

Tom

_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: New Reverb and Chorus API versus actual API

Marcus Weseloh
Am Di., 20. Okt. 2020 um 17:49 Uhr schrieb Tom M. via fluid-dev
<[hidden email]>:
> Pls. what has changed your mind to "introduce the proposed new [shell]
> commands"?
>
> With your suggestion, we would have three different ways of setting
> reverb+chorus. When changing the existing command and un-deprecate
> them, we'll only have two. While still retaining backward
> compatibility.

Argh... thanks for that! Yes, I posted too quickly and didn't think
about this enough. I guess mostly because I feel I'm currently not
very responsive (time constraints) and don't want to hold up stuff
unnecessarily.

So yes, I still really like the idea of making the existing commands
smart, so that they can detect if they have been called with a group
index or not. Un-deprecating them is a good idea.

Cheers
Marcus

_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev