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 a rounding issue is BpmControl::getBeatContext #11263

Merged
merged 4 commits into from
Feb 11, 2023

Conversation

daschuer
Copy link
Member

This causes a DEBUG_ASSERT in Beats::iteratorFrom()

const mixxx::audio::FramePos otherPositionOfThisNextBeat = otherPosition +
thisSecs2ToNextBeat * otherBeats->getSampleRate() *
pOtherEngineBuffer->getRateRatio();
// This original version has double precision issues (we kept it for documentation):
Copy link
Member

Choose a reason for hiding this comment

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

Instead of leaving the old code as comment, src/test/bpmcontrol_test.cpp should be extended by a testcase that detects the precision issue.

@daschuer
Copy link
Member Author

The test took me way more time than the fix because it was really hard to artificially trigger the asserts because it is really a LSB double issue. But OK now the test is in place.

Copy link
Member

@JoergAtGithub JoergAtGithub left a comment

Choose a reason for hiding this comment

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

LGTM! Waiting for CI.

@JoergAtGithub JoergAtGithub merged commit 7c70cee into mixxxdj:2.4 Feb 11, 2023
@JoergAtGithub
Copy link
Member

Thank you!

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