macOS multithreading fix/note

[tschiemer]

In multithreaded environments on macOS port enumeration etc might not work properly due to the system run loops not being called.

This is not directly an RtMidi bug, but for a simple to use library this might be (at least) mentioned and possibly an equally simple solution presented.

I don't like introducing system specific functions (RtMidi_multithreadRunLoop()), but this is the most simple approach as the individual API implementations are not exposed, which I think should be done..

Note that notifyProc will always be called on the run loop which was current when MIDIClientCreate was first called.

https://developer.apple.com/documentation/coremidi/1495360-midiclientcreate?language=objc
https://ios.developreference.com/article/22075958/MidiClientCreate+notifyproc+not+getting+called

[radarsat1]

Is there any reason it would be bad to just call CFRunLoopRunInMode from the RtMidiIn constructor?

[tschiemer]

Although I'm not 100% certain (not too deep an insight here), I would think it would have to be run recurringly to process events, otherwise they'd not be delivered to the RtMidi subscriber - so a single call in the constructor would not lead to the desired result.

https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Multithreading/RunLoopManagement/RunLoopManagement.html
https://developer.apple.com/documentation/corefoundation/1541988-cfrunloopruninmode?language=objc

But please feel free to test ;)

[radarsat1]

Oh I see, it's more like a polling function, I didn't read your example properly. But, from your example, it seems like something really specific to Mac (sorry I don't have one for testing), and you are literally checking if you are on OS X and calling a special function:

 while(1){
    #ifdef __APPLE__
    RtMidi_multithreadRunLoop();
    #endif
  }

But if that's the case, if this is something you know you have to do only on OS X, and you have to go out of your way like this to do it, I don't see why it's RtMidi's responsibility? You could just as easily do,

 while(1){
    #ifdef __APPLE__
    CFRunLoopRunInMode( kCFRunLoopDefaultMode, 0, false );
    #endif
  }

I don't really see what wrapping this call really adds, as it doesn't provide any OS transparency, which is the whole point of RtMidi.

On the other hand if RtMidi needed a general polling function for the main loop,

 while(1){
    RtMidi::pollMessages();
  }

it might make some sense, but then I'm not sure of the benefit of using threading here.

[tschiemer]

Not sure what you mean with OS transparency as the RtMidi interface obscures any implementation details not providing access to system objects..

but hmm, you're right that the declaration as such is suboptimal; I guess a declaration such as follows would have been more sensible in the first place such that it can be used without conditionals.

#if defined(__APPLE__)

void RtMidi_multithreadRunLoop();

// or well, if ya want, but might also require some system includes
#define RtMidi_multithreadRunLoop() CFRunLoopRunInMode( kCFRunLoopDefaultMode, 0, false )

#else

#define RtMidi_multithreadRunLoop()

#endif /* __MACOSX_CORE__ */

As the CoreMidi system driver is obscured by the generic interface (otherwise I would have added it there directly such that it's a static CoreMidi driver specific method, ex MidiInCore::multithreadRunLoop() ) I chose to add a generic function for this.

Why RtMidi should implement this?
Well, at least it should be clearly stated that in multithreaded environments on macOS said call might be needed for RtMidi to run as expected.
I would maybe more consider it a convenience function provided by RtMidi; for developers that just see or want to use the RtMidi interface without having to rely on some fancy system call, possibly having to include some additional headers etc

As long as the solution for this dormant problem is clearly pointed out, personally I don't care much wether there is a direct implementation for this by RtMidi.
The benefit of there being a specific function is that you also have it somewhat documented in code instead of only on github or readme files, so chances of being aware are greater and times finding a solution shorter, I guess - ? :/
(but really don't care ;)

*edit:

So I don't know, I can remove the code but leave a note or change the readme note accordingly. Would that be a more agreeable solution?

[garyscavone]

I generally agree with @radarsat1 on this. If this is still an issue, I would say that it would be nice to add some information about it in the doxygen documentation (probably in API Notes section?). This PR has conflicts so it would need to be resolved and updated.