Skip to content

Add KeyComparisonEffect builtin effect - #16066

Merged
daschuer merged 14 commits into
mixxxdj:mainfrom
Swarnadip-Kar:key-comparison-effect
Mar 6, 2026
Merged

Add KeyComparisonEffect builtin effect#16066
daschuer merged 14 commits into
mixxxdj:mainfrom
Swarnadip-Kar:key-comparison-effect

Conversation

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor

Summary

Adds a new builtin effect Key Comparison that plays a
pitched piano note on every downbeat, letting the DJ match it
by ear to identify or verify the musical key of a track.

How it works

The effect embeds a mono A4 (440 Hz) piano note as a static
constexpr CSAMPLE array (same pattern as the metronome
click). On each buffer the note is played back at a pitch
ratio derived from the selected key and concert pitch:

pitchRatio = 2^(keySemitones / 12) × (tuningHz / 440)

Parameters

Knob Range Default Notes
Key C – B (0–11) A Discrete, snaps to semitone steps
Tuning 415 – 466 Hz 440 Hz Matches concert pitch of the track
BPM 60 – 200 120 Active only when Sync is off
Sync toggle on Locks note to detected beat grid
Gain −24 – +3 dB 0 dB Linked to the metaknob

Files

  • pianosample.h / .cpp — embedded A4 sample recorded from
    a real piano, generated via tools/generate_piano_sample.py --input <wav> (scipy resampling)
  • keycomparisoneffect.h / .cpp — the effect implementation

Testing and Screenshot

Screenshot

Tested with Sync on and off, at multiple sample rates
(44100 / 48000), and with the Key knob across all 12 steps.

Fixes: #16034

Recorded from a real piano, loaded via the --input option
of tools/generate_piano_sample.py (scipy) and embedded as
constexpr CSAMPLE arrays for 44100, 48000, and 96000 Hz.
Avoids a runtime file dependency and matches the pattern
used by the metronome click sample.

The generator script can be re-run with a different WAV
file, or without --input to fall back to synthesis.
Plays a pitched A4 piano note on every downbeat so a DJ can
match it by ear to identify or verify the musical key of a
track.

Parameters:
- Key: discrete chromatic selector (C–B, 12 steps)
- Tuning: concert pitch in Hz (415–466, default 440)
- BPM: note rate when Sync is off (60–200)
- Sync: lock note to detected beat grid
- Gain: note volume in dB; linked to the metaknob

Pitch is derived as 2^(semitones/12) * (tuningHz/440),
keeping key steps and concert-pitch adjustment independent.
Adds pianosample.cpp and keycomparisoneffect.cpp to the
mixxx-lib target so the new effect is compiled and linked.
@github-actions github-actions Bot added effects build developer experience Issues, bugs and PRs related to the development process, development environment & developer docs labels Feb 28, 2026
@daschuer

daschuer commented Mar 1, 2026

Copy link
Copy Markdown
Member

The code works already great. Thank you for piking up the nice addition.

However have hard times to actually use it for detecting the key. Because it is "hammering" on each beat.
Interestingly it is already very useful to find the tuning of the track. But there are a few tones that fit as such a pilot tone to certain key.

As musician, do you find it useful in the current state? What do you need to make it useful?

Normally I would press the key manually on the down beat. Can we have manual trigger button (an actual piano key)?
It is also hard to use/listen to the higher tones, I would use an octave or even two below. What do you think?

We may also have three trigger buttons, on to stay on the tone, one to step up and one to step down. This way we can more easy step through the scale.

The BPM scale could be divided by 4, or an addition al button that allows to pick 1/4 BPM or other factors for common measures.

These are all improvements. Not necessary for the first merge. I need to review the code.

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

As musician, do you find it useful in the current state? What do you need to make it useful?

Thanks Daniel! Really appreciate the detailed feedback.

As a musician — agree it's more useful for tuning right now than key detection. The "hammering" is the main issue; matching a note to a moving track by ear works better when you control the timing yourself.

