Bug 18759 - boolean returns from MIDIOutput methods are inappropriately synchronous
boolean returns from MIDIOutput methods are inappropriately synchronous
Status: CLOSED FIXED
Product: AudioWG
Classification: Unclassified
Component: MIDI API
unspecified
PC All
: P2 normal
: TBD
Assigned To: This bug has no owner yet - up for the taking
public-audio
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-31 22:36 UTC by Chris Wilson
Modified: 2012-11-15 09:57 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Wilson 2012-08-31 22:36:17 UTC
Both sendMIDIMessage and sendMessage "return(s) a boolean signifying whether the operation was successful."

That sounds like this is intended to be a synchronous call, which is a very bad idea (since MIDI messages may take a long time to send, on a hardware MIDI port for example).  At the very least, this should state "signifying whether the message was successfully enqueued."

At most, we should probably add an onsuccess/onerror callback set to setMIDIMessage().
Comment 1 Jussi Kalliokoski 2012-09-05 10:00:46 UTC
(In reply to comment #0)
> Both sendMIDIMessage and sendMessage "return(s) a boolean signifying whether
> the operation was successful."
> 
> That sounds like this is intended to be a synchronous call, which is a very bad
> idea (since MIDI messages may take a long time to send, on a hardware MIDI port
> for example).  At the very least, this should state "signifying whether the
> message was successfully enqueued."
> 
> At most, we should probably add an onsuccess/onerror callback set to
> setMIDIMessage().

Agreed. The callbacks are something we can add at a later date if there are some actual use cases for them. I'll remove the notion of returning a boolean.
Comment 2 Jussi Kalliokoski 2012-09-05 12:15:22 UTC
Proposed changeset: https://dvcs.w3.org/hg/audio/rev/bf0e920450e6
Comment 3 Chris Wilson 2012-09-05 17:35:10 UTC
1. Do we want to make it explicit in the text that the operation may be "enqueues a MIDI message", rather than "send"?  (I.e. that the send may not have completed.)

2. The callback is probably most useful for the sendMIDIMessage form (since sysex may take a while) - although I do have one scenario that I wanted it for:  I have a Novation Launchpad (http://us.novationmusic.com/midi-controllers-digital-dj/launchpad), which is an 8x8 array of LED-lit buttons, plus 16 additional buttons; it is only USB 1.1, so can only accept a maximum of 
400 messages per second. Because there are 80 LED addresses, it takes 200 milliseconds to update a Launchpad completely.  Obviously, it's kinda cool to animate it - I've written a Conway's Game of Life implementation (https://github.com/cwilso/conway/tree/MIDIBridge/WMAS) that interfaces with the Launchpad - but you don't know when the messages have finished writing, so it's hard to update appropriately.  I don't just drive the animation as fast as it will go - you press a button to advance - but if I did, I might end up overrunning the buffers if I tried to push it faster than 5 updates a second.  (Probably far less, of course.)

Both OSX and Windows have callbacks to implement this, but the OSX one is ONLY for Sysex Send calls, so it would be better to have this only on sendMIDIMessage if we add it.
Comment 4 Florian Bomers 2012-09-05 19:34:45 UTC
I believe that it would be nice to know if an error occurred already when invoking the send method, most notable for the case that the MIDIPort was disconnected. For that, a boolean could be returned, specified as:
"returns false if an error occurred, returns true if the message was enqueued for asynchronous delivery"
Comment 5 Jussi Kalliokoski 2012-09-06 06:38:32 UTC
(In reply to comment #3)
> 1. Do we want to make it explicit in the text that the operation may be
> "enqueues a MIDI message", rather than "send"?  (I.e. that the send may not
> have completed.)

Yes.

> 2. The callback is probably most useful for the sendMIDIMessage form (since
> sysex may take a while) - although I do have one scenario that I wanted it for:
>  I have a Novation Launchpad
> (http://us.novationmusic.com/midi-controllers-digital-dj/launchpad), which is
> an 8x8 array of LED-lit buttons, plus 16 additional buttons; it is only USB
> 1.1, so can only accept a maximum of 
> 400 messages per second. Because there are 80 LED addresses, it takes 200
> milliseconds to update a Launchpad completely.  Obviously, it's kinda cool to
> animate it - I've written a Conway's Game of Life implementation
> (https://github.com/cwilso/conway/tree/MIDIBridge/WMAS) that interfaces with
> the Launchpad - but you don't know when the messages have finished writing, so
> it's hard to update appropriately.  I don't just drive the animation as fast as
> it will go - you press a button to advance - but if I did, I might end up
> overrunning the buffers if I tried to push it faster than 5 updates a second. 
> (Probably far less, of course.)

Sweet! And yes, it's actually an important use case (lighting systems).

> Both OSX and Windows have callbacks to implement this, but the OSX one is ONLY
> for Sysex Send calls, so it would be better to have this only on
> sendMIDIMessage if we add it.

But sendMessage can send SysEx messages too. Still, I agree, no need to complicate sendMessage with a callback.

We'll also have to clarify the enqueueing behavior. Do we want the implementations to drop messages that couldn't be sent to the port at the expected time? Or do we want the implementations to make sure that no messages are dropped?

The latter is not necessarily easy to execute and it has some drawbacks:
 * For one, keeping the buffers too busy will become a significant memory leak over time.
 * Keeping the buffers busy will also keep increasing latency.
 * What happens when there is nothing to write to (e.g. there is cable attached to a MIDI port)?
Comment 6 Chris Wilson 2012-09-06 12:38:21 UTC
Perhaps Florian's suggestion is the best - boolean return, true if successfully enqueued, false if there was a problem (MIDI port shut down, buffer overrun).
Comment 7 Chris Wilson 2012-10-17 23:46:14 UTC
This has been resolved.
Comment 8 Olivier Thereaux 2012-11-15 09:57:33 UTC
(In reply to comment #7)
> This has been resolved.

Closing.