celtera/libremidi

(Windows MM) Observer and API enhancement suggestion

Closed this issue · 4 comments

Hi,

I came across an issue with the observer_winmm. The port name the callbacks return are not identical with the port name returned by midi_in_winmm::get_port_name and midi_out_winmm::get_port_name. And as the index returned by those callbacks is always 0, this whole thing is, sorry to say, pretty useless.

Looking at the code, the reason is obvious, get_port_name adds a trailing integer to avoid port name clashing (which I believe is a good thing) but that is not implemented in the observer.

I had a look on how to fix that an in a "simple and clean way" and found out that there are quite some code redundancy between midi_in_winmm::get_port_name, midi_out_winmm::get_port_name and observer_winmm::get_port_list.

I believe there is an easy way to largely simplify/clean-up the whole thing but that would imply an API change. I briefly had a look at the other implementations (Alsa, CoreAudio,...) I believe that it would also benefit them and would even make it possible to implement the observer in a generic way for almost all implementations.

I suggest to make the following functions static:
midi_in_XXX::get_port_count
midi_in_XXX::get_port_name
midi_out_XXX::get_port_count
midi_out_XXX::get_port_name
Making them static would make the observer straight forward to implement and would remove code redundancy.
And, in the end, why do we need to instantiate a midi_in and a midi_out object to enumerate ports ?

Unfortunately as all class inherit from midi_api, that would mean having both a static and non-static version of these functions. If only C++ had virtual static functions 😄

Cheers

Renaud

Hi !

Most of the design is just the historic rtmidi API, and I agree that it really needs an overhaul aha.

I think however that them not being static is actually meaningful: in some APIs, such as JACK and ALSA (and, I think, CoreMidi maybe?), you are going to create a jack object, connect to it, etc... to be able to enumerate the other MIDI devices which will make the app itself appear in the jack patchbay / ALSA device list / ..., it's not just calling static API functions on the API side and could lead to weird behaviours / numbering if the user enumerates and then creates them. Thus using the same "connection" object makes more sense - in the software I'm developing I'm having the same problem at an even larger scale, as it causes some issues that the app opens for instance two different JACK clients for audio and MIDI ; Pipewire is likely going to have the same kind of issues.

Unfortunately as all class inherit from midi_api, that would mean having both a static and non-static version of these functions. If only C++ had virtual static functions

well, there's constexpr virtual in C++20 which means that Foo{}.bar(); has as much cost as a static function if Foo::Foo and bar are constexpr.... of course for MIDI API calls that's not the case

#69
The mechanism for adding the numbers to the device names wasn't added to each path. This should PR help.

done in v4 :-) now all the metadata is in observer and has been removed from midi_in / midi_out. Also port indexes aren't used anymore, instead it's a more complete type with all the information that we have to uniquely identify a given port.