One thing worth noting: with Sync off, the effect on/off button already behaves roughly like a piano key — you can tap it on the downbeat to hear a single note, then release. Not perfect, but it's a usable workaround today.

For future improvements, a few ideas building on what you suggested:

  • Beat multiplier knob — play every 2, 4, or 8 beats instead of every beat. Most useful at 4× since chords typically land every 4 beats. This feels like the highest-priority improvement for musical usefulness.
  • Chord mode — load samples for all 12 semitones and add a Chord parameter that triggers the correct set of notes for a given key (e.g. major triad = root + 4st + 7st). Would make key matching much faster since a chord is more distinctive than a single tone.
  • Octave knob — shift the note down 1–2 octaves. Lower tones are easier to compare against bass and kick, which is where key is most audible in a mix.

Happy to tackle the multiplier knob as a follow-up PR after this merges, and keep chord mode as a longer-term idea. Let me know what direction makes sense.

@daschuer

daschuer commented Mar 1, 2026

Copy link
Copy Markdown
Member

Yes, that would be a nice addition.
Can you make the "BPM divider" or "measure" knob usefull for all reasonable downbeat distances? Some track are in 3/4 or 5/4 time.

Adds a Measure knob (-8 to +8) to control how often the note
fires relative to the beat:
  +N = N notes per beat
   0 = every beat
  -N = every N beats

This makes the effect usable across different time signatures
(e.g. -3 for 3/4, -5 for 5/4) without changing BPM.
@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

Yes, that would be a nice addition. Can you make the "BPM divider" or "measure" knob usefull for all reasonable downbeat distances? Some track are in 3/4 or 5/4 time.

just pushed a Measure knob that covers this.

It runs from -8 to +8:

  • Positive values subdivide the beat (+4 = 4 notes per beat)
  • 0 = every beat (current behaviour)
  • Negative values skip beats (-3 = every 3 beats, -5 = every 5 beats)

So 3/4 time -> set to -3, 5/4 -> set to -5. Default is -4 which I find a lot better for actually listening and comparing.

@Eve00000

Eve00000 commented Mar 1, 2026

Copy link
Copy Markdown
Contributor

I agree with Daniels findings,

  • why can't we start with the analyser-detected key (we'd like to check if the analyser is correct)?
  • I have a problem with reading the values (small text) maybe eg a KEY/BPM display/in tooltip would be nice, I'd prefer to see the actual KEY instead of a value (I know .....)
  • what should we do with the 'result' ?

@Swarnadip-Kar

Swarnadip-Kar commented Mar 1, 2026

Copy link
Copy Markdown
Contributor Author
  • I have a problem with reading the values (small text) maybe eg a KEY/BPM display/in tooltip would be nice, I'd prefer to see the actual KEY instead of a value (I know .....)
  • what should we do with the 'result' ?

Great points Eve!

  • why can't we start with the analyser-detected key (we'd like to check if the analyser is correct)?

I will put the Analyser-detected key as default— really good idea, hadn't thought of that. If the track's key metadata is available, pre-loading the Key knob to match it would make the workflow "confirm or correct" rather than "find from scratch". I'll look into how other effects read track metadata and see if it's feasible.

Display — agreed, showing a number 0–11 is not musician-friendly. I wanted to show the actual note name (C, C#, D...) but couldn't figure out how to map the integer knob value to a string label through the effect API. If someone knows how to do this I'd love a pointer, otherwise happy to open a follow-up issue.

What to do with the result — interesting question. The simplest thing is probably nothing — the DJ just uses what they hear to inform their mix decisions (key-compatible track selection, transposing via pitch fader). But if the analyser key was wrong, it'd be nice to be able to write the corrected key back to the track metadata. Is there a pattern for effects writing back to track properties in Mixxx, or is that outside scope here?

@acolombier acolombier left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. Some initial pre-requisite before we can start considering adding the contrib to the code base.

Comment thread src/effects/backends/builtin/pianosample.cpp
@daschuer

daschuer commented Mar 1, 2026

Copy link
Copy Markdown
Member

The mixxx binary will still contain the binary blob. Maybe we can use a lossy compression format? Or one with a low samplerate? This way the user can easily change the sample file to one of their favorite instrument.

I don't think we want to depend Mixxx on Python for the Synthesizer. Is there a c++ alternative that can do it?

Using python outside Mixxx or during build time works for me.

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author
  • why can't we start with the analyser-detected key (we'd like to check if the analyser is correct)?

