MidiOutput Bug if more than one device is open

Linux
1

Hi,

I think there is a bug in the MidiOutput on Linux.

If I have more than one MidiOutput device open and i send a message to output_a and a message to output_b, then both messages are physically received by the device 1 and not one at dev 1 and one at dev 2.

This is my test code:

int main (int argc, char* argv[])
{
    MidiOutput* output_a = MidiOutput::openDevice( 1 );
    MidiOutput* output_b = MidiOutput::openDevice( 2 );

    MidiMessage message = MidiMessage::noteOn( 1, 64, 1.0f );
    
    // delete port_a;
    // output_a = MidiOutput::openDevice( 2 );

    output_a->sendMessageNow( message );
    output_b->sendMessageNow( message );
    
    return 0;
}

If i change the device for output_a to device 2, than both messages received by dev 2! (the commented out block)


Is my code wrong or is this a bug?

Cheers

 

EDIT: I have tested the same code on Windows: works.

EDIT: The correct connections will be create ("openDevice(1)" creates a connection to dev 1 and "openDevice(2)" creates a connection to dev 2, but only dev 1 receive the events)

EDIT: If i create new devices instead open it and connect the created dev 1 and 2 manually via QJackCtl or what ever: every thing works fine. Something with the openDevice handling is may be wrong.

 

EDIT NOTE in file juce_linux_Midi.cpp : >> static_cast <MidiOutputDevice*> (internal) << are different pointers for output_a and _b.

void MidiOutput::sendMessageNow (const MidiMessage& message)
{
    static_cast <MidiOutputDevice*> (internal)->sendMessageNow (message);
}
2

There are some global ALSA midi objects on linux, but TBH I didn't write the linux midi support myself, and don't have a suitable linux machine for testing it, so am not 100% sure what's going on there..

3

Ok, thats not so good :-)

I will take a look. But i'am not sure if I'm able to find the problem -> so any linux geek help is welcome! :-)

 

All what I will figure out, I will post as edit in the first entry.

Cheers.

4

File juce_linux_Midi.cpp: If i remove the static in:

static AlsaClient::Ptr globalAlsaSequencerOut()
{
    /*static*/ AlsaClient::Ptr global (new AlsaClient (false));
    return global;
}

then i create a new device group for each openDev, but everything works.

For me is this enough for now. I go out and buy me a beer :-)
Later I will check a bit more - but I'm not familiar with ALSA.

Cheers

5

Interesting. Would welcome any feedback from other linux midi people, as this is one of the very few corners of the codebase that I'm hazy about!

6

Just to clarify this, this is not a bug - I have done some things wrong!

Excuse me for wasting your time!

7

Hi monotomy,

 

I am experiencing the same problem under Linux when trying to open two individual MIDI output devices. My code is similar to your test code, removing static works as a quickfix. Could you give me a hint on what you were doing wrong?

 

Best,

Sebastian

8

Hi monotomy and sarnold. I also have the exact same problem under Linux, and removing static also works for me as a quickfix. What’s happening here?

9

No, this is a bug in JUCE actually.

Here’s the proper fix:

--- a/pluginhost/JuceLibraryCode/modules/juce_audio_devices/native/juce_linux_Midi.cpp
+++ b/pluginhost/JuceLibraryCode/modules/juce_audio_devices/native/juce_linux_Midi.cpp
@@ -450,7 +450,7 @@ public:
             numBytes -= numSent;
             data += numSent;
 
-            snd_seq_ev_set_source (&event, 0);
+            snd_seq_ev_set_source (&event, port.portId);
             snd_seq_ev_set_subs (&event);
             snd_seq_ev_set_direct (&event);
10

Here’s another fix. When calling createNewDevice, the alsa midi code created a whole new alsa client instead of just adding a new port to the existing client.

--- a/pluginhost/JuceLibraryCode/modules/juce_audio_devices/native/juce_linux_Midi.cpp
+++ b/pluginhost/JuceLibraryCode/modules/juce_audio_devices/native/juce_linux_Midi.cpp
@@ -387,19 +387,6 @@ static AlsaPort iterateMidiDevices (const bool forInput,
     return port;
 }
 
-AlsaPort createMidiDevice (const bool forInput, const String& deviceNameToOpen)
-{
-    AlsaPort port;
-    AlsaClient::Ptr client (new AlsaClient (forInput));
-
-    if (client->get())
-    {
-        client->setName (deviceNameToOpen + (forInput ? " Input" : " Output"));
-        port.createPort (client, forInput ? "in" : "out", forInput);
-    }
-
-    return port;
-}
 
 //==============================================================================
 class MidiOutputDevice
@@ -450,7 +437,7 @@ public:
             numBytes -= numSent;
             data += numSent;
 
-            snd_seq_ev_set_source (&event, 0);
+            snd_seq_ev_set_source (&event, port.portId);
             snd_seq_ev_set_subs (&event);
             snd_seq_ev_set_direct (&event);
 
