1728436 - Vendor the midir crate

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, task)

Description

[Gabriele Svelto [:gsvelto] (PTO)]

The midir create provides a platform-independent abstraction for implementing midi functionality that supports several backends including Windows (two, one backwards compatible with Windows 7), macOS (via CoreMIDI) and Linux (via alsa). We should vendor it to implement WebMIDI on those platforms. There are some issues that need to be addressed first though including dependencies that we don't want to duplicate:

• It requires a version of nix that's more recent than what we already have (0.15 vs 0.13.1)
• It requires versions 0.2.3 of both the core-foundation and core-foundation-sys crates, which are both much older than what we already have
• It includes wasm as a target which depends on a number of wasm crates that we don't care about and don't want to vendor. I haven't figured out how to exclude those dependencies yet.

Comment 1

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: alsa-error.log

Also it seems that a custom build step in the alsa crate fails (but it shouldn't). Something else to look into.

Comment 2

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: Bug 1728436 - Vendor the midir crate r=padenot

This is as close to usptream as currently possibly. Only a few changes were
done to the dependencies: the wasm target was removed and the coremidi
dependency was updated to pick up a more recent version so that we don't need
to vendor separate versions of the core-foundation and core-foundation-sys
crates.

This vendors the following crates:

• alsa-sys
• alsa
• coremidi
• coremidi-sys
• memalloc
• midir

Overall this adds ~30K lines of code, over half of which is in the alsa
bindings alone.

Comment 3

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: Bug 1728436 - Added libasound2 dependency to the sysroot r=glandium

This is required to build the midir crate on Linux

Depends on D124640

Comment 4

[Gabriele Svelto [:gsvelto] (PTO)]

This is pretty straightforward, just a small set of changes to minimize the crates we need to vendor and a few changes on our side to avoid building on Android and to add the required dependencies on Linux. I'm going to contact upstream to see if I they can accommodate changes for our requirements. The only thing I'm not sure how to tackle yet are the wasm target dependencies (which I removed). I think I could modify ./mach vendor rust to automatically ignore them given we don't support that particular target. Paul, what do you think?

Comment 5

[Bobby Holley (:bholley)]

See also https://github.com/rust-lang/cargo/issues/7058

Comment 6

[Mike Hommey [:glandium]]

Note we currently don't build libxul against libalsa...

Comment 7

[Mike Hommey [:glandium]]

also, I'm not sure what that would mean for the sound stack when midir would use alsa and other parts of the browser would use pulseaudio...

Comment 8

[Paul Adenot (:padenot)]

Gabriele, this plan sounds good, hopefully we'll be able to use mainline at some stage, those changes are not too problematic I think?

As for Alsa, it won't disturb audio, we're not using Alsa for any audio stuff here, just MIDI. Pulse is always a priority when trying to find what audio backend to use. If we don't want to link against Alsa or lower the vendored line count, we can probably do late binding and/or subset the functions that bindgen binds, it's all generated anyway.

Comment 9

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Paul Adenot (:padenot) from comment #8)

As for Alsa, it won't disturb audio, we're not using Alsa for any audio stuff here, just MIDI.

Yes, I've looked at the API and from what I can tell the sequencer API (ALSA speak for MIDI) is completely separate from the audio bits and pulse doesn't touch it. It should be possible to use pulse for audio output and ALSA for MIDI control simultaneously.

Comment 10

[Gabriele Svelto [:gsvelto] (PTO)]

FYI since we'll probably need to fork/patch the crate for the initial integration I'm informing the author beforehand to inform him that I'd very much prefer using the upstream crate whenever possible. For the time being I don't think it would be possible because of the issue highlighted in comment 5.

Comment 11

[Gabriele Svelto [:gsvelto] (PTO)]

I forgot... I will wait for a response from the author then fork midir on GitHub into mozilla/midir and vendor from there. If anybody thinks that's a bad idea just let me know. Now onto the actual implementation work...

Comment 12

[Gabriele Svelto [:gsvelto] (PTO)]

Alright, so the upstream author welcomes patches and was delighted to hear his project was ending up in Firefox. Hurray for a nice upstream experience \o/.

I'll try and generate some bindings and I'll send that upstream first. One thing worth of note is that the crate is switching from winapi-rs to windows-rs for Windows targets so we'll have to vendor that eventually. I knew it would happen sooner or later anyway.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/791abc86e799
Vendor the midir crate r=padenot
https://hg.mozilla.org/integration/autoland/rev/063de5a6f6dd
Added libasound2 dependency to the sysroot r=glandium

Comment 14

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/791abc86e799
https://hg.mozilla.org/mozilla-central/rev/063de5a6f6dd