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

Fix repeated key and beat analysis with default settings #2775

Merged
merged 1 commit into from
May 10, 2020
Merged

Fix repeated key and beat analysis with default settings #2775

merged 1 commit into from
May 10, 2020

Conversation

uklotzde
Copy link
Contributor

Found while analyzing #2773.

Ideally the settings should never return an empty value, but I won't fix this.

@uklotzde uklotzde added this to the 2.3.0 milestone May 10, 2020
@Be-ing Be-ing changed the base branch from master to 2.3 May 10, 2020 12:51
@Be-ing
Copy link
Contributor

Be-ing commented May 10, 2020

Thank you, this fixes the bug with some of my tracks mysteriously not having beats detected.

@Be-ing Be-ing merged commit 0b89edd into mixxxdj:2.3 May 10, 2020
@uklotzde
Copy link
Contributor Author

I noticed even more issues about how the beat analyzer decides when to execute the analysis and when finished how to store the results. But I'll stop at this point and leave the rewrite to someone else. It is to dangerous to touch this code without the ability to test all code paths.

@uklotzde uklotzde deleted the default_analysis_plugin_selection branch May 10, 2020 19:02
@Be-ing
Copy link
Contributor

Be-ing commented May 10, 2020

Uhhh yeah something is very wrong here... now when I do batch analysis, many files still don't have a BPM marked, and those that do all show the same value for BPM... 😕

@uklotzde
Copy link
Contributor Author

@be Did you try to reset the BPM of those tracks before analyzing them?

Tracks with an invalid BPM (<= 0) might not be re-analyzed depending on the settings and the database contents. This doesn't make sense to me. Those tracks should always be re-analyzed.

@uklotzde
Copy link
Contributor Author

Track that have been reset while the database column was missing need to be reset again.

@Be-ing
Copy link
Contributor

Be-ing commented May 10, 2020

Yes, I reset the BPM and beatgrid from the track context menu. I am experimenting with a fresh database now...

@uklotzde
Copy link
Contributor Author

Testing with a fresh database revealed no new issues.

@Be-ing
Copy link
Contributor

Be-ing commented May 10, 2020

Now zero tracks have a BPM detected with a fresh database... 😕

@Be-ing
Copy link
Contributor

Be-ing commented May 10, 2020

Nevermind, I was testing with this branch only. With the 2.3 branch including #2773, BPMs are showing up when analyzing with a fresh databse.

@uklotzde
Copy link
Contributor Author

uklotzde commented May 10, 2020

Those issues occur when resetting the bpm with the database column update missing. We could mitigate the issues for early testers by re-analyzing always if bpm <= 0.

@uklotzde
Copy link
Contributor Author

The commit in #2776 could fix some inconsistent database entries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants