WebAudio / web-midi-api

The Web MIDI API, developed by the W3C Audio WG
http://webaudio.github.io/web-midi-api/
Other
327 stars 49 forks source link

Allow cancelling future output #102

Closed agoode closed 9 years ago

agoode commented 10 years ago

It is not clear to me how to undo the effects of queuing up future messages: I think there should be a way to purge the queue, even if it is just a "delete everything" type of thing.

agoode commented 10 years ago

Actually, the more I think about it, the more I don't like the timestamp on send at all. (Timestamps are fine for incoming messages.)

I would much prefer solving this queuing problem in a general way either in web audio or another web standard. I don't like the special case of web midi taking a timestamp for outgoing events.

cwilso commented 10 years ago

To borrow from the CoreMIDI model, we could have a .flush() on output; but can you elaborate on why you "don't like timestamp on send() at all"? The jitter on callbacks in main-thread Javascript can be substantial (like, 50ms if JS garbage-collects; 10-15ms if substantial visuals or layout is happening). That's well above the detection threshold (i.e. users could hear that jitter). Why wouldn't you want to avoid that with a scheduling mechanism? We can't boil the ocean of turning Javascript into a realtime language here; without timestamps, the best we could do is throw our hands up and force everyone to put their MIDI code into a Worker thread (to protect it from layout, visuals, and hopefully GC).

In case it's not clear - I'm against removing timestamps. :) If we do remove them from send, we should presume the problem is not going to be there for receives, either.

agoode commented 10 years ago

I think what I want is a MIDIOutputNode and MIDIDestinationNode in the Web Audio sense. Web Audio already has a nice scheduling mechanism with start and stop, which I feel is what we want here.

This is definitely sounding like a v2 thing. But I think we could add flush() right now and still get much of the way.

cwilso commented 10 years ago

I think the need for processors would get covered by virtual midi ports; I don't want to do a whole node-graph system here. I can see wanting to clear the send cache, though.

ghost commented 10 years ago

Hello, I have just bumped into this issue and I'm wondering if there is any update about adding a flush method to cancel previously scheduled send calls (which are not yet sent to the port)?

I am hitting the situation where I want to send "All Notes Off" / "All Sound Off" (or perhaps individual "Note Off" messages if necessary) to implement a "stop" function in my player logic. These messages would be sent with a timestamp of 0 to send as soon as possible. The problem is that any number of scheduled "Note On" messages may be queued up and pending in the MIDIOutput, so I cannot arrive at a reliable way to stop all notes. Is there another way to do this with the current API?

Thanks!

nfroidure commented 10 years ago

@rmsmith the problem has been discussed in a previous issue, i managed to get a stop button working for this project https://github.com/nfroidure/MIDIPlayer .

cwilso commented 10 years ago

With the current API, you'd need to schedule an all notes off for the future (just past your lookahead limit).

Good question, though - should cancel()/flush() clear only note ons/controllers? Should it auto-send an all-notes-off?

On Sun, Oct 5, 2014 at 9:53 AM, Nicolas Froidure notifications@github.com wrote:

@rmsmith https://github.com/rmsmith the problem has been discussed in a previous issue, i managed to get a stop button working for this project https://github.com/nfroidure/MIDIPlayer .

— Reply to this email directly or view it on GitHub https://github.com/WebAudio/web-midi-api/issues/102#issuecomment-57943923 .

bome commented 10 years ago

does that mean that the API implementation inspects the scheduled messages and if it detects an All Notes Off or All Sound Off controller scheduled in the future, it cancels any pending output?

That doesn't sound right to me:

  1. Controllers are used for many things, and some devices or software use the respective All Notes Off and other controllers for other purposes. So it's likely that such a controller is sent without the intention to cancel any output.
  2. What happens to scheduled messages that are not audible like pitch bend or sys ex?
  3. Stored MIDI performances may include an All Note Off at the end of performance. If you schedule that performance, you won't hear anything...

A dedicated flush() method "sounds" much better. Actually, flush() may not be the right term, cancel() or purge() seem better suited. Thanks.

cwilso commented 10 years ago

1 no of course not. 2 good point, controllers shouldn't be messed with.

My question was more "should flush() automatically send a note off, or should the user have to do that to prevent stuck notes?"

On Oct 6, 2014 12:30 PM, "Florian" notifications@github.com wrote:

does that mean that the API implementation inspects the scheduled messages and if it detects an All Notes Off or All Sound Off controller scheduled in the future, it cancels any pending output?

That doesn't sound right to me:

  1. Controllers are used for many things, and some devices or software use the respective All Notes Off and other controllers for other purposes. So it's likely that such a controller is sent without the intention to cancel any output.
  2. What happens to scheduled messages that are not audible like pitch bend or sys ex?
  3. Stored MIDI performances may include an All Note Off at the end of performance. If you schedule that performance, you won't hear anything...

A dedicated flush() method "sounds" much better. Actually, flush() may not be the right term, cancel() or purge() seem better suited. Thanks.

— Reply to this email directly or view it on GitHub.

bome commented 10 years ago

OK. I know that (some) DAW's send All Sound Off on all channels when stopping and it's probably not (very) harmful :) Though maybe it's better to not do it in the implementation, but mention it in the documentation to such a cancel() method.

ghost commented 10 years ago

With the current API, you'd need to schedule an all notes off for the future (just past your lookahead limit).

Thanks, I have this implemented now and it is working; I had to reduce my lookahead amount to keep the delay until notes off reasonable (which is unfortunate as I really appreciate the scheduling capability!).

Good question, though - should cancel()/flush() clear only note ons/controllers? Should it auto-send an all-notes-off?

