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

Add autoconnect to winmidi driver #1031

Closed
wants to merge 7 commits into from
Closed

Add autoconnect to winmidi driver #1031

wants to merge 7 commits into from

Conversation

jjceresa
Copy link
Collaborator

This PR is an alternative to commit d031d4b of PR #1023

When midi.autoconnect is set to true it works as commit d031d4b of PR #1023.
When midi.autoconnect is false it works as actual.

jjceresa added 4 commits January 15, 2022 11:44
It allows to open all MIDI in devices according
to MIDI channels limit set by synth.midi-channels setting.
-update fluidsettings.xml documentation.
Copy link
Member

@derselbst derselbst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to admit it's hard for me to comprehend your different views / use-cases and understand the differences. Which makes it hard to make a decission here. AFAIU, your change keeps backward compatibility with your original use-case?

It would make things easier if @pedrolcl could comment on this proposal as well.

@@ -469,6 +494,8 @@ new_fluid_winmidi_driver(fluid_settings_t *settings,
MMRESULT res;
int i, j;
int max_devices; /* maximum number of devices to handle */
int autoconnect = 0;
int synth_midi_channels = 16;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this to be initialized to magic 16. Pls. either unconditionally use fluid_settings_getint(settings, "synth.midi-channels") or explicitly call fluid_settings_getint_default().

@pedrolcl
Copy link
Contributor

I have to admit it's hard for me to comprehend your different views / use-cases and understand the differences. Which makes it hard to make a decission here. AFAIU, your change keeps backward compatibility with your original use-case?

It would make things easier if @pedrolcl could comment on this proposal as well.

Well, I've explained in #414 (comment) why I think that "The synth.midi-channels winmidi independence of #677 was an important design choice." creates a nasty bug. For instance: https://lists.nongnu.org/archive/html/fluid-dev/2021-11/msg00000.html . I am not sure if @es20490446e is using Windows or something else, but his problem is exactly the bug in winmidi that I tried to fix in #1023, no matter if autoconnect is enabled or not.

In this comment #414 (comment) the argumentation is that MIDI channels collision is also a bug, but in my view it is a problem of the MIDI protocol, which has been affecting every MIDI installation for the last 30 years if the users don't understand MIDI 1.0 and its limitations. The solution proposed for the winmidi bug, that users should use the midi router of the command line fluidsynth program to map the incoming event channels by hand, is preposterous, when there is a much more easy, universal and sensible solution for the user: to increase the value for the setting 'synth.midi-channels', which every MIDI driver should respect, whether autoconnect is enabled or disabled.

@es20490446e
Copy link
Contributor

I'm on Linux.

* for this setting is 16, in which case the second MIDI device will not be hear
* on the fluid synth instance.
* a) You have to rise the setting synth.midi-channels to accept the number
* of MIDI channels provided by the MIDI driver.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are saying that your driver policy rules, and the user should simply obey it and set the value of "synth.midi-channels" accordingly.

IMO, the user setting of "synth.midi-channels" should be prevalent, and all the drivers should limit their channels mapping to this value, like does the ALSA sequencer driver.

@pedrolcl

This comment has been minimized.

@derselbst
Copy link
Member

Well, I've explained in #414 (comment) why I think that "The synth.midi-channels winmidi independence of #677 was an important design choice." creates a nasty bug

You have explained, that some channels will be ignored by the synth, yes. Might be not nice, but I wouldn't call it nasty bug either, as it can be fixed by simply increasing synth.midi-channels.

I also understand JJC's point of view, that a hardcoded modulo operation of the channel_map is a bad idea.

From what I see, the ALSA driver limits it's destination ports, but it doesn't do a modulo operation either.

For instance: https://lists.nongnu.org/archive/html/fluid-dev/2021-11/msg00000.html .

There already was an issue #999. It's related to Systemd, not MIDI.

My proposal to handle the "insufficient number of midi channels" situation would be to either print a warning, or fail with an error during driver creation.

