Midi Mapping Wizard maps to clock signal

Bug #1176184 reported by Michael Sawyer
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mixxx
Fix Released
Low
Michael Sawyer

Bug Description

In the midi mapping wizard, when I try to map signals from my Korg KP3, it receives the clock signal and thinks that everything should be mapped to the clock signal. I have patched the portmidicontroller.cpp to ignore the clock signal, but it would be better to ignore this signal in the midi mapping wizard rather than ignoring it all together.

RJ Skerry-Ryan (rryan)
Changed in mixxx:
status: New → Confirmed
importance: Undecided → Low
tags: added: midi midilearn
tags: added: easy
Revision history for this message
Michael Sawyer (michaelsawyer92) wrote :

I believe ignoring the clock signal in portmidicontroller.cpp is the best solution. Works for me.
Can someone confirm that it does not ignore the clock signal all together, just in the midi mapping wizard?

Changed in mixxx:
assignee: nobody → Michael Sawyer (michaelsawyer92)
Revision history for this message
RJ Skerry-Ryan (rryan) wrote : Re: [Bug 1176184] Re: Midi Mapping Wizard maps to clock signal

This makes it impossible to intentionally map the clock though. I think
this should be done at the MIDI learning level instead -- MIDI learning in
particular should ignore clock signals.

On Thu, Sep 19, 2013 at 5:05 PM, Michael Sawyer
<email address hidden>wrote:

> ** Patch added: "portmidicontroller.cpp patch to ignore clock signal"
>
> https://bugs.launchpad.net/mixxx/+bug/1176184/+attachment/3830295/+files/portmidicontroller.patch
>
> --
> You received this bug notification because you are a member of Mixxx
> Development Team, which is subscribed to Mixxx.
> https://bugs.launchpad.net/bugs/1176184
>
> Title:
> Midi Mapping Wizard maps to clock signal
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/mixxx/+bug/1176184/+subscriptions
>

Changed in mixxx:
status: Confirmed → In Progress
Revision history for this message
Michael Sawyer (michaelsawyer92) wrote :

This patch does indeed fix the midi mapping wizard assigning controls to the midi clock signal.

Revision history for this message
Michael Sawyer (michaelsawyer92) wrote :

Can someone confirm the above patch, and possibly commit fix?

Revision history for this message
Daniel Schürmann (daschuer) wrote :

Hi Michael,

Thank you for the patch. It looks like it will work now, but I do not fully understand the issue.

I hope you can help me with some general questions:
Does your controller periodically emit 0xF8? What is the use for it? How often does your emit this message?
Can you configure the rate? Is 0xF8 a midi standard clock signal or controller specific?

Please define 0xF8 by a macro to make the code more descriptive.
It would be nice if you can put a brief explanation into the source as well so that other contributors will understand you change when reading the code later.

Before committing your patch, you have to become a Mixxx contributor. Please sign https://docs.google.com/a/mixxx.org/spreadsheet/viewform?formkey=dEpYN2NkVEFnWWQzbkFfM0ZYYUZ5X2c6MQ and comment here when done.

Thank you

Daniel

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

http://en.wikipedia.org/wiki/MIDI_beat_clock

0xF8 is indeed the standard MIDI clock message value. The patch LGTM though
I agree it would be nice if it were a macro rather than a literal 0xF8. We
may also want to ignore other MIDI realtime messages (0xF8 through 0xFF)
since it's rare someone would want to MIDI learn them and they can occur at
any moment.

See the "System Real-Time Messages" section:
http://www.midi.org/techspecs/midimessages.php

Thanks Michael!

On Tue, Sep 24, 2013 at 4:24 AM, Daniel Schürmann <
<email address hidden>> wrote:

> Hi Michael,
>
> Thank you for the patch. It looks like it will work now, but I do not
> fully understand the issue.
>
> I hope you can help me with some general questions:
> Does your controller periodically emit 0xF8? What is the use for it? How
> often does your emit this message?
> Can you configure the rate? Is 0xF8 a midi standard clock signal or
> controller specific?
>
> Please define 0xF8 by a macro to make the code more descriptive.
> It would be nice if you can put a brief explanation into the source as
> well so that other contributors will understand you change when reading the
> code later.
>
> Before committing your patch, you have to become a Mixxx contributor.
> Please sign
>
> https://docs.google.com/a/mixxx.org/spreadsheet/viewform?formkey=dEpYN2NkVEFnWWQzbkFfM0ZYYUZ5X2c6MQ
> and comment here when done.
>
> Thank you
>
> Daniel
>
> --
> You received this bug notification because you are a member of Mixxx
> Development Team, which is subscribed to Mixxx.
> https://bugs.launchpad.net/bugs/1176184
>
> Title:
> Midi Mapping Wizard maps to clock signal
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/mixxx/+bug/1176184/+subscriptions
>

Revision history for this message
Michael Sawyer (michaelsawyer92) wrote :

Hey guys thanks for the response, I see two ways of doing this, one with a macro and one with a helper function. The second version would be better for future development on using the clock signal, for example, if someone wanted to utilize all midi clock signals in the range of 0xF8 to 0xFF.

Also correct me if I'm wrong, but doesnt midiKey.key & 0xF8 == 0xF8 mean that as long as the first 5 bits are set it will return true. So 0xF8 through 0xFF will be handled by this?

Unfortunately I could not get the second method to work, so using a macro it is! Patch attached.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

Hi Michael,

thank you for your work. And sorry for being not precise enough and putting additional work on you.

My request was much simpler, like this:

#define MIDI_SYS_RT_MSG_MASK 0xF8 // from MIDI Message Table http://www.midi.org/techspecs/midimessages.php

// Ignore all standard MIDI System Real-Time Messages because the are continuously send and
// preventing mapping of the pressed key
if ((mappingKey.key & MIDI_SYS_RT_MSG_MASK) == MIDI_SYS_RT_MSG_MASK) {
    return;
}

But I like your helper function Idea es well:

#define MIDI_SYS_RT_MSG_MASK 0xF8 // from MIDI Message Table http://www.midi.org/techspecs/midimessages.php

bool MidiController::isClockSignal(MidiKey &mappingKey) {
   if ((mappingKey.key & MIDI_SYS_RT_MSG_MASK) == MIDI_SYS_RT_MSG_MASK) {
        return true;
    }
    return false;
}

...

// Ignore all standard MIDI System Real-Time Messages because they are continuously send and
// preventing mapping of the pressed key
if (isClockSignal(mappingKey) {
    return;
}

isClockSignal(MidiKey &mappingKey) must be declared in midicontroller.h, than it should work.
And please not that there should be a blank between if and (

Please prepare a final patch based on this comment and do not forget to sign the Mixxx Contributor Agreement from comment #6.

Thank you,

Daniel

Revision history for this message
Michael Sawyer (michaelsawyer92) wrote :

Hey thanks Daniel! And no worries, coding is fun. I thought I had signed it but I will sign it again. And thanks for the pointer (pun intended), I forgot to pass mappingKey by reference so it was not working correctly. I will compile, run, and test once I get home then I will submit the working patch.

Revision history for this message
Michael Sawyer (michaelsawyer92) wrote :

Created new helper function to deal with midi clock signals, and ignores these during midi mapping wizard.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

Thank you very much!
I have committed the patch as https://github.com/mixxxdj/mixxx/commit/72201ee62ab12644a78412fe980e4cebff9326ce
1.12.0-alpha-pre (build master r3563)

Changed in mixxx:
status: In Progress → Fix Committed
milestone: none → 1.12.0
RJ Skerry-Ryan (rryan)
Changed in mixxx:
status: Fix Committed → Fix Released
Revision history for this message
Swiftb0y (swiftb0y) wrote :

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/7013

lock status: Metadata changes locked and limited to project staff
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.