Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Register MessagePort directly instead of onmidimessage #215

Closed
wrongbad opened this issue Oct 8, 2020 · 6 comments
Closed
Labels
class: substantive https://www.w3.org/policies/process/#correction-classes
Milestone

Comments

@wrongbad
Copy link

wrongbad commented Oct 8, 2020

Currently, to send midi to a worker or AudioWorklet, you have to register a midi callback in the main js context, which receives messages and then forwards again to worker or AudioWorklet. For example:

midiInput.onmidimessage = msg=>audioNode.port.postMessage(msg)

A simple improvement would allow direct message port hook up to establish direct communication between high priority contexts. Like this:

midiInput.registerPort(audioNode.port)

@cwilso cwilso added this to the V1 milestone Oct 8, 2020
@toyoshim
Copy link
Contributor

Can we use a shared ArrayBuffer pattern rather than exposing new APIs to the AudioWorklet?

@wrongbad
Copy link
Author

wrongbad commented Oct 12, 2020

Maybe I'm misunderstanding you, but my suggestion is using the message port API which has already been present on the AudioWorklet and adding a new API to MidiAccess in the main page context to forward directly.

@toyoshim
Copy link
Contributor

It's really difficult to route a received message to another thread, i.e. worker or worklet, directly without trapped by the main thread as other related contexts lie on the main thread. So, even if we implement something you suggested, possible result would be just a syntax sugar, and could not mitigate jitter issues caused by the thread-hop.

Also, regarding the port use, assuming the port attribute of the AudioWorkletProcessor, it's a general MessagePort, and the spec does not limit any possible use that users may utilize. So, I think it isn't a good idea to introduce Web MIDI specific restrictions to the port, as we need to define a message format for the Web MIDI and users need to avoid conflicting with it if we go.

If AudioWorklet inherited EventTarget, we could redirect the msssage as a midimessage as-is. But it isn't, and even if it is, still it would be just a syntax sugar and not so powerful as the original snippet is already a simple one liner.

Regarding the shared ArrayBuffer pattern, I'm actually not familiar with it, but @hoch will be able to answer if you have a question about it.

@wrongbad
Copy link
Author

wrongbad commented Oct 22, 2020

Regarding sharing the worklet message port, I'm assuming that more ports could be sent over into the audio worklet context to establish non-conflicting channels for user messages and midi messages.

But your first point seems more of a blocker. IMO the only reason to do something like this would be for latency/jitter improvement. I don't really understand browser architecture enough to see why, but that's a real bummer if MessagePorts can't operate uninhibited between threads. Maybe that should get fixed also? It just seems like the most safe and elegant way to hand arbitrary low-level API access to a high-priority worker - get access to whatever API (midi, serial, etc) in main thread and then send the ports to the worker.

Shared ArrayBuffer seems interesting... I'm assuming you'd implement some form of ring-buffer-queue on top of it to dynamically queue different amounts of midi data? (Unlike audio, it's not locked to a fixed bytes/cycle). And I guess whatever the format, it's stuck in the API spec because the high-priority midi context is all native code.

Maybe this all circles back to some way to grant audio context direct midi access (issue I've seen on here already).

@mjwilson-google mjwilson-google added the class: substantive https://www.w3.org/policies/process/#correction-classes label Sep 13, 2023
@mjwilson-google mjwilson-google added the status: needs WG review Needs to be discussed with the Audio Working Group before proceeding label Sep 30, 2023
@mjwilson-google
Copy link
Contributor

Audio Working Group 2023-10-05 meeting conclusions:

  • We think the best way to handle this is through using SharedArrayBuffer (possible now) and a Worker-instantiated MIDIAccess (Issue MIDI API should be available from Workers? #99)
  • We don't want to pursue adding a register port method

I will close this issue and we will focus on #99 as a solution for the use case. Please feel free to reopen or add discussion here or on #99 if there is something we are missing.

@mjwilson-google mjwilson-google closed this as not planned Won't fix, can't repro, duplicate, stale Oct 5, 2023
@mjwilson-google mjwilson-google removed the status: needs WG review Needs to be discussed with the Audio Working Group before proceeding label Oct 5, 2023
@wrongbad
Copy link
Author

wrongbad commented Oct 6, 2023

Ah I misunderstood the SharedArrayBuffer suggestion earlier - I thought they meant as a new MIDI API, but now I see it's a user-code suggestion to hop from main thread to audio worker.

Also I see the link in #99 that explains how postMessage() is much less efficient than I assumed. I'm used to c++ where you just std::move things into queues and it's very cheap, but I guess JS shared object ownership model doesn't generally allow moving like that.

Anyway, if #99 is in progress now, that sounds like a much better long term solution. Thanks for looking into this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
class: substantive https://www.w3.org/policies/process/#correction-classes
Projects
None yet
Development

No branches or pull requests

4 participants