Backpressure exposure for asynchronous send()

[agoode]

We have so far been able to use synchronous send but this provides no mechanism for backpressure.

The current implementation in Chrome uses large internal buffers to hide this issue, but this is wasteful and results in the browser silently dropping messages when the buffer is overrun. This condition is not common with normal MIDI messages, but sysex messages can be of arbitrary length and can be slow to send, so Chrome also puts limits on those as well.

In order to allow implementations to avoid these buffers and arbitrary limits, we need a mechanism for non-blocking send.

[yutakahirano]

Is Streams useful for the use case? cc: @domenic @tyoshino

[domenic]

This does sound like a pretty good fit for a writable stream... In fact MIDIOutput looks pretty WritableStream-like in general. It is specifically designed to take care of this queuing and backpressure-signaling concern for you, so that other specs can build on top of it and reuse the architecture.

I'm not sure about MIDIInput, as I'm not familiar enough with MIDI. The choice would be between the current spec's no-backpressure, event-based model (where if you aren't subscribed you miss the event), vs. a readable stream model with the data buffered for future reading, and potential backpressure when it's not read.

Unfortunately the spec's current setup where MIDIInput and MIDIOutput both derive from MIDIPort doesn't seem very easy to fit with the stream API. I'm not sure how we'd do this without just creating a parallel WritableMIDIPort type (and maybe ReadableMIDIPort).

There's also the issue that writable streams need some spec love, but if this is a urgent use case I can turn my attention to them and get that straightened out quickly.

[agoode]

The event-based model for MIDIInput is fine, since MIDI is essentially a multicast system, with no backpressure possible at the protocol level. MIDI doesn't guarantee reliable delivery.

Can WritableStream handle framing? We do parse the bytes and require that they are valid MIDI packets (1, 2, or 3 bytes in the common case, or arbitrary length in the case of sysex). Is this implementable as chunks in the streams standard? Can a stream reject an invalid chunk?

[domenic]

Can WritableStream handle framing? We do parse the bytes and require that they are valid MIDI packets (1, 2, or 3 bytes in the common case, or arbitrary length in the case of sysex). Is this implementable as chunks in the streams standard? Can a stream reject an invalid chunk?

Yes, for sure. The underlying source's write hook (and optional writev hook, once we get that working) can process the data in arbitrary ways, returning a promise for when the processing is finished---which can be rejected to indicate that the input was invalid.

[jussi-kalliokoski]

👍 for using streams from me.

We could add a new method to MidiPort: Promise<Stream> openAsStream() to preserve bw compat.

However, how do we solve sending timed messages?

[bome]

I'm somewhat reluctant for adding streams, it's seems like mainly adding clutter to the WebMIDI API and implementation, while providing very little new functionality. Can't we just add a function sendAsync(data, timestamp) with a Promise as return, or define a MIDIOutput listener which fires whenever a MIDI message is delivered?

results in the browser silently dropping messages

That's really bad. In that case, the send() function should either block, or throw an exception of some sort (which would need to be defined in the spec).

[cwilso]

I'm REALLY uncomfortable rebasing on top of Streams, since it's highly
unlikely you would be piping any readablestream into a MIDI output device.
I think the best way to solve this is have send() return a Promise (that
resolves when it's been sent to the hardware), and expose the "current
available output message size" (i.e. the maximum chunk size you can write,
right now). This would enable things that currently work to keep working;
it would enable developers who want to send large amounts of sysex data to
do so, too.

On Thu, Feb 11, 2016 at 9:53 AM, Florian notifications@github.com wrote:

I'm somewhat reluctant for adding streams, it's seems like mainly adding
clutter to the WebMIDI API and implementation, while providing very little
new functionality. Can't we just add a function sendAsync(data, timestamp)
with a Promise as return, or define a MIDIOutput listener which fires
whenever a MIDI message is delivered?

results in the browser silently dropping messages
That's really bad. In that case, the send() function should either block,
or throw an exception of some sort (which would need to be defined in the
spec).

—
Reply to this email directly or view it on GitHub
#158 (comment)
.

[jussi-kalliokoski]

