Implement a reliable port selection API

[keinstein]

The current RtMidi implementation has a race condition that may be very annoying in certain cirumstances.

Steps to reproduce the problem:

1. add several virtual and/or usb devices
2. Start a RtMidi application with interactive port selection
3. After the Application presented the port list to the user, remove the removable device in the list from the system (delete or unplug it).
4. Select the a device that is below the unplugged device in the list.
5. RtMidi will fail or open the wrong device.

The current patch set contains a suggestion for a reliable API.
The main problem that shall be addressed is that the current API does not store port descriptors together with the meta data (port names/port numbers). Thus, both can get out of sync.

The patches are based both on master and winks include:

• Winks patches as well as reverting them to avoid problems synchronizing with the master branch.
• some code reorganization (extract common API into a separate class)
• Add a port descriptor class (With a more flexible port naming interface).
• Add port descriptor API: getPortList, openPort(PortDescriptor &, const std::string&), getPortDescriptor()
• Add Example programs.
• Add an automatic test case for all backends that support virtual devices. This has been inspired by the automatic loopback test case of the node-midi project.

I came across the following questions:
• What's the purpose of class RtMidi? Is it used directly?
• RtMidiIn and RtMidiOut serve as API containers. Why do you use virtual inline functions, here? I'd suggest to remove the virtual classifier from the member functions and declare them only in RtMidi if they are used both by RtMidiIn and RtMidiOut. This would render CommonMidiApi useless and I would reintegrate it into MidiApi. Otherwise the inline functions should be marked extern to avoid linker errors.
• There is no function call ot set or get the current API object from RtMidi(In|Out). This and the fact that the implementation classes Midi(In|Out)(Alsa|Jack|WinMM|WinKS|Core|Dummy) are not documented separately suggest that these classes are private to the RtMidi library. Is that right? In this case I'd suggest to remove these classes from RtMidi.h. Otherwise I'd suggest to add two functions to set and get the API object that is used by the RtMidi(In|Out).
• Maybe… probably… I might add a container API that allows to collect several APIs into one common API.

[keinstein]

If you would consider to merge the autotools patch #22 (which I recommend) I'd suggest that you merge that patch first. It is easier for me to adopt my changes to Sebastian's patch set than vice versa.

[keinstein]

Ok. I have merged the last chanegs on the master branch. @garyscavone some remark to your memory leak fix. CFRelease should be called before the error check as error(...) may not return, but the application may continue to run.

[keinstein]

I have integrated all changes and bug fixes so far in this fork. I have reorganized the code to better match upstream.

Some additional thoughts behind the structure:
The code uses C++ features to avoid code duplication within one interface. The interfaces have been split into three parts:

• A Sequencer class which does basic client/server or API communication.
• A port descriptor class which is intended to be minimial in size while allowing reliable access for the lifetime of the addressed MIDI endpoint (Hardwar device or Software).
• An API data class which extends the port descriptor and provides all other port and connection related data and functions.

In order to not clobber the global namespace more than necessary, everything is encapsulated in a namespace “rtmidi”.

The API itself is also kept minimal:

• getPortCount has its counterpart in getPortList
• openPort has been complemented by an implementation that asks the port descriptor to generate a backend object
• getPortDescriptor allows to get back the port descriptor from an open virtual port. This is used in the loopback test case to do real MIDI communication (except on Windows that does not support virtual devices).
• The new function hasVirtualDevices has been added to query the existence of virtual devices so that the loopback device can shut down gracefully when the backend has no virtual devices.

getPortName has been exteded to support different needs of users and programmers. All backends except Windows are organized in two levels: devices/clients and ports. The naming schemes are not consistent. Even not in within one backend. This will be always the case as the naming is left to the device driver or MIDI client. So sometimes the port name contains the client name and sometimes not. Leaving one out, may lead unclear descriptions, while keeping everything leads sometimes to overly long strings, where the important part is hidden because of space or other limitations. This is part is related to #100.

Some bugs have been fixed a long time ago, but I had no time to port them to upstream and back.

Most of the changes in the structure of the old functions are simply copy & paste operations to a place that looked useful to avoid code duplications or produced clashes with the needs of the new API.

Some further simplifications have been intentionally left out to in order to keep some fixed points to align code diffs. Unfortunately, git diff seems to ignore these points. At least it mixes the different APIs in the diffs.