@@ -507,8 +494,11 @@ MidiOutput* MidiOutput::openDevice (int deviceIndex)
 MidiOutput* MidiOutput::createNewDevice (const String& deviceName)
 {
     MidiOutput* newDevice = nullptr;
+    AlsaPort port;
 
-    AlsaPort port (createMidiDevice (false, deviceName));
+    const AlsaClient::Ptr client (globalAlsaSequencer (false));
+
+    port.createPort (client, deviceName, false);
 
     if (port.isValid())
     {
@@ -584,8 +574,11 @@ MidiInput* MidiInput::openDevice (int deviceIndex, MidiInputCallback* callback)
 MidiInput* MidiInput::createNewDevice (const String& deviceName, MidiInputCallback* callback)
 {
     MidiInput* newDevice = nullptr;
+    AlsaPort port;
+
+    const AlsaClient::Ptr client (globalAlsaSequencer (true));
 
-    AlsaPort port (createMidiDevice (true, deviceName));
+    port.createPort (client, deviceName, true);
 
     if (port.isValid())
     {
11

Here’s a diff, for convenience:
https://github.com/kmatheussen/radium/commit/aa54e6143624c61eb1d4c63785467b20be7bfab6.patch

12

Thanks for the patches. This seems reasonable. I’ll a go at it tomorrow and see if we can merge it in.

13

Here’s yet another fix for alsa midi:
https://github.com/kmatheussen/radium/commit/24da7d24cc5085451a672bd8479ec0ebe654fdd8.patch

Without this patch, juce will call setName() every time you call openDevice, overriding the value you might have previously set calling setName().

In addition, the default client name was never set, unless you called openDevice at some point, causing the default name to be initially autocreated by ALSA.

A third thing the patch does is to name ports created by openDevice the same as the port names they are connected to, making it simpler to remember where ports where connected initially in case you connect ports manually afterwards. (this is a minor issue)

14

Hold on, the original problem was that setName changed the alsa client name instead of the alsa port name. Going to make new patch.

15

No, I was confusing the local AlsaClient::setName function with the MidiInput::setName / MidiOutput::setName functions. It seems like the author of the juce alsa midi code might have been confused about the same thing.

So the only change from the previous patch is to remove the AlsaClient::setName function, since it is only called from the constructor.

In other words, two patches.
1:
https://github.com/kmatheussen/radium/commit/24da7d24cc5085451a672bd8479ec0ebe654fdd8.patch

2:
https://github.com/kmatheussen/radium/commit/76d70bc46196739318b52071f682e209564c5a8d.patch

16

The last post was perhaps a bit confusing. Those two patches comes in addition to the first one. Totally in this thread, there’s three patches for alsa midi:

https://github.com/kmatheussen/radium/commit/aa54e6143624c61eb1d4c63785467b20be7bfab6.patch

https://github.com/kmatheussen/radium/commit/24da7d24cc5085451a672bd8479ec0ebe654fdd8.patch

https://github.com/kmatheussen/radium/commit/76d70bc46196739318b52071f682e209564c5a8d.patch

17

Another fix for alsa midi:

https://github.com/kmatheussen/radium/commit/f8ad8bd5c1753a3b9bde71c25feb4acc8cf25ab6.patch

This patch includes the alsa port name into the juce device name. A “device” in JUCE is the combination
of “client” and “port” in Alsa, so it is necessary to include the port name into the device name. If not,
the user has to guess which device to use if a client has several ports.

18

I’ll have a look at the patches. I want to completely understand what’s happening so it may take some time. Please be patient.

19

Okay, just let me know if something is unclear, and I’ll try to make the code clearer.

Patch 5:
https://github.com/kmatheussen/radium/commit/5e084309e342547e1b19f30dabc476b4af343d2a.patch

This patch lets the alsa midi input thread run SCHED_RR/0. If not the messages are likely be delayed xx milliseconds before reaching the program.

BTW, I looked at the juce::Thread::setPriority function, to see if I could use that one instead of using pthread_setschedparam directly, but Thread::setPriority seems semi-broken for posix. In windows, setting priority to 5 would cause the thread to run at normal priority (which seems correct when reading the documentation), while on osx (I presume) and linux, priority 5 will cause the thread to run at SCHED_RR/50, which is a very high realtime priority.

20

Any updates on getting this merged? The current state of alsa midi is quite broken, and these 5 small patches fixes it IMO.

  1. https://github.com/kmatheussen/radium/commit/aa54e6143624c61eb1d4c63785467b20be7bfab6.patch
  2. https://github.com/kmatheussen/radium/commit/24da7d24cc5085451a672bd8479ec0ebe654fdd8.patch
  3. https://github.com/kmatheussen/radium/commit/76d70bc46196739318b52071f682e209564c5a8d.patch
  4. https://github.com/kmatheussen/radium/commit/f8ad8bd5c1753a3b9bde71c25feb4acc8cf25ab6.patch
  5. https://github.com/kmatheussen/radium/commit/5e084309e342547e1b19f30dabc476b4af343d2a.patch