diff --git a/CHANGELOG.md b/CHANGELOG.md index 221be1a..72a330e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,22 @@ This change log follows the conventions of closing transmitters and receivers. - We now close any devices (and their transmitters or receivers) when the underlying CoreMIDI device disappears. +- It turns out that Java instantiates many, many instances of our + `CoreMidiDeviceProvider` class, each of which was getting added to + the listener list to be notified when the MIDI environment changed, + causing the device list to be rebuilt hundreds of times more than it + needed to be. We now work around this by only calling the listener + on the most-recently constructed instance. + +### Added + +- We now allow you to distinguish between multiple devices of the same + type by showing the user-assigned name of the device (and of the + port, if there is more than one port on the device), as discussed in + [Issue 21](https://github.com/DerekCook/CoreMidi4J/issues/21). +- If the user changes a device or port name while Java is running, the + device information is properly updated, although the device itself + retains its object-level identity and opened state. ## [1.0] - 2017-05-14 diff --git a/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiClient.java b/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiClient.java index f0edc14..1434ecb 100644 --- a/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiClient.java +++ b/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiClient.java @@ -14,8 +14,10 @@ package uk.co.xfactorylibrarians.coremidi4j; -import java.util.ArrayList; -import java.util.List; +import java.util.*; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; /** * CoreMidiClient class @@ -26,7 +28,15 @@ public class CoreMidiClient { private final int midiClientReference; - private final List notificationListeners = new ArrayList(); + private final Set notificationListeners = + Collections.newSetFromMap(new ConcurrentHashMap()); + + /** + * Keeps track of the latest {@link CoreMidiDeviceProvider} added to our listener list; this is the only one that + * we want to call when the MIDI environment changes, since we only need to update the device map once, and Java + * creates a vast number of instances of our device provider. + */ + private CoreMidiNotification mostRecentDeviceProvider = null; /** * Constructor for class @@ -77,6 +87,97 @@ public CoreMidiOutputPort outputPortCreate(final String name) throws CoreMidiExc } + /** + * Used to make sure we are only running one callback delivery loop at a time without having to serialize the process + * in a way that will block the actual CoreMidi callback. + */ + + private final AtomicBoolean runningCallbacks = new AtomicBoolean(false); + + /** + * Used to count the number of CoreMidi environment change callbacks we have received, so that if additional ones + * come in while we are delivering callback messages to our listeners, we know to start another round at the end. + */ + + private final AtomicInteger callbackCount = new AtomicInteger( 0); + + /** + * Check whether we are already in the process of delivering callbacks to our listeners; if not, start a background + * thread to do so, and at the end of that process, see if additional callbacks were attempted while it was going on. + */ + + private void deliverCallbackToListeners() { + + final int initialCallbackCount = callbackCount.get(); + + if (runningCallbacks.compareAndSet(false, true)) { + + new Thread(new Runnable() { + + @Override + public void run() { + + try { + + int currentCallbackCount = initialCallbackCount; + while ( currentCallbackCount > 0 ) { // Loop until no new callbacks occur while delivering a set. + + // Iterate through the listeners (if any) and call them to advise that the environment has changed. + final Set listeners = Collections.unmodifiableSet(new HashSet<>(notificationListeners)); + + // First notify the CoreMidiDeviceProvider object itself, so that the device map is + // updated before any other listeners, from client code, are called. + if (mostRecentDeviceProvider != null) { + + try { + + mostRecentDeviceProvider.midiSystemUpdated(); + + } catch (CoreMidiException e) { + + throw new RuntimeException("Problem delivering MIDI environment change notification to CoreMidiDeviceProvider" , e); + + } + + } + + // Finally, notify any registered client code listeners, now that the device map is properly up to date. + for ( CoreMidiNotification listener : listeners ) { + + try { + + listener.midiSystemUpdated(); + + } catch (CoreMidiException e) { + + throw new RuntimeException("Problem delivering MIDI environment change notification" , e); + + } + + } + + synchronized (CoreMidiClient.this) { + + // We have handled however many callbacks occurred before this iteration started + currentCallbackCount = callbackCount.addAndGet( -currentCallbackCount ); + + } + + } + + } finally { + + runningCallbacks.set(false); // We are terminating. + + } + + } + + }).start(); + + } + } + /** * The message callback for receiving notifications about changes in the MIDI environment from the JNI code * @@ -91,19 +192,15 @@ public void notifyCallback() throws CoreMidiException { synchronized(this) { - // Iterate through the listeners (if any) and call them to advise that the environment has changed - for ( CoreMidiNotification listener : notificationListeners ) { - - listener.midiSystemUpdated(); - - } + callbackCount.incrementAndGet(); // Record that a callback has come in. + deliverCallbackToListeners(); // Try to deliver callback notifications to our listeners. } } /** - * Adds a notification listener to the listener list maintained by this class + * Adds a notification listener to the listener set maintained by this class * * @param listener The CoreMidiNotification listener to add * @@ -111,21 +208,26 @@ public void notifyCallback() throws CoreMidiException { public void addNotificationListener(CoreMidiNotification listener) { - // Need to ensure that any CoreMidiDeviceProvider objects are at the head of the notification list, so that the device map is updated before other listeners are called - if ( listener instanceof CoreMidiDeviceProvider ) { + if ( listener != null ) { - notificationListeners.add(0,listener); + // Our CoreMidiDeviceProvider is a special case, we only want to notify a single instance of that, even though + // Java keeps creating new ones. So keep track of the most recent instance registered, do not add it to the list. + if (listener instanceof CoreMidiDeviceProvider) { - } else { + mostRecentDeviceProvider = listener; - notificationListeners.add(listener); + } else { + + notificationListeners.add(listener); + + } } } /** - * Removes a notification listener from the listener list maintained by this class + * Removes a notification listener from the listener set maintained by this class * * @param listener The CoreMidiNotification listener to remove * diff --git a/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDestination.java b/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDestination.java index c9dc76a..295150d 100644 --- a/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDestination.java +++ b/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDestination.java @@ -34,7 +34,7 @@ public class CoreMidiDestination implements MidiDevice { - private final CoreMidiDeviceInfo info; + private CoreMidiDeviceInfo info; private final AtomicBoolean isOpen; // Tracks whether we are conneted to CoreMIDI and can be used private final AtomicLong startTime; // The system time in microseconds when the port was opened private final Set receivers; @@ -69,6 +69,15 @@ public Info getDeviceInfo() { } + /** + * Changes the MIDI Info object; can only be done by this package as a result of a MIDI environment change event. + */ + void updateDeviceInfo(CoreMidiDeviceInfo info) { + + this.info = info; + + } + /** * Opens the Core MIDI Device * diff --git a/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDeviceProvider.java b/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDeviceProvider.java index d79f0d1..1851caa 100644 --- a/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDeviceProvider.java +++ b/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiDeviceProvider.java @@ -45,11 +45,15 @@ private static final class MidiProperties { * */ - private void initialise() throws CoreMidiException { + private synchronized void initialise() throws CoreMidiException { - midiProperties.client = new CoreMidiClient("Core MIDI Provider"); - midiProperties.output = midiProperties.client.outputPortCreate("Core Midi Provider Output"); - buildDeviceMap(); + if ( midiProperties.client == null ) { + + midiProperties.client = new CoreMidiClient("Core MIDI Provider"); + midiProperties.output = midiProperties.client.outputPortCreate("Core Midi Provider Output"); + buildDeviceMap(); + + } } @@ -100,10 +104,15 @@ private void buildDeviceMap() throws CoreMidiException { // Keep track of the IDs of all the devices we see devicesSeen.add(uniqueID); - // If the unique ID of the end point is not in the map then create a CoreMidiSource object and add it to the map + // If the unique ID of the end point is not in the map then create a CoreMidiSource object and add it to the map. if ( !midiProperties.deviceMap.containsKey(uniqueID) ) { - midiProperties.deviceMap.put(uniqueID,new CoreMidiSource(getMidiDeviceInfo(endPointReference))); + midiProperties.deviceMap.put(uniqueID, new CoreMidiSource(getMidiDeviceInfo(endPointReference))); + + } else { // We already know about the device, but may need to update its information (e.g. user renamed it). + + CoreMidiSource existingDevice = (CoreMidiSource) midiProperties.deviceMap.get(uniqueID); + existingDevice.updateDeviceInfo(getMidiDeviceInfo(endPointReference)); } @@ -119,11 +128,16 @@ private void buildDeviceMap() throws CoreMidiException { // Keep track of the IDs of all the devices we see devicesSeen.add(uniqueID); - // If the unique ID of the end point is not in the map then create a CoreMidiDestination object and add it to the map + // If the unique ID of the end point is not in the map then create a CoreMidiDestination object and add it to the map. if ( !midiProperties.deviceMap.containsKey(uniqueID) ) { midiProperties.deviceMap.put(uniqueID, new CoreMidiDestination(getMidiDeviceInfo(endPointReference))); + } else { // We already know about the device, but may need to update its information (e.g. user renamed it). + + CoreMidiDestination existingDevice = (CoreMidiDestination) midiProperties.deviceMap.get(uniqueID); + existingDevice.updateDeviceInfo(getMidiDeviceInfo(endPointReference)); + } } @@ -314,9 +328,9 @@ public boolean isDeviceSupported(final MidiDevice.Info info) { /** * Called when a change in the MIDI environment occurs - * + * * @throws CoreMidiException if a problem occurs rebuilding the map of available MIDI devices - * + * */ public void midiSystemUpdated() throws CoreMidiException { @@ -327,7 +341,9 @@ public void midiSystemUpdated() throws CoreMidiException { } /** - * Adds a notification listener to the listener list maintained by this class + * Adds a notification listener to the listener set maintained by this class. If it is a + * {@link CoreMidiDeviceProvider}, only keep the most recent one, since Java will create many, + * and we only want to update the device map once when the MIDI environment changes. * * @param listener The CoreMidiNotification listener to add * diff --git a/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiSource.java b/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiSource.java index 6e704b5..ecd6658 100644 --- a/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiSource.java +++ b/CoreMIDI4J/src/uk/co/xfactorylibrarians/coremidi4j/CoreMidiSource.java @@ -35,7 +35,7 @@ public class CoreMidiSource implements MidiDevice { - private final CoreMidiDeviceInfo info; + private CoreMidiDeviceInfo info; private final AtomicBoolean isOpen; private final AtomicReference input; private final Set transmitters; @@ -77,6 +77,15 @@ public Info getDeviceInfo() { } + /** + * Changes the MIDI Info object; can only be done by this package as a result of a MIDI environment change event. + */ + void updateDeviceInfo(CoreMidiDeviceInfo info) { + + this.info = info; + + } + /** * Opens the Core MIDI Device *