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

Extend MIDI auto connect #1023

Merged
merged 12 commits into from
Sep 11, 2022
Merged

Conversation

pedrolcl
Copy link
Contributor

@pedrolcl pedrolcl commented Jan 11, 2022

  • Add midi auto connect support for winmidi
  • Make minor adjustments for alsa_seq driver
  • Populate the setting midi.alsa.device with available devices

For full context, please see discussion in #414.

Before this patch, all external devices were connected to the last MIDI IN port, applying the same mapping to all events.
After the patch, the external devices are connected in order to each MIDI IN port. When there are more devices than MIDI IN ports, the connections are distributed among all MIDI IN ports, sequentially.

For instance, when synth.midi-channels is 32, then 2 MIDI IN ports are created. If the user has 3 controllers: device1, device2 and device3 and nothing more, then autoconnect will do:

MIDI IN port 1 connected to device1 and device3.
MIDI IN port 2 connected to device2.

Context: ticket FluidSynth#414

The alsa_seq driver creates multiple ports (N/16) for
N="synth.midi-channels", each mapped to synth channels: port*16+channel.
This is already implemented.

When the "midi.autoconnect" is specified, instead of subscribing only
the last created port, all ports are sequencially subscribed to each
available external MIDI inputs. When the last port has been subscribed,
the next subscription starts again from port 0.
@jjceresa
Copy link
Collaborator

I'm not familiar with alsa_seq or alsa_raw driver, so would you please teach how the auto connect future is intended to behave from a musician user point of view. Assuming the following setup:

1)The synth system having 2 physical MIDI inputs (i.e device 1, device 2) feeding a single fluid synth instance with 16 MIDI channels(0 to 15).

2)The musician connect 2 MIDI keyboards , both transmitting on MIDI channel 0.

With "auto connect" set to on, does the alsa_seq (or alsa_raw) driver redirect the 2 MIDI keyboards to the fluid synth's MIDI channel 0 input ?.

What happens with auto connect set to off ?

Is there really 2 distinct alsa MIDI driver in fluidsynth library ?. Sorry for my ignorance.

@pedrolcl
Copy link
Contributor Author

I'm not familiar with alsa_seq or alsa_raw driver, so would you please teach how the auto connect future is intended to behave from a musician user point of view. Assuming the following setup:

1)The synth system having 2 physical MIDI inputs (i.e device 1, device 2) feeding a single fluid synth instance with 16 MIDI channels(0 to 15).

ALSA sequencer provides virtual MIDI ports, each one supports a maximum of 16 channels. When the user sets "synth.midi-channels" to a value greater than 16, then additional virtual ports are created by the alsa_seq driver.

If the value of synth.midi-channels" is the default (16), then only a single MIDI IN port is created by the alsa_seq driver. The user may connect both device1 and device2 to the single port, and ALSA merges both MIDI streams into one. No mapping will be performed (the synth receives the events exactly as they have been generated).

When the value of synth.midi-channels" is for instance 32, the alsa_seq driver creates two MIDI ports. The user may connect device1 to the first port and device2 to the second port, by hand (using the console command aconnect, or one of the graphic connection tools available). The events sent by device1 are received by the synth on channels 1-16. The events sent by device2 are received by the synth on channels 17-32. The mappings are associated to the virtual MIDI ports.

This was already implemented in the alsa_seq driver, and nothing changed on this area with my patch.

2)The musician connect 2 MIDI keyboards , both transmitting on MIDI channel 0.

With "auto connect" set to on, does the alsa_seq (or alsa_raw) driver redirect the 2 MIDI keyboards to the fluid synth's MIDI channel 0 input ?.

Before my patch, all external devices were connected to the last MIDI IN port, applying the same mapping to all events.
After the patch, the external devices are connected in order to each MIDI IN port. When there are more devices than MIDI IN ports, the connections are distributed among all MIDI IN ports, sequentially.

For instance, when synth.midi-channels is 32, then 2 MIDI IN ports are created. If the user has 3 controllers: device1, device2 and device3 and nothing more, then autoconnect will do:

  • MIDI IN port 1 connected to device1 and device3.
  • MIDI IN port 2 connected to device2.

What happens with auto connect set to off ?