The build system has been developed following the Autotools include paradigm. This has been done in parallel with Mutabor development.

If someone is willing to play around with these ideas I'm open for discussions. Design decisions have been the result of an evolutionary process and are not carved in stone. But they will be much deeper when I cleaned up the code and removed some further duplications.

I could also offer to split up the patch into several parts for reviews (Namespace, getPortList/openPort for each backend separately, other API functions for each backend separately). But I currently don't have the time to keep this bunch of patches in sync with upstream for several years. This time would be better used to update the fork and to fix bugs.

[radarsat1]

Hi,

Thanks for all that work. What's the advantage of the PortDescriptor scheme over just adding openPort(std::string name, ...)? Wouldn't that be much simpler? You mention some O(n) consequences for some APIs, but we're talking about very small "n" here, and as far as I can tell WinMM is the only API that does not have a name-based port accessor function.

[keinstein]

There are DAWs that use JACK for its internal routing backend. In fact JACK allows you to store JACK sessions, so that you even don't need a real DAW in order to achieve that. Theoretically you can build up a DAW from hundreds of independent small JACK clients. The same is possible with ALSA. So in the end $n$ may exceed 100, leading to 1000 000 library calls with several string copy operations.

There are different reasons:
• Port names are not always unique (exception: JACK)
• Ports can be renamed, which introduces a race condition
• ALSA has no name based subscription function: https://alsa-project.org/alsa-doc/alsa-lib/group___seq_subscribe.html
• Translation between port names and port descriptors takes time imposing a gap for race conditions, which can not be closed by locking mechanisms
• At least JACK knows the difference between a port name and its UUID
• A clearly name based API can be implemented using std::string as base class for port descriptors but a numeric oriented API has a difference between a name and a description of the numbers.
• You never know which type of handles other backends will use on future systems.

I'm thinking about a feature to save and restore port descriptors. But this would be based on numeric ids and/or UUIDs, if possible, so that it allows mostly automatic reconfiguration after a system restart without the problems that arise from reordering devices/modules/plugins/filters during restart.

On a linux system you could open three shell windows. Try the following commands (each in a different window:)

# 1st window:
timidity -iA -Os

# 2nd window:
timidity -iA -Os

# 3rd window:
aconnect -i -o -l

You will get something like:

client 0: 'System' [type=kernel]
    0 'Timer           '
    1 'Announce        '
        Connecting To: 130:0, 131:0
client 14: 'Midi Through' [type=kernel]
    0 'Midi Through Port-0'
        Connecting To: 131:0[real:2]
        Connected From: 131:0
client 128: 'TiMidity' [type=user,pid=17396]
    0 'TiMidity port 0 '
        Connected From: 131:0
    1 'TiMidity port 1 '
        Connected From: 131:0
    2 'TiMidity port 2 '
        Connected From: 131:0
    3 'TiMidity port 3 '
        Connected From: 131:0
client 129: 'TiMidity' [type=user,pid=17473]
    0 'TiMidity port 0 '
        Connected From: 131:0
    1 'TiMidity port 1 '
        Connected From: 131:0
    2 'TiMidity port 2 '
        Connected From: 131:0
    3 'TiMidity port 3 '
        Connected From: 131:0

As you can see there are two clients announced as “TiMidity”. Each of them has a port named “TiMidity port 0” so writing “TiMidity port 0”. They can only be differenciated by the port descriptor 128:0 and 129:1. Yes, you can serialize the port descriptor into a string representation. But both represent different information. And users dislike the three-fold redundency when you put everything into one string just to make sure, that the port connection is stable.

What I forgot in my last comment is the ALL_API mode, in which case the calling software does not care about the available APIs and can use all ports from all supported interfaces with one library call. In fact the portDescriptor API can easily be extended to provide the necessary information for tree based presentation of MIDI ports as it is commonly used in GUIs like WINE, qjackctl or in the above example of aconnect. Actually, the tree presentation is the best compromise between screen usage and completeness of port names.

[keinstein]

In addition to the last answer danomatika/ofxMidi#21 mentions port name changes due to port renumbering.

My approach is simply: Don't try to be more clever than the underlying API. Otherwise you will have to deal with many unexpected side effects.