I think the best way to solve this is have send() return a Promise (that
resolves when it's been sent to the hardware), and expose the "current
available output message size"

This would just be duplicating stream functionality, except in a more awkward and less interoperable way. I can see plenty of use cases for MIDI messages in Streams. For example:

• Developers can use general purpose stream transform utilities (such as buffering with different strategies like FIFO to ensure all messages get delivered in order, or another strategy that makes sure certain messages take priority).
• Pipe messages from one client's MIDIInput to another's MIDIOutput via P2P communications or websockets.

[domenic]

I agree that such a design is just duplicating streams and will need to reinvent the queuing mechanisms and so forth they are specifically designed to let other specs reuse. The "current available output message size" is further evidence of that kind of duplication (of the desiredSize property of a writable stream, after whatwg/streams#318 is implemented). It's similar to reinventing an IDBRequest-style object when promises are available, based on the speculation that nobody would ever need to reuse the return value inside a promise callback.

However, how do we solve sending timed messages?

I'd assume each chunk would be of the form { timestamp, data } or similar.

[jussi-kalliokoski]

I'd assume each chunk would be of the form { timestamp, data } or similar.

That sounds reasonable.

[cwilso]

Can someone who understands streams put together a sketch of an API based on it, and examples of simple and complex usage?

[jussi-kalliokoski]

Maybe something like this:

interface MIDIOutput {
  // ...
  Promise<WritableStream> openAsStream({ size, highWaterMark });
}

interface MIDIInput {
  // ...
  Promise<ReadableStream> openAsStream({ size, highWaterMark });
}

here's 2 usage examples - the first is just a dumb non-synced sequencer writing to the first available MIDIOutput, the second one takes advantage of piping streams to pipe from first MIDIInput to first MIDIOutput, filtering out messages that aren't note on or note off, pushing all messages to channel 3, pitch shifting by 2 octaves and buffering based on number of messages at high water mark of 1000.

EDIT: Note that the advanced example uses TransformStream which is not in the Streams spec yet.

[toyoshim]

Reply for the first Adam's description:
send is not synchronous at all. If so, we can just return boolean, but we cannot. Also generally said we should avoid having synchronous APIs as much as possible on the main thread. So, what we want to do in this thread is to add a reliable send-like method.

Maybe using Streams would be a right approach. But I feel it's a little complicated as Chris said. Also, I feel Web MIDI should be aligned with other similar modern APIs. For instance Web Bluetooth and WebUSB return Promise.

So my preference is bome's sendAsync like thing. But, I feel sendAsync is confusing name since as I said existing send is also async.

Here is my proposal.
send: a user can use it for short messages, and sysex. But internal implementation may drop a message if a user want to send many data in a short time. Also very long sysex mesages would fail to send always.
sendLong: returning Promise. could not be called twice before the previous call finish. internal implementation never drop a message for buffer limits, but it does not ensure that the message reaches to the device because MIDI transport layer does not have any fail-safe mechanisms and the message could be dropped on immediate browser and OS shutdown. We probably need to allow sending long sysex messages as fragments.

[domenic]

The sketch in #158 (comment) is pretty reasonable, although we'd update it to use the writer design. I'll add some explicit demonstrations of backpressure support:

destination.openAsStream().then(s => s.getWriter()).then(async (writer) => {
  console.log(writer.desiredSize); // how many bytes (or other units) we "should" write

  // note: 0 means "please stop sending stuff", not "the buffer is full and
  // we will start discarding data". So, desiredSize can go negative.

  writer.write({ data: data1, timestamp: ts1 }); // disregard backpressure

  // // wait for successful processing before writing more
  await writer.write({ data: data2, timestamp: ts2 });

  await writer.waitForDesiredSize(100); // wait until desiredSize goes >= 100

  writer.write({ data: oneHundredTwentyBytes, timestamp: ts3 });

  await writer.waitForDesiredSize(); // wait until desiredSize goes >= high water mark

  // also, maybe default timestamp to performance.now()?
});

I might also collapse @jussi-kalliokoski's .openAsStream(new CountQueuingStrategy({ highWaterMark: 1000 })) into .openAsStream({ highWaterMark: 1000 }).

[toyoshim]

After thinking about incremental writing of a large sysex, I noticed that it will allow a malicious attack to lock all output ports exclusively. E.g. just sending a "sysex start" byte will lock the port forever. So, we should keep on having the restriction that a user can not send an incomplete or a fragment message.

So even if we have a back-pressure, the sysex size will be limited to the maximum size of ArrayBuffer.