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

Add MidiInputPort::id() and MidiOutputPort::id() #157

Merged
merged 6 commits into from
Nov 20, 2024

Conversation

oscartbeaumont
Copy link
Contributor

@oscartbeaumont oscartbeaumont commented Oct 3, 2024

Allows getting a unique reference to the port as an opaque string. The main advantage of this over just using the MidiInputPort and MidiOutputPort structs is that this identifier can be serialized.

In my case i'm trying to make tauri-plugin-midi work with many different devices that have the same device name. Being able to serialize these identifiers means we can share them with the webview and use them to address any communications.

By using the identifiers from the OS midi stuff, we should also be able to get identifier which are stable across restarts of the application (and potentially the system but that's really up to the OS).

I think this might close #34

One implication of the code in this PR is that it does some lossy string conversions. I suppose we could probs add in a 3rd party dependency or extra logic to not lossily encode them but i'm not sure it's worth doing unless it does actually become a problem but happy to see what you think.

@oscartbeaumont oscartbeaumont changed the title MidiInputPort::id() and MidiOutputPort::id() Add MidiInputPort::id() and MidiOutputPort::id() Oct 3, 2024
@Boddlnagg
Copy link
Owner

Thanks for the PR!

The idea certainly makes sence, but I have to admit I don't really understand this sentence:

Being able to serialize these identifiers means we can share them with the webview and use them to address any communications.

Maybe I just don't know enoguh about tauri ... but how is serialization helpful without deserialization? How can you reconnect to the same port based on the id()? I think deserialization would be needed to actually solve #34.

Anyway, I'm on vacation now until the beginning of November, so it might take a while until I get back to you.

@oscartbeaumont
Copy link
Contributor Author

oscartbeaumont commented Oct 4, 2024

With Tauri we define a set of commands in Rust that can be called from the webview to do MIDI actions. These look like:

  • openInput(id: string)
  • closeOutput(id: string)
  • outputSend(id: string, msg: number[])

We also maintain two maps in Rust which contain a mapping between the id and the input/output (Eg. HashMap<String /* MidiInputPort::id() */, midir::MidiInputConnection<()>>). Then we can use the id being parsed to the command to grab the input/output out of the map and use it for the specific action requested by the webview.

Basically MidiInputPort::id() means we can stop using the ports name to identify the port which is proving very problematic with multiple devices that have identical names (Eg. multiple physical devices of the same type).

In pratice with the previous system of identify ports by name the call to map.insert(name, port) for the second device would override the first one due to the name (and hence map key) being the same.

but how is serialization helpful without deserialization

The ID being returned is a String in Rust so it can easily do both.

I think keeping it as a string makes the most sense because it doesn't require any special integration with serialization libraries and it also allows joining the two different internal identifier used by Alsa into one.

@Boddlnagg
Copy link
Owner

Okay, so the "deserialization" basically enumerates all input/output ports and picks the one where the id() matches the one we're looking for?

In that case, I think it would make sense to also have a method find_port_by_id(id: String) -> Option<Midi{Input,Output}Port> on MidiInput/MidiOutput that streamlines this.

@oscartbeaumont
Copy link
Contributor Author

Okay, so the "deserialization" basically enumerates all input/output ports and picks the one where the id() matches the one we're looking for?

Basically.

When the webview calls open_output/open_input we iterate all ports for the one with the correct id.

We then put the socket into a shared map (keyed by the id) which holds the input/output between open, send and close calls so we don't open, send then close a new Midi input/output on every send call from the webview. This helps us match the semantics of the WebMIDI API.

In that case, I think it would make sense to also have a method find_port_by_id(id: String) -> Option<Midi{Input,Output}Port> on MidiInput/MidiOutput that streamlines this.

I suppose this does make sense, implemented. This would remove a tiny bit of code from our open methods which is nice.

Also if you do wanna have to look the implementation of tauri-plugin-midi it is less than 300 lines of Rust and if you haven't touched Tauri before everything with #[tauri::command] is callable by the webview (hence the arguments and results are serialized with Serde) and that's about all that is different.

src/backend/alsa/mod.rs Outdated Show resolved Hide resolved
@Boddlnagg
Copy link
Owner

I suppose this does make sense, implemented. This would remove a tiny bit of code from our open methods which is nice.

Thanks, this is exactly what I had in mind 🙂

@oscartbeaumont
Copy link
Contributor Author

Fixed!

@Boddlnagg Boddlnagg merged commit 5b05f61 into Boddlnagg:master Nov 20, 2024
11 checks passed
@Boddlnagg
Copy link
Owner

Thanks again, and sorry that it took me so long to get back to you!

@Boddlnagg
Copy link
Owner

I also published version 0.10.1 which includes this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Persistent port identifiers
2 participants