* mapping restarts with the channels 0-15.
* This makes all MIDI devices audible on the synth instance. However with more than
* 1 MIDI devices and the default value of synth.midi-channels of 16 then MIDI
* channels conflicts are possible if MIDI devices transmit on the same MIDI channel.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think of this scenario: The user connects two MIDI controllers to the MIDI IN ports of a MIDI merge box, and the MIDI OUT of the merge box to the input of Fluidsynth (or any other synthesizer). Then the user plays both controllers in the same MIDI channel. This may produce event collisions, of course, because that is how MIDI works. Each MIDI controller should use a different MIDI channel.

@pedrolcl
Copy link
Contributor

You have explained, that some channels will be ignored by the synth, yes. Might be not nice, but I wouldn't call it nasty bug either, as it can be fixed by simply increasing synth.midi-channels.

The user will only observe that fluidsynth is ignoring the events received from some MIDI controllers, and he won't know what to do about it. Your proposal is to return a warning message, or fail to start the driver. Well, that's better. Somewhat rude, though.

Let's summarize the problem with an example, including a simple (but common) use case.

The user runs Windows + FS with the default settings. He connects his USB MIDI keyboard, which uses the output MIDI channel 1 to the computer, and changes the setting midi.winmidi.device to match his device id. He plays the piano keys and FS produces sound. Now, his friend brings another USB MIDI keyboard, adjusting the output MIDI channel to 2, and connects it to the same computer. The user changes the setting midi.winmidi.device again to be "0;1" (assuming that this is the correct setting value) restarting the synth. Now, playing both MIDI controllers at once only the first one produces sound. And there is nothing explaining why. The channel mappings applied by winmidi are not exposed at all, neither are evident or expected. Why should the user know that increasing the number of MIDI channels of the synth to 32 solves his problem?

Because the typical use case is that each MIDI controller sends events on one MIDI channel only. Using the default value of "synth.midi-channels" is enough for 16 MIDI controllers playing together with the same FS instance, using each one a different MIDI program. There is no need for channel mappings in this use case. Only basic knowledge of the MIDI standards.

@derselbst
Copy link
Member

Why should the user know that increasing the number of MIDI channels of the synth to 32 solves his problem?

Ok, I get your point. This makes me think that we should do the channel modulo wrapping as Pedro suggested. And I would also tend to say that a possible risk of "MIDI channels collision" is an inherent problem of the MIDI standard, or better of the two friends playing with two instruments on the same MIDI channel.

@jjceresa
Copy link
Collaborator Author

AFAIU, your change keeps backward compatibility with your original use-case?

Right. The original use case is to have a driver behaviour that makes a straightforward aggregation of the MIDI in devices on predictible separate MIDI channels to ensure no MIDI channels conflicts. I will explain why in next comments.

Of course having an "autoconnect" behaviour for the reason explained by Pedro is nice.
Obviously, "autoconnect" needs to take account of synth.midi-channels and magically map any devices MIDI channel confined to synth.midi-channels.

I don't like this to be initialized to magic 16.

Oh yes. That will be fixed

@jjceresa
Copy link
Collaborator Author

You leave a comment in the source code, but this won't help the users.

The comments are for developers not for end-users.

Then the user plays both controllers in the same MIDI channel. This may produce event collisions, of course, because that is how MIDI works. Each MIDI controller should use a different MIDI channel.

In a perfect world each MIDI controller should use a different MIDI channel but there case were this is not true. Please see my comment below.

@jjceresa
Copy link
Collaborator Author

One of my friend user is using multiples MIDI controllers (2 keyboards), and several custom MIDI cc controllers).
One constraint is that the custom MIDI cc transmit all on the same MIDI channel (and this cannot be changed).
At the output he is often using 2 synths (1 fluid synth + hardware synth).

To map any MIDI input controllers (Keyboard layers+ CCs) to any synths (fluid synth or/and hardware synths) this user prefers to use a central matrix (MIDI channel mapping/key split/ and routing) situated between the MIDI driver and the synths.
With the help of the fluidsynth library architecture it is possible to have a MIDI driver connected to a custom application and the output of this application connected to fluid synth + other synths.
The application allows a direct and fast access to the central matrix which is opaque to the human user
through a user interface.
Also the user never need to enter in MIDI controllers menu to change the MIDI channels setup.

