Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unplug #260

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Unplug #260

wants to merge 2 commits into from

Conversation

folkertvanheusden
Copy link

Throw an error back to the calling python script when the (virtual/physical-) MIDI device got unplugged.

@garyscavone
Copy link
Contributor

I haven't looked at every API, but I don't think the lock is necessary because a thread will only access the queue if a callback is set, in which case getMessage() cannot be used.

As for handling an unplug, I think there needs to be a careful analysis of how that may be done for each API and then an implementation designed that supports such. For ALSA, a notification comes through a "midi" message. But in CoreMidi, we could set a notification callback that would inform us of a setup change. There is also the question of what should happen if a device currently in use is unplugged. Ideally, the port would be closed and the user informed of the problem.

There are other issues to deal with as well, such as a better device enumeration / identification scheme and a better error handling scheme. If I only had time ...

@insolace
Copy link
Contributor

I'm not sure Windows provides any useful means of knowing when a device is unplugged. Our approach has been to store all of the MIDI port numbers/names in a table and then regularly polling them for changes. If there's a change, then we call an update function.

@keinstein
Copy link

The queue is accessed as a ring buffer. That means the buffer is intended to be accessed by exactly one thread in parallel from the user code as well by the internal thread. The responsibilities are spread between the threads. The realtime thread writes data and increases the end pointer while the user thread increases the start pointer. Race conditions can occur when one pointer is close to the other but this means that he user thread reads beyond end or the realtime thread recognises a buffer overrun. So the worst thing that can happen is that the realtime thread is informed too late about free memory. This is a race condition that has no logical solution.

Practically the compiler optimisations or the processor cache may cause trouble. In such a case a memory barrier must be installed in order to make sure that all data is written before it is read.

If you need several user threads to access the queue you can add locking mechanisms there.

Furthermore, in realtime functions probably spinlocks are a better choice as they return faster.

@sagamusix
Copy link
Contributor

sagamusix commented Sep 10, 2023

I'm not sure Windows provides any useful means of knowing when a device is unplugged

Indeed there isn't. What I'm doing in my own application (but would be difficult to add to RtMidi as-is) is to listen to WM_DEVICECHANGE messages, and if the event type is DBT_DEVNODES_CHANGED, I call midiInGetNumDevs(), which is enough to provoke the MIDI driver to send a MIM_CLOSE message in the MIDI callback, which can then be acted upon.

I would guess the new Windows UWP API supported in RtMidi 6.0 finally fixes this shortcoming, but I haven't tested it yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants