1750763 - Sending MIDI data via through port doesn't seem to work

Status: VERIFIED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Gabriele Svelto [:gsvelto]]

See bug 836897 comment 114. The STR is the following:

1. Open https://blog.karimratib.me/demos/sheetplayer/ in a tab and allow it to use WebMIDI
2. Switch the output to a MIDI through port
3. Open https://mmontag.github.io/dx7-synth-js/ in another tab
4. Go back to the first tab and click play

Expected result:

The second tab plays the music being sent via the MIDI through port.

Actual result:

A glitchy sounds is briefly heard then only silence.

Comment 1

[Gabriele Svelto [:gsvelto]]

FYI I did a test using https://mikatahara.github.io/MidiMonitor/ as the sink instead of https://mmontag.github.io/dx7-synth-js and it works as expected, strange.

Comment 2

[Karim Ratib]

Thanks. I filed an issue on the DX7 repo https://github.com/mmontag/dx7-synth-js/issues/12, and time permitting I will examine the DX7 Web MIDI implementation to determine whether they are making non-standard use of the Web MIDI API, or if it's the current implementation here that's not expecting a legitimate use-case.

Comment 3

[Gabriele Svelto [:gsvelto]]

I still think it's a problem on our side; the new code needs to be battle-tested and while it's usable I'm sure there's still quirks and issues lurking within.

Comment 4

[Karim Ratib]

The funny thing is, if I run https://blog.karimratib.me/demos/sheetplayer/ from Chrome and https://mmontag.github.io/dx7-synth-js/ from FF 98.0a1 on the same Midi Through Port, I do hear the full piece.

Comment 5

[Gabriele Svelto [:gsvelto]]

I've been investigating this a bit and the issue might be on the sending side. I hadn't noticed but Chrome implements two send() methods on the MIDIOutput object. One takes a Uint8Array for the data element and the other takes a sequence<unsigned long> one. I wonder if this difference might be causing the issue.

Comment 6

[Karim Ratib]

According to the spec, send() should accept both.

Comment 7

[Gabriele Svelto [:gsvelto]]

Yeah, and indeed that didn't make any difference. The existing code works fine, I think the only difference with Chrome is that we'll throw a different exception if sending data that can't be converted from 32- to 8-bits. So I did a bunch of tests. It seems that the issue is sending from Firefox. Chrome -> Firefox works fine, Firefox -> Firefox and Firefox -> Chrome don't. I verified that the send path delivers the data correctly to midir so I'm down to two possible issues: a bug in midir but that sounds unlikely and a bug in the way we handle timestamps. I'm leaning on the latter.

Comment 8

[Gabriele Svelto [:gsvelto]]

I just verified that this happens on Windows too using loopMIDI to pipe the messages so it's definitely a bug in Firefox and not in midir.

Comment 9

[Gabriele Svelto [:gsvelto]]

Alright, this merits some serious facepalming on my part (insert double- or even triple-facepalm TNG meme here). We're ignoring the timestamps passed to send(). The noise you hear when we start playing is all the notes being pushed into the output port at once.

Comment 10

[Gabriele Svelto [:gsvelto]]

Attachment: Bug 1750763 - Honor the timestamps on outgoing MIDI messages r=padenot

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fffc34a8a4ff
Honor the timestamps on outgoing MIDI messages r=padenot

Comment 12

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/fffc34a8a4ff

Comment 13

[Karim Ratib]

I downloaded Firefox 99.0a1 (2022-02-10) but I'm seeing the same issue. Is this not what

Target Milestone: --- → 99 Branch

means?

Comment 14

[Gabriele Svelto [:gsvelto]]

The last version of nightly was released this morning, the patch here will go in tomorrow's build.

Comment 15

[Gabriele Svelto [:gsvelto]]

Comment on attachment 9263082 [details]
Bug 1750763 - Honor the timestamps on outgoing MIDI messages r=padenot

Beta/Release Uplift Approval Request