On the analyser-detected key — I looked into this but couldn't figure out how to read the track's detected key from inside the effect. If someone can point me in the right direction I'm happy to add it.

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

The mixxx binary will still contain the binary blob. Maybe we can use a lossy compression format? Or one with a low samplerate? This way the user can easily change the sample file to one of their favorite instrument.

I don't think we want to depend Mixxx on Python for the Synthesizer. Is there a c++ alternative that can do it?

Using python outside Mixxx or during build time works for me.

For now I'm going with a synthesised note in C++ — It's not a real piano sound but it's accurate enough for pitch matching.

Does this approach work for you?

The embedded constexpr float array was 282k lines which is too large
to commit. Replace it with additive harmonic synthesis at init time:
9 harmonics of A5 with per-harmonic exponential decay, linear attack
ramp, and peak normalisation. The sample is generated once in
audioParametersChanged() and cached in the group state, so there is
no cost in the audio thread.

Also removes the Python generation script, which is no longer needed.
@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

Pushed a fix for the binary blob issue — replaced the 282k line generated file with C++ additive synthesis (9 harmonics with exponential decay). No external dependencies, nothing committed that shouldn't be. The sample generates once at init time and is cached in the group state.

The Python script is also removed since it's no longer needed.

One thing worth noting for later: since the synthesis is isolated in generatePianoSample(), it would be straightforward to let users supply their own audio file in the future (a flute, a sine wave, whatever they prefer for ear training). Could be a nice follow-up.

@daschuer

daschuer commented Mar 1, 2026

Copy link
Copy Markdown
Member

Great update. Thank you.

To be honest I am still struggling to use it effectively. My original Idea was to have a replacement for my wife's piano that allows me to change the pitch easily.

Found this video that roughly matches my approach: https://www.youtube.com/watch?v=2kqKT83aXIM

I think I need a bit training with this plugin to suggest improvements. Does it work for you now?

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

Great update. Thank you.

To be honest I am still struggling to use it effectively. My original Idea was to have a replacement for my wife's piano that allows me to change the pitch easily.

Me Too.

Found this video that roughly matches my approach: https://www.youtube.com/watch?v=2kqKT83aXIM

I think I need a bit training with this plugin to suggest improvements. Does it work for you now?

Well, being in the keyboard business for about 8 years I can say what's shown in the video (guessing the key just by listening) is definitely possible — though I can't do it myself that quickly. Sitting in front of my keyboard I could probably figure out the notes of a song in around 10 minutes with a bit of trial and error. I think people with highly trained ears would get the most out of this!

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

@acolombier @daschuer — just checking in. Happy to address any further feedback if there's anything blocking review.

@daschuer

daschuer commented Mar 5, 2026

Copy link
Copy Markdown
Member

The piano sound starts with bit of a delay:

Here is a recording when playing the key and the metronome plug-in together:

grafik

The small peak in front of the piano sample is the metronome.
I think they should be aligned. Is this a build up in the piano sample?

@daschuer

daschuer commented Mar 5, 2026

Copy link
Copy Markdown
Member

Is the sample off tune? Or my 440 Hz sine generated from Audacity?
There should be no such wobbly interferences.

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

I think they should be aligned. Is this a build up in the piano sample?