The user needs to use an external connection tool. He may connect every controller to the first MID IN port, or not. The mappings are associated to the MIDI IN ports anyway. It doesn't matter if he uses autoconnect or does the connections by hand in relation to the MIDI channel mappings.

Is there really 2 distinct alsa MIDI driver in fluidsynth library ?. Sorry for my ignorance.

Yes. The alsa raw MIDI driver in Linux is like the winmidi driver. It has no virtual MIDI ports, and the driver has to open the MIDI devices, read events from them, and close the devices when finished. The alsa sequencer MIDI driver is totally different, more like the CoreMIDI driver in macOS providing virtual MIDI ports, and virtual cables (MIDI routing).

@jjceresa
Copy link
Collaborator

Thanks Pedro, very nice explanations about alsa_seq (virtual ports) and alsa_raw behaviour !.

if the value of synth.midi-channels" is the default (16), then only a single MIDI IN port is created by the alsa_seq driver. The user may connect both device1 and device2 to the single port, and ALSA merges both MIDI streams into one.

Ok, that means that device1 MIDI streams on Channel x are merged with device2 MIDI streams on Channel x (with x in the range: 1..16). If 2 musicians play at the same time on both devices, this could lead in MIDI streams conflicts. Does I understand well ?

@pedrolcl
Copy link
Contributor Author

Ok, that means that device1 MIDI streams on Channel x are merged with device2 MIDI streams on Channel x (with x in the range: 1..16). If 2 musicians play at the same time on both devices, this could lead in MIDI streams conflicts. Does I understand well ?

Of course, that is something expected. And that is why each musician should use different MIDI channels in that scenario. There are 16 MIDI channels, and each MIDI controller should allow to select the base channel. And to provide more flexibility, the Fluidsynth synthesizer has a maximum of 256 MIDI channels, that you may request using the setting "synth.midi-channels" that you choose to ignore.

What are you telling me? That every MIDI merge box in the market is wrong because doesn't solve this problem that comes from the very deep guts of the MIDI 1.0 specification?

@jjceresa
Copy link
Collaborator

As already said I am not familiar at all with ALSA stuff. I have found your explanations very useful. I just needed your confirmation about virtual ports. Thanks.

This is a preliminary revision of the ALSA Raw MIDI driver, implementing
the enumeration of Raw MIDI devices. The list is retrieved once, before
the actual driver is created, and it is stored as options in the setting
"midi.alsa.device".
@sonarcloud
Copy link

sonarcloud bot commented Jan 13, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

10.6% 10.6% Coverage
0.0% 0.0% Duplication

@lgtm-com
Copy link

lgtm-com bot commented Jan 13, 2022

This pull request introduces 1 alert when merging 906ecb6 into 015c6af - view on LGTM.com

new alerts:

  • 1 for Call to alloca in a loop

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 really hoped that some more people would comment on those changes. Anyway, fluidsynth 2.3.0 is about to come and I would really like to have those change in. I need to make a decision which way to go forward. In the past days I had a look at them again and I have to admit that the mergebox setting suggested by @jjceresa did not convince me. Therefore, I will apply Pedro's proposal with this PR. If need be, we can think about any mergebox-like settings later. But for the moment, Pedro's suggestion seems to be more straightforward.

There are a few issues in your PR though. E.g. calling alloca in a loop. I will take care of them. Anotherone below.

src/drivers/fluid_alsa.c Outdated Show resolved Hide resolved
@derselbst derselbst marked this pull request as ready for review September 6, 2022 13:00
@pedrolcl
Copy link
Contributor Author

pedrolcl commented Sep 6, 2022

I'm afraid I don't have time right now to work on this, sorry.

@sonarcloud
Copy link

sonarcloud bot commented Sep 8, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

12.3% 12.3% Coverage
0.0% 0.0% Duplication

@derselbst derselbst changed the title extended auto connect #414 Extend MIDI auto connect Sep 8, 2022
@derselbst derselbst added this to the 2.3 milestone Sep 8, 2022
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.

Should be good now. Thanks!

@derselbst derselbst merged commit 8182ace into FluidSynth:master Sep 11, 2022
@pedrolcl pedrolcl deleted the extended_autoconnect branch September 11, 2022 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants