WebAudio/web-midi-api

What is MIDIOutput.clear() meant to do?

djipco opened this issue · 6 comments

djipco commented

I'm not sure if the problem is with the wording of the spec, with the way Firefox implemented it or maybe it's just me but I was expecting MIDIOutput.clear() to also delete messages queued up for later delivery. After running some tests, it seems it is not the case.

For example, the code below does trigger the notes scheduled for 3 seconds later (in Firefox 120.0.1):

function onMIDISuccess(midiAccess) {
  midiAccess.outputs.forEach(o => {
    o.send([0x90, 60, 0x7f]);
    o.send([0x90, 60, 0x7f], performance.now() + 3000); // those also get sent
    o.clear();
  });
}

navigator
  .requestMIDIAccess()
  .then(onMIDISuccess, msg => `MIDI error: ${msg}`);

The spec says that the clear() method Clears any pending send data that has not yet been sent from the MIDIOutput's queue. Shouldn't that also include messages scheduled to be sent later? Is there something I'm not understanding properly?

mjwilson-google commented

First, as noted in the linked issue, Chromium (and by extension Chromium-based browsers) have not implemented clear(). Here is the tracking bug: https://crbug.com/471798. This is something I would like to fix early next year if possible.

Reading the spec, I would also expect that the pending messages shouldn't be sent. @padenot can you comment on the Firefox implementation -- if this is a Firefox bug or if we need to clarify the specification?

padenot commented

Here's our implem:

https://searchfox.org/mozilla-central/source/dom/midi/MIDIOutput.cpp#97-101 (not very interesting, just does the IPC call to the process that can use the system-level MIDI API)
https://searchfox.org/mozilla-central/source/dom/midi/MIDIPlatformService.cpp#107 (just call clear on the message queue that contains the message that haven't been sent yet)

When a message has a timestamp, it goes into a different queue that's delayed, and that doesn't get cleared, but reading the spec it's our bug. https://bugzilla.mozilla.org/show_bug.cgi?id=1869483 tracks this on our side.

djipco commented

Thanks @padenot. This clears things out.

I guess the question now becomes: should the spec explicitly state that clear() must cancel both queued and scheduled messages? In all honesty, I really wasn't sure if the spec intended for scheduled notes to also be cleared. I think they should be but maybe the spec's language could be made clearer in this regard.

mjwilson-google commented

The spec doesn't use the term scheduled anywhere, so I don't think we should introduce it.

The current wording is: Clears any pending send data that has not yet been sent from the MIDIOutput's queue.

However, pending is mostly used in the spec to refer to a port connection state, which is unrelated to the clear method.

Would something like Clears any enqueued send data that has not yet been sent from the MIDIOutput's queue. be better? That would mirror the language of the send method.

padenot commented

Would something like Clears any enqueued send data that has not yet been sent from the MIDIOutput's queue. be better? That would mirror the language of the send method.

I think so, at least it's clear to me as an implementor.

  • 👍2
mjwilson-google commented

Ok, let's make the change. Thank you for bringing this up!

  • 👍1
  • 🎉1