Is this the synthetic one or the original sample one i used? (Also which approach to take? - synthetic genration or actual sample - I wil align the smaple if required)?

@daschuer

daschuer commented Mar 5, 2026

Copy link
Copy Markdown
Member

Is the sample 400 Hz?

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

While working on this PR I noticed that both the metronome and KeyComparison gain knobs load at -24 dB regardless of the default value set in setRange. For example:
gain->setRange(-24.0, -5.0, 3.0); // -5.0 should be the default
Both effects load at -24 dB instead. Should I raise a separate issue for this? My PR closely follows metronomeeffect.cpp so both have the same problem : ) .

Separate resampling from the stereo mix so SampleUtil::addMonoToStereoWithGain
can be vectorized. Add tempMono buffer to group state to avoid audio thread allocation.
@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

Her my code review finding.

I am really impressed that you picked this not easy task up and implemented it such fast. I will give it a final test soon. Well done :-)

Thank you 😁! Most of the structure was adapted from metronomeeffect.cpp — studying that file closely is what helped me understand the patterns. I started this during my post-exam break so had time to dig into many new things..

@Swarnadip-Kar
Swarnadip-Kar requested a review from daschuer March 5, 2026 16:45
@daschuer

daschuer commented Mar 5, 2026

Copy link
Copy Markdown
Member

Two remaining issues:

  • Key and tuning knobs are cracking
  • The effect does not start with a down-beat. In case of a 4/4 measure the beat is played after 3 beats. It should be immediacy play the sound when starting the effect on a down-beat.

@daschuer

daschuer commented Mar 5, 2026

Copy link
Copy Markdown
Member

Can we have 13 Keys? From C to C?

@daschuer

daschuer commented Mar 5, 2026

Copy link
Copy Markdown
Member

But i can confirm it works now. I am actually able to find the key or the neighbour in the key wheel for some tracks without cheating.

ChangingpitchRatio mid-note no longer jumps the read position and causes a crack.

Now first beat fires immediately on the
downbeat rather than after a full measure.
@Swarnadip-Kar

Swarnadip-Kar commented Mar 6, 2026

Copy link
Copy Markdown
Contributor Author

Two remaining issues:

  • Key and tuning knobs are cracking
    immediacy play the sound when starting the effect on a down-beat.

Hello @daschuer @acolombier

Fixed both — crackling was caused by recomputing the source position from framesSinceLastNote * pitchRatio each buffer, so changing the Key or Tuning knob mid-note caused a position jump. Now tracking position directly in m_srcFramePos and advancing smoothly.

  • The effect does not start with a down-beat. In case of a 4/4 measure the beat is played after 3 beats. It should be

First-beat fix was just initialising m_beatCount to measure - 1 on enable.

Can we have 13 Keys? From C to C?

Also extended the Key range to 13 (C to C). I think an Octave knob will also be good (Should I implement here or save that for a follow-up PR - The chords one I will try)?

@daschuer

daschuer commented Mar 6, 2026

Copy link
Copy Markdown
Member

Let's bring this in. I will do a final test.

All other improvements depends a bit on you as pianist. I am none. My wife is (does it too little ...)

So my old ideas are probably not too significant
What do you need to be better supported?

I notice when coming from a lower key, the one key before the matching sounds the worst. The matching key sounds best, but like other neighbours in the key wheel.
The tiny knobs are a bit fiddly, but my controller has a pad to enable/disable and a super knob. I can map it to tuning first and than to key.

Does this suite as general usage instructions?

We may also introduce an enum knob, to show the keys explicit.

Mixxx does also support different key notations. Maybe we should also conditioner how to show them.

...

@daschuer

daschuer commented Mar 6, 2026

Copy link
Copy Markdown
Member

I just did a test and the instant start does still not work. In comparison with the Metronome, I can "click" immediately, while this PR does start with silence. We may also consider that the user is a spit of a second too late when sync is enabled to be on the down beat. It would be nice to consider that case as well to make it more easy to start the effect on a down-beat.