In this case that means that the MIDI driver behaviour must be constant and predicitible. The MIDI driver should not do a magic device channel mapping merging dependent of synth.midi-channels. This is the reason of the actual winmidi driver behaviour.

@jjceresa
Copy link
Collaborator Author

I have added a "midi.winmidi.mergebox" setting toggle setting.
With this setting set to 1 (the default) the driver behaves like d031d4b of PR #1023.
With this setting set to 0 the driver behaves like actual.

@pedrolcl
Copy link
Contributor

One of my friend user is using multiples MIDI controllers (2 keyboards), and several custom MIDI cc controllers). One constraint is that the custom MIDI cc transmit all on the same MIDI channel (and this cannot be changed). At the output he is often using 2 synths (1 fluid synth + hardware synth).

To map any MIDI input controllers (Keyboard layers+ CCs) to any synths (fluid synth or/and hardware synths) this user prefers to use a central matrix (MIDI channel mapping/key split/ and routing) situated between the MIDI driver and the synths. With the help of the fluidsynth library architecture it is possible to have a MIDI driver connected to a custom application and the output of this application connected to fluid synth + other synths. The application allows a direct and fast access to the central matrix which is opaque to the human user through a user interface. Also the user never need to enter in MIDI controllers menu to change the MIDI channels setup.

In this case that means that the MIDI driver behaviour must be constant and predicitible. The MIDI driver should not do a magic device channel mapping merging dependent of synth.midi-channels. This is the reason of the actual winmidi driver behaviour.

For the particular case of a custom application used only by your friend, you can provide him with a custom FS library, fine tuned as you see fit. Instead, you are forcing everyone to follow your particular policy. That is unfair. I am talking here as the packager of Qsynth for Windows, not only as a FS user.

@pedrolcl
Copy link
Contributor

I have added a "midi.winmidi.mergebox" setting toggle setting. With this setting set to 1 (the default) the driver behaves like d031d4b of PR #1023. With this setting set to 0 the driver behaves like actual.

So, you think that it is fine to add and obey a new user setting in the winmidi driver, but you insist on ignoring in some cases the setting "synth.midi-channels"?

Why not change "synth.midi-channels" to the maximum value of 256 channels in your friend's custom application instead, and always fulfill this setting?

@es20490446e
Copy link
Contributor

I would do the least surprising thing for the most general case. While not enforcing any particular policy, but making the most common the default.

@jjceresa
Copy link
Collaborator Author

you are forcing everyone to follow your particular policy....That is unfair. I am talking here as the packager of Qsynth for Windows.
So, you think that it is fine to add and obey a new user setting in the winmidi driver...

With the help of midi.winmidi.mergebox setting everyone aren't forced to follow my particular policy. The default value of this setting is 1 and in this case the driver behave as a merging box that is the policy of PR #1023 who is making a silent magic
MIDI channel mapping (for ALSA seq also if understood well).

Note that the MIDI driver merging box policy nature makes the use of fluid router or custom router impossible because the MIDI channel mapping at the router input isn't easily predictable.

With the midi.winmidi.mergebox setting presence it is now more clear how the driver behaves.

Why not change "synth.midi-channels" to the maximum value of 256 channels in your friend's custom application instead, and always fulfill this setting?

This excessive and sensible solution is a workaround, because if "synth.midi-channels" is accidentally changed for any reason than MIDI channel mapping at the MIDI driver output becomes unpredictable again. The MIDI driver output is connected to MIDI input of this custom application. So the important point is to keep all the MIDI in configuration routing independent of the synths MIDI channels capabilities. This user use a fluid synth + others synth.

Please, as the packager of Qsynth for Windows why midi.winmidi.mergebox setting presence is a problem ?

Add midi.winmidi.mergebox setting documentation in fluidsettings.xml
@derselbst
Copy link
Member

Superseded by #1023

@derselbst derselbst closed this Sep 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants