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

Ironing beats #3626

Merged
merged 27 commits into from Mar 15, 2021
Merged

Ironing beats #3626

merged 27 commits into from Mar 15, 2021

Conversation

daschuer
Copy link
Member

@daschuer daschuer commented Feb 12, 2021

This continues #2930 and aims to solve the issues discussed in #3012 with ideas from #2668.

These are the raw beats of the input of "Isometric - Mood Swings" input of BeatUtils::calculateBpm
grafik

It is a const BPM track with 148 BPM, which shown perfectly how our current detector is broken for such EDM tracks.

The very first part at is an intro without good onsets. The result is floating. Than it hooks to the correct 148 BPM. However due to the resolution of the detector it needs shorter adjustment beats. They are floating, until an easy to detect beat starts. Here the adjustment beats are in an equal distance. In the rest of the track the detector get hocked to the 2/3 BPM value and floats around 98,66.

Currently I have implemented four stages to get a constant beat. This deals with the assumption that many tracks are at a full integer or integer fraction of 1/12 BPM.

  1. Iron the adjustment beats and return const regions that are not more than 25 ms off of the raw detected beat. This is way more than the quantization jitter of 12 ms, to also cover different onsets shapes of different instruments. This becomes our new non-const beat map.
  2. use the longest const region and find other regions in the same temp and phase. As longer the resulting region, as lower is the range of the possible BPM.
  3. Round BPM to a 1/12 BPM if the value is within the possible range, to ensure that no beat is more than 25 ms off off the original detected one.
  4. Adjust the phase, also only in the range of 25 ms.

Result: many track are at a full BPM and some at 1/2 fraction most hand made tracks are not rounded. Unfortunately according to these rules the Isometric track is still at the "wrong" 98,66 BPM because this region was the longest. For this track it would be easy to fix it aromatically. But I am afraid that this likely messes the track up, so I have not included this here.

With these steps we do not loose the phase information so there is no longer need for the offest adjustment preferences option.

The QM currently prefers tempos near 120 BPM. I guess this is the reason that the Isometric track is with 98,66 closer to 120 compared to 148 BMP. The additional BPM limits from the preferences just interferes with that assumption and was messing up good results, or good enough results that can be fixed by a manual click on the BPM multiplier buttons.
So I have removed them as well.
If it turns out that such a feature is required, we can pass the preferred BPM value into the detector.

I have done this on top of 2.3 because i consider the current rounding as buggy. However I did not expect such a size of a PR which should better go to 2.4. Since I am not an EDM DJ I do not suffer from the decimal places.

@poelzi Does this solve at lest some of your problems? Do you have tracks where this still fails?

daschuer and others added 11 commits February 5, 2021 21:58
Co-authored-by: Cristiano Lacerda <cristiano.lacerda@usp.br>
… during an offset shift that happens when the beat instrument changes.
A small Offset Correction in the range of +-25 ms is always applied. The Analyzer always returns the best Tempo near 120 BPM. This may fail by an interger fraction.
Applying an additional adjustment into a range will make the issue worse.
@github-actions github-actions bot added the ui label Feb 12, 2021
@daschuer
Copy link
Member Author

+336 −711 Nice, this removes more code than it introduces.

@Holzhaus
Copy link
Member