• User impact if declined: Notes (re)played from Firefox via Web MIDI are delivered in the wrong sequence and all at the same time
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See the STR: https://bugzilla.mozilla.org/show_bug.cgi?id=1750763#c0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Web MIDI is not enabled yet, we'll turn it on only after landing all the stabilization patches
• String changes made/needed: none

Comment 16

[Pascal Chevrel:pascalc]

Is Web MIDI shipping in 98? If not, I'd prefer Web MIDI patches to ride the trains unless there is a specific need for 98 such as running an experiment.

Comment 17

[Gabriele Svelto [:gsvelto]]

We were supposed to ship in 97 but this bug and bug 1748643 were in the way. With both fixed we should be good so we slipped only one release.

Comment 18

[Karim Ratib]

(In reply to Gabriele Svelto [:gsvelto] from comment #14)

The last version of nightly was released this morning, the patch here will go in tomorrow's build.

I verified the fix in today's 990a1. Thank you!

Comment 19

[Hani Yacoub, Desktop QA]

I’m having difficulties trying to reproduce the issue on my end in order to confirm it.

1. I didn’t receive any prompt to allow WebMIDI like the one I received on Chrome
2. The only output displayed is local synth, there is no option MIDI through port displayed

I would appreciate some extra steps to help me reproduce the issue.
Thanks.

Comment 20

[Gabriele Svelto [:gsvelto]]

(In reply to Hani Yacoub from comment #19)

I’m having difficulties trying to reproduce the issue on my end in order to confirm it.

1. I didn’t receive any prompt to allow WebMIDI like the one I received on Chrome
2. The only output displayed is local synth, there is no option MIDI through port displayed

I would appreciate some extra steps to help me reproduce the issue.

Sorry, I think I left out some steps:

• First of all this needs to be done on Linux
• These two preferences need to be set first in about:prefs

dom.webmidi.enabled true
dom.webmidi.gated false

Then the ports should be visible and the test should work.

Comment 21

[Hani Yacoub, Desktop QA]

Thanks for the extra details.
The issue seems to be reproducible on the latest Firefox Nightly following these steps:

1. dom.webmidi.enabled true
   dom.webmidi.gated false
2. Open https://blog.karimratib.me/demos/sheetplayer/ in a tab and allow it to use WebMIDI
3. Switch the output to a MIDI through port
4. Open https://mmontag.github.io/dx7-synth-js/ in another tab
5. Tried allowing, ignoring and blocking accessing my midi devices.
6. Go back to the first tab and click play

If I reload the page the browser crashes, faced this every time.

Comment 22

[Gabriele Svelto [:gsvelto]]

Thanks, that's the same I saw. I need to fix bug 1748643 before we can uplift this.

Comment 23

[Gabriele Svelto [:gsvelto]]

Comment on attachment 9263082 [details]
Bug 1750763 - Honor the timestamps on outgoing MIDI messages r=padenot

Cancelling approval request pending a fix for bug 1748643 which needs to land first.

Comment 24

[Hani Yacoub, Desktop QA]

I didn't reproduce the crash anymore, but after following the steps from comment 21 there is no sound being played.
Gabriele, could you please take a lot at this?
Thanks.

Comment 25

[Karim Ratib]

I didn't reproduce the crash anymore, but after following the steps from comment 21 there is no sound being played.

Running FF 99.0a1 (2022-02-18) (64-bit) on Ubuntu 21.10, I do get sound. The DX7 emulator is a bit tricky: even if the page shows the MIDI device on loading, you still need to open the dropdown and close it for it to take effect.

Comment 26

[Gabriele Svelto [:gsvelto]]

I second what Karim said, manually select the device on the DX7 page. It shows a device as the default but it doesn't seem to open it until you interact with the menu yourself.

Comment 27

[Hani Yacoub, Desktop QA]

Thanks for the details.
Verified as fixed on Ubuntu 20.04 x64.