Second Qm rounding fix #2112
Second Qm rounding fix #2112
Conversation
This fixes a rounding error when using source sample rates not divisible by 8
After this change, the refence sine waves of a full half note are centered on the desired bin. This was not the case before.
|
|
||
| // std::cout << "fractional key pre-sorting: " << (maxBin + 2) / 3.0 << std::endl; |
There was a problem hiding this comment.
Uncomment this for the fractional key debug output.
| @@ -33,8 +33,8 @@ int Chromagram::initialise( ChromaConfig Config ) | |||
| m_BPO = Config.BPO; // bins per octave | |||
| m_normalise = Config.normalise; // if frame normalisation is required | |||
|
|
|||
| // No. of constant Q bins | |||
| m_uK = ( unsigned int ) ceil( m_BPO * log(m_FMax/m_FMin)/log(2.0)); | |||
| // No. of constant Q bins, extended to a full cotave | |||
| const double real = cos(angle); | ||
| const double imag = sin(angle); | ||
| const double absol = hamming(hammingLength, i)/hammingLength; | ||
| hammingWindowRe[ origin + i ] = absol*real; | ||
| hammingWindowIm[ origin + i ] = absol*imag; | ||
| } | ||
|
|
||
| /* This code splits the hanning widow and moves it to the beginning |
| @@ -268,8 +268,10 @@ int GetKeyMode::process(double *PCMData) | |||
| // m_Keys[1] is C center 1 / 3 + 1 = 1 | |||
| // m_Keys[4] is D center 4 / 3 + 1 = 2 | |||
| // '+ 1' because we number keys 1-24, not 0-23. | |||
| key = MathUtilities::getMax( m_Keys, 2* m_BPO, &dummy ) / 3 + 1; | |||
| int maxBin = MathUtilities::getMax( m_Keys, 2* m_BPO, &dummy ); | |||
Oh yes, I need to update the patch.
I have published the patch upstream, and on GitHub. I gut no response so far. There is also an other unmerged pending patch with no responds. I guess, we can considder the development stalled. |
|
The last upstream commit was on May 1 and there were more commits in February this year, so I wouldn't consider upstream development stalled yet. It has only been a few days since the upstream PR was opened. In the meantime, keeping a patch in our repository is fine. |
|
2018-05-01, last year. I don't see any significant progress since their last release in 2015: |
|
Only build fixes. No new features, no bug fixes. |
|
Done. |
|
LGTM. Thanks for working on this tricky DSP code! If the changes are not merged upstream we might think about forking the entire qm-dsp stuff and delete all files that we don't need. |
Here is another rounding fix.
It works really great now. All my test sine files are now only at the center bin, as expected.
This was not the case before.
Almost all tracks are always detected on a center bin as well. Exceptions are passages with vocal and drum solos, which is IMHO the expected result.
I have expetional tracks though. One of these interesting tracks is:
Moby - Bodyrock (Olav Basoskis Da Hot Funk Da Freak Funk Remix)
https://www.youtube.com/watch?v=oDJ35h_0xWs
It is detected as G Major. Played together with an other G Major track is sounds badly off pitch. So I guess it is. See below. With this fixed algorithm, we have a chance to evaluate the quality of the detection and remove the key value from such a track. I like the idea, instead of consider it compatible to any other track. In the case of this Moby track I would brefere to make it as off pitch.
What do you think?