Loopescape Bugs

[daschuer]

This fixes:
https://bugs.launchpad.net/mixxx/+bug/1743885
And
https://bugs.launchpad.net/mixxx/+bug/1743010

[ronso0]

I will test this soon.

I forgot to report a bug that might be related:
tiny loops don't get activated
https://bugs.launchpad.net/mixxx/+bug/1744162

[ronso0]

I just tested the fixes, and I think I stressed it a lot with controller and GUI buttons.
1/32 beatloops via spinboox/loop_toggle still fail in less than 1 of 10 tests, though.
Activating a 1/16 loop, then halving it to 1/32 works 100% here.

[ronso0]

I can 100% reproduce a failing 1/32 beatloop (rolling or normal)

• if it's the first loop I try to activate after track load or
• if I try to set that loop at a play position before any other previously set (and working) loop

Still doesn't matter if I trigger beatlooproll_0.03125_activate or beatloop_size = 1/32 + beatloop_activate.
After that, it fails now & then for 1/32.

Edited

[daschuer]

Got it! Just a typo.

[ronso0]

Awesomecredible! Just tested again, 1/32 loops only.. And it works like a charm now!
Thanks!

[daschuer]

Thank you very much for testing. Does anyone wants to do a code review?

[uklotzde]

This commented out debug output is confusing, because m_oldLoopSamples == loopSamples after the assignment and why should we print the pTarget pointer? I think it is better to simply remove this commented out piece of code.

There is also a minor typo in another comment (line 390) above if you touch this file: "Ceck" -> "Check"

[uklotzde]

I don't understand each and every detail of the code, but with the additional comments the special case handling should be clear. Just one minor remark. I rely on @ronso0 for testing.

[uklotzde]

LGTM.

[uklotzde]

@daschuer I should have run the tests locally before merging:

src/test/looping_control_test.cpp:536: Failure
Value of: m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()
  Actual: 199
Expected: 200
src/test/looping_control_test.cpp:550: Failure
Value of: m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()
  Actual: 199
Expected: 200
in ~EngineMaster() 
[  FAILED  ] LoopingControlTest.LoopMoveTest (39 ms)
[----------] 1 test from LoopingControlTest (39 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (39 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] LoopingControlTest.LoopMoveTest

Looks like a rounding error/deviation?

[ronso0]

Could I have noticed that fail when testing manually?
Because, actually moving and resizing worked correctly.

[uklotzde]

@ronso0 No, don't worry ;) It is just a one-off error in one of our unit tests. The infamous sample vs. frame position issue bites us once again.

[daschuer]

I cannot reproduce the issue here. Maybe this is also a gcc 7.2.1 rounding issue. I give it a try.

[ronso0]

This isn't merged to master yet, or is it?
I experience the same bug there

[daschuer]

This was recently merged to master 2018-02-19 18:15:42
231132e