@daschuer Thanks you for working on this. You said this is continued the changing tempos PR (#2930) but there are no commits from @crisclacerda in this branch. Did you reimplement this from scratch?

@daschuer
Copy link
Member Author

daschuer commented Feb 12, 2021

The code in the first commit is from @crisclacerda

src/preferences/dialog/dlgprefbeats.cpp Outdated Show resolved Hide resolved
src/preferences/dialog/dlgprefbeats.cpp Outdated Show resolved Hide resolved
src/track/beatfactory.cpp Outdated Show resolved Hide resolved
src/track/beatfactory.h Outdated Show resolved Hide resolved
@Holzhaus
Copy link
Member

I tested this with my test track "The Learning Process" by T. Williams & James Jacob.

Non-const

For non-const beatgrids this gives still incorrect results, but they are a bit closer to the truth (120.04 vs 120.19 BPM, correct would be 120 BPM) and the beatgrid looks better aligned in this branch.

Upstream:
2 3-nonconst

This branch:
ironbeats-nonconst

The BPM detection for the part without audible beats is still wrong, but that's expected I think.

Const

Unfortunately, this has regressed considerably, at least for this track. BPM is correctly detected in both branches, but the beatgrid is shifted in this branch (by ~0.68 beats).

Upstream:
2 3-const

This branch:
ironbeats-const

@poelzi
Copy link
Contributor

poelzi commented Feb 12, 2021

I analyzed 4200 tracks and compared the results. Unfortunatelly the csv export seems to crop bpm values so it is hard to compare, but some manual tests showed that tracks detected as 2/3 become the correct, round value after correcting.
Unfortunately, more tracks are detected wrongly:
348 where detected faster on the old vs the new.
133 where detected faster on the new branch vs the old.

I think, if a segment is detected in a bpm in which bpm*3/2 is very close to a integer, we should take this bpm for this segment.

@Holzhaus
Copy link
Member

@poelzi what about beatgrid alignment? Is it worse, better or roughly equal? Did you use const beatgrid?

@daschuer
Copy link
Member Author

think, if a segment is detected in a bpm in which bpm*3/2 is very close to a integer, we should take this bpm for this segment.

Of case we can do it, however I am afraid there is no unambiguous rule? There is also a problem that we likely mess up the phase. Because only every third beat matched the detected beats.
To make this feature work, we need to concider the onset grid, detected in one of my other branches. A future topic.

What we can do is make the rounding of 1/12 BPM to a full integrer a preferences option.
But let's postpone it to another PR.

@Holzhaus
Copy link
Member

@daschuer do you have an idea why the phase is off for const beatgrids? The non-cost phrase is detected properly.

@daschuer
Copy link
Member Author

The cvs report is fixed here: #3632. A regression since #2001

@daschuer
Copy link
Member Author

@daschuer do you have an idea why the phase is off for const beatgrids? The non-cost phrase is detected properly.

This is caused by the different offset correction strategy. The detector detects phase shifts in the track due to the missing base drum in some passages. In this areas it gets hooked on the high heads. This is of cause wrong, but it is a known issue that the QM detector valuates the high heads more, because they have a wider spectrum.

Instead of using the first beat as phase giving indicator which turns out to be wrong on the majority of tracks in my collection, we use now the phase of the longest constant region. It seems that such a high head region is the longest region that is used set the phase for the whole track.

In my case the phase is aligned perfectly to the high heads in during the whole track. I cannot confirm the issue from the screen-shot. Maybe I have different recording?

grafik

@daschuer
Copy link
Member Author

I have now tested a different version which has the correct offset.

@daschuer
Copy link
Member Author

grafik

The blue curve is the non-const beat map of the 2.3 branch. The red curve is this branch. The dips in the curve are the places where the detector shifts the phase form the base drum to the high heads. In my case the region around 200 is the longest one.

@daschuer
Copy link
Member Author

@Holzhaus: Notes are addressed now.

Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

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

I didn't test rigorously, but I did not experience any regressions so far.

@uklotzde
Copy link
Contributor

@Holzhaus Merge?

Copy link
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

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

LGTM, thank you. A lot less unnecessary fluctuations in beatmap tracks!

@Holzhaus Holzhaus merged commit 2a330ca into mixxxdj:2.3 Mar 15, 2021
2.3 release automation moved this from Needs Review to Done Mar 15, 2021
@kek001
Copy link

kek001 commented Apr 21, 2021

Are you sure, this fix is done everywhere. It looks if mass analyze multiple tracks, the grid/wave are out of sync in some tracks. I have tested this, doing single analze without dropping left side analyze magic wand icon, multile tracks.

Windows 10. It can be, the fixed working method/function is not replaced, where mixxx do multiple track analyzes, and its using old not working procedures.

@daschuer
Copy link
Member Author

If you use the recent 2.3 beta, this PR is included in all cases.

By default Mixxx keeps the old analysis data. In the "Beat Detection" preferences you can select "Re-Analyze beats when settings change or beat detection data is outdated" to detect the beats again. Or reset the beat grid before analysis the be entirely sure it is new.

Do you use const or non const analyzer setting?
I have found recently a bug in the const code affecting long tracks, which is fixed here: #3794
The windows build can be found here:
https://github.com/mixxxdj/mixxx/suites/2524332177/artifacts/54690006

I would be happy to receive test results and get to know tracks that are still failing?

@kek001
Copy link

kek001 commented Apr 21, 2021

My current mixxx is 8126.
I found this thing, analysing a new tracks, and mass analysing them.
But when I am renanalysing, clearing grid and bpm, and loading track to the deck, and let it analyze the track, then grid and waveform is fine. and have to say, the grid locations is much better now, much more often the grid is right spot, and not need to fine tune.

So if mass analyze is using methods what you have fixed, then this is little bit odd, i will study this more. while checking a new tracks.

@daschuer
Copy link
Member Author

The old method is no longer in the code. It can be an issue though that you see the old analysis, done with a previous version.

@kek001
Copy link

kek001 commented Apr 21, 2021

I just listen and preparing a new tracks, what i mass analyzed yesterday.
This is real thing, the mass analysing not work. But if i re-analyze clearing waveforms and bpm grid, and load a track again, everything is fine. I mass analyzed yesterday using 8126 build for windows 10. which is dated 2021-04-12.
These tracks what i am preparing havent been in mixxx database day before yesterday. So they are not an old analyzer results.

@kek001
Copy link

kek001 commented Apr 21, 2021

the behaviour is the grid and waveform are fine first part of tracks, but about 1/2 - 2/3 of a track , shift happends.

@daschuer
Copy link
Member Author

Which track is it? Does #3794 fix the issue?

@kek001
Copy link

kek001 commented Apr 22, 2021

Allmost every track.
The #3794 is not related, remember if I do reanalyze a single track, the grid and waveform works well.

@daschuer
Copy link
Member Author

I probably have no understand the the issue.
You have really different results if you analyze a track via the mass analysis compared to the analysis in the deck?

Please test this:

  • Make a copy of an affected track. (Do NOT load the track into a deck or press the preview button)
  • Menübar -> Library -> Rescan library.
  • Change to Analyze view
  • Select the new track and some others
  • Press the analyze button and wait.
  • Analyze the copy with the mass analyzes
  • Load the Track into a deck
  • Make a hot-cue on a beat maker somewhere at the beginning and the end of the track
  • Right on the track title Reset -> Beat Grid and BPM
  • Load the track to another deck
  • Verify if the cue points are still at the beat markers

This is the case here using the latest Mixxx 2.3. Can you confirm that?

@kek001
Copy link

kek001 commented Apr 22, 2021

Yes If I drag drop multiple tracks to the analyz icons, i get grid/waveform placements wrong or they start shift.
So far its all new tracks what i have analyzed its not only one.

I have done most of the steps what you wrote.
After mass analyze grids are not a right place second half of the track, if i put hotcues ~ beging and ~ end.
second half and second hotcue is not correct place.

after clear waveform and bpm and and after loading a single track, both hotcues are correct place.

@kek001
Copy link

kek001 commented Apr 22, 2021

I havent use "analyze functions" if clicking the icon. I just let deck load the track and do the analyze, after clearing a waveform/bpm.

@daschuer
Copy link
Member Author

I have just tested the drag and drop analysis with a new duplicated track and cannot confirm the issue.
Maybe you see the Serato or Rekordbox beat grid? Do you use on of these?

There is an extra checkbox in the Preferences to discard their beat grid when importing in Mixxx.

@kek001
Copy link

kek001 commented Apr 22, 2021

No I dont use serato or rekordbox. I try to load some public track, so i can make link and images, you will see it better.
I am using windows 10, so if you are using linux, there may have a windows only thing.

@kek001
Copy link

kek001 commented Apr 22, 2021

Clearing BPM and Grid wont help, need to clear waveform. I dont know if this helps.

@kek001
Copy link

kek001 commented Apr 22, 2021

Thank you Daniel checking this thing.

I downloaded ektoplazm two albums, but offcourse, there was no probelm at all. Which is offcourse a great thing.

I was mass analysing over +200 tracks, where i saw this behaviour. I am not going to download +20 albums from ektoplazm, they server has slow speed .... etc.

Next week I have plan to buy similar ammount of tracks, and will do mass analysing, so if I can reproduce this.
Because i have allready prepare allmost all of those tracks what i got it, and not want mess with them.

What you mean latest Mixxx version,the version which is public available or latest build from server ?

@daschuer
Copy link
Member Author

I mean the latest beta version form the download page on www.mixxx.org.

For testing you can start with a new database an you current tracks. In order to this you can copy the start icon to the desktop and add –settingsPath PATH
To the start parameter.
PATH can be any path where a new library schould be stored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
2.3 release
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

8 participants