atsushieno / ktmidi

Kotlin multiplatform library for MIDI access abstraction and data processing for MIDI 1.0, MIDI 2.0, SMF, SMF2 (MIDI Clip File), and MIDI-CI.
MIT License
70 stars 7 forks source link

MidiInput is missing a method for removeMessageReceivedListener() #90

Open FDelporte opened 2 months ago

FDelporte commented 2 months ago

Because of race conditions, it seems it is possible to assign a listener twice. It would probably be nice to have a way to remove a listener before assigning a new one, e.g. when change midi inputs.

public interface MidiInput : dev.atsushieno.ktmidi.MidiPort { public abstract fun setMessageReceivedListener(listener: dev.atsushieno.ktmidi.OnMidiReceivedEventListener): kotlin.Unit }

atsushieno commented 2 months ago

Adding the same event handler twice itself is not strictly harmful, but it indeed seems error-prone. And having no way to reset is indeed problematic, as the listener argument is not nullable.

We should really switch to add/remove listener style. I kind of avoided that as it is a breaking API change to the entire MidiAccess, but probably earlier to execute is better...

FDelporte commented 2 months ago

This is strange... I checked the ktmidi code and the listener is never a list, it's a single object being assigned. But still when switching between MIDI inputs I need to recreate the listener, otherwise it doesn't work, but I get the input events 2, 3, 4 times... Even tried making it null before creating a new one, but still get multiple input events

runBlocking {
            midiInput = JvmMidiAccess().openInput(portDetails.id)
            logger.info("Adding midi input handler for {}", midiInput!!.details.name)
            if (midiInputHandler != null) {
                midiInputHandler = null
            }
            midiInputHandler = MidiInputHandler()
            midiInput!!.setMessageReceivedListener(midiInputHandler!!)
            logger.info("Added midi input handler for {}", midiInput!!.details.name)
        }
atsushieno commented 2 months ago

Because you didn't close() the old MidiInput ?

FDelporte commented 1 month ago

Thanks, indeed closing en open midiinput before opening a new one, solves the double events!