The instant start is a merge blocker for me the mentioned snapping (like quantized play) is probably advanced and can be postponed.

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

We may also introduce an enum knob, to show the keys explicit.

Mixxx does also support different key notations. Maybe we should also conditioner how to show them.

I looked into the enum knob idea before too but couldn't find a way to show letter labels in the existing effect parameter UI — all the effects I checked only show numbers. Showing key names (and respecting Mixxx's key notation settings) would probably need some rework at the parameter display level. Happy to look deeper if that's the direction, or treat it as a follow-up.

@Swarnadip-Kar

Swarnadip-Kar commented Mar 6, 2026

Copy link
Copy Markdown
Contributor Author

I just did a test and the instant start does still not work. In comparison with the Metronome, I can "click" immediately, while this PR does start with silence. We may also consider that the user is a spit of a second too late when sync is enabled to be on the down beat. It would be nice to consider that case as well to make it more easy to start the effect on a down-beat.

The instant start is a merge blocker for me the mentioned snapping (like quantized play) is probably advanced and can be postponed.

Sorry, I think I may have misunderstood — could you clarify what "instant start" means here? When I tested, pressing the button fires a note on the immediate next beat, and the metronome seems to behave the same way. If you enable it just after a beat passes, it will wait until the next one — is that the silence you're referring to?
(if enabled just beofre the beat is about to hit - it will actually start on that beat itself. - On the "split second too late" snap — I'd rather keep that as a follow-up, agreed it's advanced.)

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

All other improvements depends a bit on you as pianist. I am none. My wife is (does it too little ...)

So my old ideas are probably not too significant What do you need to be better supported?

I notice when coming from a lower key, the one key before the matching sounds the worst. The matching key sounds best, but like other neighbours in the key wheel.

On chords — as a pianist I think that's actually the most intuitive upgrade. Notes one semitone away from the chord clash harshly regardless of key, while chord tones blend even if you're slightly off. A chord mode (playing a triad instead of a single note) would make matching much faster. Happy to work on that after this is merged.

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

The tiny knobs are a bit fiddly, but my controller has a pad to enable/disable and a super knob. I can map it to tuning first and than to key.

Does this suite as general usage instructions?

Yes that sounds like a natural workflow — start with the Sync button on the pad, then use the super knob on Key to find the right semitone, and once you're close switch it to Tuning to fine-tune the cent offset. As a pianist that's roughly how I'd approach it by ear too. That said, tracking exact tuning by ear is quite hard even for trained musicians — I'd personally rely on Mixxx's key detection for the tuning value rather than dialling it in by ear. -
But I also Think that Gain should also be easily accesible - (sometines higher volume to listen the note properly relative to the song helps)

@daschuer

daschuer commented Mar 6, 2026

Copy link
Copy Markdown
Member

The "enum knob idea" is a whole follow up project. We need also look into the external effect APIs whether such T´things are supported there. I would do it in a bigger context of Improving the Effect UI. This is only one shortcoming of more.

Sorry, I think I may have misunderstood — could you clarify what "instant start" means here? When I tested, pressing the button fires a note on the immediate next beat, and the metronome seems to behave the same way.

I have tested the "Sync off" state. It is notable the best if you compare metronome and this effect.

If you enable it just after a beat passes, it will wait until the next one — is that the silence you're referring to?

Probably as well. This behavior is kind of OK for measure = 1. But it feels wrong for measure = 4. A poor mans solution would be to wait 4 beats instead, if you are a bit behind, lets say in the first quarter after the beat. Would be already helpful for me.

@daschuer

daschuer commented Mar 6, 2026

Copy link
Copy Markdown
Member

On chords — ... Happy to work on that after this is merged.

Sound great. I am curious.

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

The "enum knob idea" is a whole follow up project. We need also look into the external effect APIs whether such T´things are supported there. I would do it in a bigger context of Improving the Effect UI. This is only one shortcoming of more.

