Skip to content

Loopescape Test Fix #1491

Merged
uklotzde merged 2 commits intomixxxdj:2.1from
daschuer:loopescape
Jan 24, 2018
Merged

Loopescape Test Fix #1491
uklotzde merged 2 commits intomixxxdj:2.1from
daschuer:loopescape

Conversation

@daschuer
Copy link
Copy Markdown
Member

Try to fix the test fails reported in #1487 (comment)

@uklotzde
Copy link
Copy Markdown
Contributor

Still failing with a rounding error:

The difference between 200 and m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample() is 1, which exceeds kLoopPositionMaxAbsError, where
200 evaluates to 200,
m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample() evaluates to 199, and
kLoopPositionMaxAbsError evaluates to 1.0000000000000001e-09.
src/test/looping_control_test.cpp:554: Failure
The difference between 200 and m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample() is 1, which exceeds kLoopPositionMaxAbsError, where
200 evaluates to 200,
m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample() evaluates to 199, and
kLoopPositionMaxAbsError evaluates to 1.0000000000000001e-09.

@daschuer
Copy link
Copy Markdown
Member Author

Strange. Is this probably caused by this commit: af218bc ?
It looks like there is some unintended remaining integer rounding in place, which looses one sample. Maybe because the rounding error in the previous test. If this where an intentional rounding, it should probably round to an even value.

@uklotzde
Copy link
Copy Markdown
Contributor

@daschuer If I revert this commit the test succeeds.

@daschuer
Copy link
Copy Markdown
Member Author

I have got it. A "real" bug: seekInsideAdjustedLoop() has truncated the result to an int. Fixed.

@uklotzde
Copy link
Copy Markdown
Contributor

LGTM 👍 Waiting for the build server to finish.

@uklotzde uklotzde merged commit 812f512 into mixxxdj:2.1 Jan 24, 2018
@daschuer daschuer deleted the loopescape branch September 26, 2021 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants