Boddlnagg / midir

Cross-platform realtime MIDI processing in Rust.
MIT License
601 stars 74 forks source link

hotplug support with CoreMIDI? #78

Open rryan opened 3 years ago

rryan commented 3 years ago

On macOS (CoreMIDI), I'm finding that I can't "hotplug" MIDI devices. When I create a MidiInput/MidiOutput and iterate the available ports, devices that I've plugged in since the program started are not available (even when I create a new MidiInput/MidiOutput after the device was plugged in).

Is this working as intended? Any chance this feature is on the roadmap?

rryan commented 3 years ago

(similarly, devices that I've unplugged while the program is running still show up as available ports)

Boddlnagg commented 3 years ago

It's actually suprising to me that this doesn't already work. We use the standard CoreMIDI enumeration APIs, e.g. MIDIGetNumberOfSources (through the coremidi crate) and I would have expected them to always reflect the current state of the system. I don't actually own a macOS device, so someone else would have to look into that ... @chris-zen (maintainer of coremidi), do you happen to know anything about this?

What is currently not supported (for any platform) is change notifications (see e.g. https://developer.apple.com/documentation/coremidi/midinotificationmessageid/kmidimsgobjectadded), but that is tracked in #35. And if I understand you correctly then that's not what you're looking for.

chris-zen commented 3 years ago

@Boddlnagg I got some PRs last year targeting several bugs for client notifications and some other things. I released them with 0.5.0. I'm wondering if updating to that last version would fix those problems. @rryan I'll try to test your specific use case when I have some time, but as @Boddlnagg says, the coremidi crate adds very few things (if nothing) to the standard CoreMIDI system calls.

Boddlnagg commented 3 years ago

Interestingly, this comment https://github.com/micdah/RtMidi.Core/issues/18#issuecomment-422573100 points to a very similar issue with rtmidi on macOS, but it somehow got resolved (on the application side) without ever identifying the actual problem ...

It would be interesting to see a minimal reproducing example and running that on different (macOS) hardware.

slajerek commented 1 year ago

Enumerating in runtime works correctly, I can connect MIDI device, then enumerate and it is available. Although, hotplugging would be really nice to have as this is a much better UX (as an event, not polling).

mockersf commented 1 year ago

hot plugging with coremidi (and with midir on macOS) works on two conditions:

chris-zen commented 1 year ago

This line in the examples of the CoreMIDI crate is key for the notifications (and hot plugging) to work properly. I guess that this is what a NSApplication does behind the scenes. This is not something that the midir or coremidi crates have to enable, but the responsibility of the application using them.

https://github.com/chris-zen/coremidi/blob/master/examples/notifications.rs#L23

valsteen commented 8 months ago

I'm writing a midi TUI and had a good fight with this

I started with this prior art https://github.com/Boddlnagg/midir/issues/86#issuecomment-991967845

Getting it working is quite intrusive so possibly, it should be a separate crate. I may give it a try at some point but don't mind if someone feels like doing it.

The core thing about the code:

cargo.toml

[dependencies]
midir = "0.9.1"
core-foundation = "0.9.4"
once_cell = "1.19.0"
coremidi = "0.8.0"

main.rs

use std::error::Error;
use std::sync::mpsc::Sender;
use std::sync::{mpsc::channel, mpsc::sync_channel, Arc, Mutex};
use std::thread;

use core_foundation::runloop::CFRunLoop;
use coremidi::{Client, Notification};
use once_cell::sync::OnceCell;

static EVENT_TX: OnceCell<Arc<Mutex<Sender<()>>>> = OnceCell::new();

fn create_notification_client(
    event_tx: Arc<Mutex<Sender<()>>>,
) -> Result<Client, String> {
    Client::new_with_notifications("YourProjectName", move |_: &Notification| {
        println!("Received device update notification");
        if let Ok(event_tx) = event_tx.lock() {
            if event_tx.send(()).is_err() {
                println!("Error while sending RefreshDevices event.");
            }
        } else {
            println!("Poisoned lock while attempting to send RefreshDevices event, skipping");
        }
    })
    .map_err(|e| e.to_string())
}

fn start_notification_loop(
    event_tx: Sender<()>,
) -> Result<Arc<Mutex<Sender<()>>>, String> {
    let event_tx = Arc::new(Mutex::new(event_tx));
    let (notify_tx, notify_rx) = sync_channel(1);

    thread::spawn({
        let event_tx = event_tx.clone();
        move || {
            // important : _client must not be dropped, or notifications won't be received
            let Ok(_client) = create_notification_client(event_tx)
            else {
                println!("Could not create notification client, leaving notifier thread");
                return;
            };

            if notify_tx.send(()).is_err() {
                println!("Unable to notify thread creation, leaving thread");
                return;
            }

            // important : for some reason calling CFRunLoop::get_current().stop() and 
            // attempting to start it again won't work, it's why it's a thread that has to be permanent

            println!("Starting CFRunLoop::run_current in device notifier thread");
            CFRunLoop::run_current();
            println!("CFRunLoop::run_current() returned, which is possibly a bug. The loop should never stop.");
        }
    });

    notify_rx.recv().map_err(|_| {
        "Failed to receive from notify_rx".to_string()
    })?;
    Ok(event_tx)
}

pub fn ensure_notification_event_loop_started(
    event_tx: &Sender<()>,
) -> Result<(), String> {
    let current_event_tx = EVENT_TX.get_or_try_init(|| {
        start_notification_loop(event_tx.clone())
    })?;
    let mut event_tx_lock = current_event_tx
        .lock()
        .map_err(|_| "Lock is poisoned".to_string())?;
    *event_tx_lock = event_tx.clone();

    event_tx
        .send(())
        .map_err(|_| "Failed to send RefreshDevices".to_string())?;
    Ok(())
}

fn main() -> Result<(), Box<dyn Error>> {
    let (devices_tx, device_rx) = channel();

    // the same function can be called again later with another channel if needed for 
    // the lifecycle of the application
    ensure_notification_event_loop_started(&devices_tx)?;

    let midi_input = midir::MidiInput::new("example")?;

    // important : midir can only be used starting from this point
    // also, if midir functions are called in parallel while the notification client is initializing, 
    // it may crash the process

    while let Ok(()) = device_rx.recv() {
        // note that you may receive multiple updates in a row while just one device was added/removed
        println!("device list was updated");
        for input_port in midi_input.ports() {
            println!("{}", midi_input.port_name(&input_port)?)
        }
    }
    Ok(())
}
raveslave commented 8 months ago

related to this: midi-device / port-changes are not updated when using WebMidi in Firefox https://bugzilla.mozilla.org/show_bug.cgi?id=1802149 (same issue on Linux)

alisomay commented 8 months ago

By the way I'm just starting to implement hot plugging to one of my existing apps, I'm developing on a mac but the actual target is Windows. Did anyone successfully implemented it in Windows?

By the way thanks for your efforts the code you have shared works really nice, I'm so happy that I didn't have to go through the research there :)

alisomay commented 8 months ago

I have now checked it in Windows, a simple polling loop retrieves the updated devices.

valsteen commented 8 months ago

I just published the crate, enjoy !

https://crates.io/crates/coremidi-hotplug-notification https://github.com/valsteen/coremidi-hotplug-notification