Sorry, I think I may have misunderstood — could you clarify what "instant start" means here? When I tested, pressing the button fires a note on the immediate next beat, and the metronome seems to behave the same way.

I have tested the "Sync off" state. It is notable the best if you compare metronome and this effect.

If you enable it just after a beat passes, it will wait until the next one — is that the silence you're referring to?

Probably as well. This behavior is kind of OK for measure = 1. But it feels wrong for measure = 4. A poor mans solution would be to wait 4 beats instead, if you are a bit behind, lets say in the first quarter after the beat. Would be already helpful for me.

Thanks for clarifying — in Sync off mode the note should fire immediately on enable like the metronome, will fix that.

For Sync on with measure=4, the "first quarter" snap sounds like a good pragmatic solution — I'll look into that
Should the "first quarter" snap for Sync on also be included in this PR or treat it as a follow-up?
On chords — will start exploring after this is merged!

@daschuer

daschuer commented Mar 6, 2026

Copy link
Copy Markdown
Member

Should the "first quarter" snap for Sync on also be included in this PR or treat it as a follow-up?

If you have a solution at hand. It would be nice to have it here, otherwise it can also be postponed.

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

If you have a solution at hand. It would be nice to have it here, otherwise it can also be postponed.

Okay. I will try to implement in this PR itself..

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

Fixed both — in unsynced mode the note now fires immediately on enable. In sync mode, if the effect is enabled within the first quarter of a beat, it snaps back and fires immediately too so a split-second-late press still lands on the downbeat. Otherwise it waits for the next beat as before.

Comment on lines +27 to +35
std::vector<CSAMPLE> pianoSample;
// Temporary mono buffer for the resampling step. Sized to framesPerBuffer
// in audioParametersChanged so no allocation happens on the audio thread.
std::vector<CSAMPLE> tempMono;
// Tracks position in the piano sample in source frames. Stored as double
// so that changing pitchRatio mid-note does not cause a position jump and
// the resulting crack.
double m_srcFramePos = 0.0;
std::size_t framesSinceLastNote = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for all this forth and back. Since we now want m_ prefix, it is missing here:

Suggested change
std::vector<CSAMPLE> pianoSample;
// Temporary mono buffer for the resampling step. Sized to framesPerBuffer
// in audioParametersChanged so no allocation happens on the audio thread.
std::vector<CSAMPLE> tempMono;
// Tracks position in the piano sample in source frames. Stored as double
// so that changing pitchRatio mid-note does not cause a position jump and
// the resulting crack.
double m_srcFramePos = 0.0;
std::size_t framesSinceLastNote = 0;
std::vector<CSAMPLE> m_pianoSample;
// Temporary mono buffer for the resampling step. Sized to framesPerBuffer
// in audioParametersChanged so no allocation happens on the audio thread.
std::vector<CSAMPLE> m_tempMono;
// Tracks position in the piano sample in source frames. Stored as double
// so that changing pitchRatio mid-note does not cause a position jump and
// the resulting crack.
double m_srcFramePos = 0.0;
std::size_t m_framesSinceLastNote = 0;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — added m_ prefix to pianoSample, tempMono, and framesSinceLastNote. Sorry for missing those earlier. Thank you for the patience through all the back and forth — I've learned a lot through this PR, from vectorization and audio thread constraints to the finer points of C++ conventions. Really appreciate it!

@Swarnadip-Kar
Swarnadip-Kar requested a review from daschuer March 6, 2026 15:08

@daschuer daschuer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Green, Juchu!

@daschuer
daschuer merged commit 67fbbfc into mixxxdj:main Mar 6, 2026
17 checks passed
@Swarnadip-Kar
Swarnadip-Kar deleted the key-comparison-effect branch March 28, 2026 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build developer experience Issues, bugs and PRs related to the development process, development environment & developer docs effects needs review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

key comparsion effect like metronome

5 participants