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

MIDI out sending double clocks when unit itself is receiving MIDI clocks #164

Closed
DavidMenting opened this issue Jan 15, 2024 · 5 comments · Fixed by #172
Closed

MIDI out sending double clocks when unit itself is receiving MIDI clocks #164

DavidMenting opened this issue Jan 15, 2024 · 5 comments · Fixed by #172
Assignees

Comments

@DavidMenting
Copy link
Member

A receiving DUO synced to a sending DUO which itself is receiving MIDI clock will put out double MIDI clocks: the ones it's receiving are passed on to the output and the ones it generates are also sent out of the output. This probably also applies to other manufacturers' devices.

The quick workaround is to connect the second DUO with a sync cable

@ggVGc
Copy link
Contributor

ggVGc commented Jan 15, 2024

I think this is something we should handle, and it should be a fairly quick change.
I will try putting together a small PR.

@ggVGc ggVGc self-assigned this Jan 15, 2024
@ggVGc
Copy link
Contributor

ggVGc commented Jan 16, 2024

After a closer look, the reason we're seeing this is because the MIDI library has passthrough enabled by default.
It's possible to add filter but not per message type: https://github.com/datomusic/duo-imxrt/blob/main/platform/brains2/components/arduino_midi_library/src/midi_Defs.h#L126

I think we have two options with different downsides:

1. Disable MIDI thru
This will fix the problematic behaviour, but has wider consequences of not being able to use the Duo as a repeater in a MIDI chain.

2. Conditionally handle MIDI output
If an action was caused by a message we react to (Clock or Note currently, I think), do not output MIDI message for it.
This requires more code changes, and feels problematic without yet having a more centralized place for MIDI-related things.
There's also the concern that we might still want to output a message to usbMIDI if the message came from serial, and vice versa.

The question is what is currently the expected behaviour. The way the firmware is written, it feels like the MIDI thru functionality is unexpected and unwanted.
In that sense, I believe the most reasonable change currently is to disable MIDI thru and leave the rest as is, and if we want to, we can add our own MIDI thru which is more selective in the filtering it does, letting through everything except what we generate such as Clock and NoteOn/Off.

@DavidMenting
Copy link
Member Author

MIDI passthrough is actually quite handy, as it allows the DUO to be used as a MIDI interface. Having the MIDI Thru mode configurable over Sysex might be a nice future upgrade, but for now there is a third option to solve double clocks that might be more pragmatic:

3. Don't output own MIDI realtime messages when receiving MIDI clock
We know the clock source, so we can use that to determine whether we send MIDI realtime messages or not. This prevents us from having to filter incoming messages. Also, this solves issue #162 by passing on either Start or Continue depending on the upstream qMIDI sender.

@ggVGc
Copy link
Contributor

ggVGc commented Jan 16, 2024

@DavidMenting Yeah, that was what I meant as option 2. But, then we need to decide if we still want to output Clock to usbMIDI when we receive it from serial, and vice versa.
Currently we output Start/Continue/Clock on both, always. If we rely on midi-thru, it will only be sent through on the interface where it came from.
So, if we always want it to go out over both, we still need to handle it and output on the other one.

@ggVGc
Copy link
Contributor

ggVGc commented Jan 22, 2024

I made a small fix for this specific issue in the #172 and made a new issue for the MIDI through stuff here #171

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 a pull request may close this issue.

2 participants