I believe that the cancel / clear method should simply drop all pending scheduled messages and leave the rest to the user; this would be the most flexible solution.

cwilso commented 10 years ago

As long as it doesn't need to communicate with the user what's being cleared out, I think that's fine.

On Tue, Oct 7, 2014 at 1:05 AM, Ross Smith notifications@github.com wrote:

With the current API, you'd need to schedule an all notes off for the future (just past your lookahead limit).

Thanks, I have this implemented now and it is working; I had to reduce my lookahead amount to keep the delay until notes off reasonable (which is unfortunate as I really appreciate the scheduling capability!).

Good question, though - should cancel()/flush() clear only note ons/controllers? Should it auto-send an all-notes-off?

I believe that the cancel / clear method should simply drop all pending scheduled messages and leave the rest to the user; this would be the most flexible solution.

— Reply to this email directly or view it on GitHub https://github.com/WebAudio/web-midi-api/issues/102#issuecomment-58150166 .

ghost commented 10 years ago

I vote for the simple clear() function that clears everything that has previously been sent. Users can resend parts of the stream if they need to do that afterwards.

interface MIDIOutput : MIDIPort {
    void send (sequence<octet> data, optional double timestamp);
    void clear ();
};

Naming the function flush would probably cause confusion, the name suggests any pending (previously sent) data would be flushed/pushed to the hardware.

Just my two cents.

toyoshim commented 9 years ago

Hi guys,

Now I implemented open() and close() in Chromium though there is room for discussion on state transition. So can I revisit this issue to have cancelling future.

In my preference, having such future sounds reasonable.

For naming, as some people said, flush() sounds misleading. I will imagine that the pending messages with timestaamps will be sent out immediately by flush(). cancel() and clear() sound fine, but I slightly prefer cancel() because it does not imply doing additional some cleanup efforts,

I believe main use case is for stopping playback, or shutting down DAW system. and process is

  1. cancel()
  2. send() with some useful messages, e.g., all note off. this may depend on device and applications want to handle it by themselves.
  3. optional: close()
  4. optional: nullify Web MIDI API related references, or purge embedded frame that uses Web MIDI API.

So what cancel() should do is just remove pending messages from an internal message queue for the MIDIPort from the MIDIAccess.

nfroidure commented 9 years ago

Sounds great, MIDI players implementation would be less tricky.

ghost commented 9 years ago

This feature is at the top of my Web MIDI wish list, mainly for reason @nfroidure mentioned but it will also be useful for clearing buffered sequencer output etc, so it gets two thumbs-up from me.

toyoshim commented 9 years ago

yeah, also an application may send data with wrong timestamp that specify further future, e.g. one month later due to unexpected application bugs. In that case, it would be painful to live without cancellation.

@si-robertson do you have any thoughts on naming do you still prefer clear() rather than cancel()?

ghost commented 9 years ago

Given the choice, I would choose clear(). Naming the method cancel() is fine but it implies the cancellation of an ongoing operation rather than the clearance of a buffer.

ghost commented 9 years ago

Great to hear that this is happening! +1 for clear but cancel also works.

cwilso commented 9 years ago

@toyoshim are you happy with clear()?

bome commented 9 years ago

I guess the problem is that we execute the method (clear/cancel/flush/purge) on a port, but in fact it applies to the buffer. So maybe something like clearBuffer or cancelEvents is less controversial (albeit more to type and harder to remember)?

ghost commented 9 years ago

How about clearData? That comes from the parameter name of the send method: void send (sequence<octet> data, optional double timestamp);

cwilso commented 9 years ago

I'd rather keep away from declaring anything about a "buffer", since it may be implemented in various ways internally. Can anyone not live with clear()?

toyoshim commented 9 years ago

I'm fine with clear(). Now I feel cancel() is ambiguous because it can be cancellation of open or send. When we say clear(), it apparently clears pending data.

Also, should is be available only on open state?

cwilso commented 9 years ago

Yes, this should only function in the open state; close() should probably clear any pending data?

toyoshim commented 9 years ago

I think close() should do nothing against pending data because developer may want to call them like;

port.clear(); port.send([...]); // note off or something useful for port finalization port.close();

In this case, we do not want close() to clear data sent by the previous send(). So, close() may finish when the last data from the MIDIAccess is sent out, or device got disconnected. But, when all MIDIAccess references are gone, and MIDIAccess is GC-ed, all pending data should be cleared.

If close() clear all pending data, it will be very difficult to send such finalization data correctly without returning Promise even for send.

bome commented 9 years ago

I can see the use case for sending All Sound Off or similar, but close() should not be allowed to take forever. So maybe it should only wait for messages to be sent immediately; any messages scheduled for any point in the future will get cleared. This is clumsy for the spec, but might yield best user experience? Having said that, I'd also be fine with close() discarding all messages. If you want to send a message right before closing, you'd need to wait a couple of milliseconds before calling close(). This is not foolproof, but will work fine.

ghost commented 9 years ago

I would like to see close() clear all messages, having the port remain active would be confusing.

Maybe the API could be extended to indicate if there are any buffered messages, that way developers could check if it's safe to close a port without losing data.

Either way, close() should clear and close the port immediately IMHO.

toyoshim commented 9 years ago

Since close() is async interface returning Promise, it's natural to wait for something. But it may be reasonable that close() will clear data with timestamp.

Also, I noticed that now open and connection are separate two states, open() and close() do not need to be asynchronous APIs returning Promise. That can be simply void open() / void close().

cwilso commented 9 years ago

As noted in #79, open() can still fail (exclusive-access systems).