Do not create a new loop if it's the same as an existing loop #11006
Conversation
This works around mixxxdj#11003 in which the adjustment of the spinbox creates a new loop, causing a sudden unexpected seek
|
hmmmm this is supposed to be covered by currentLoopMatchesBeatloopSize |
Also pass in loopinfo to checking functions to make more const-safe
There was a problem hiding this comment.
Even though this PR fixes the issue, we have the situation that the final loop does not match the stored one. It is the normalized version due to the Beat Spin Box roundtrip.
I think we should avoid this in the first place.
Looking at your backtrace we may call
blockSignals(true);
in WBeatSpinBox::slotControlValueChanged
Will this this be a solution?
| return positionNear(loopInfo.endPosition, loopEndPosition); | ||
| } | ||
|
|
||
| bool LoopingControl::positionNear(mixxx::audio::FramePos a, mixxx::audio::FramePos target) const { |
There was a problem hiding this comment.
This function does not use the object and can be therefore a free function in the anonymous namespace.
| // Only update seek mode if the endpoints have changed sufficiently. | ||
| if (positionNear(newloopInfo.startPosition, loopInfo.startPosition) && | ||
| positionNear(newloopInfo.endPosition, loopInfo.endPosition)) { | ||
| newloopInfo.seekMode = loopInfo.seekMode; |
There was a problem hiding this comment.
When the play head is behind the new playposition but before the old position, the loop is activated but the playhead keeps running outside the loop. This must not happen.
So I think we need here:
newloopInfo.seekMode = LoopSeekMode::MovedOut;
There was a problem hiding this comment.
when you say "new play position" do you mean "new start position"? This function has no concept of "old and new play head position".
It is fine to activate loops when the play head is before the loop start position, this is the intended behavior.
There was a problem hiding this comment.
The current play position is relevant when seek mode is used. In a rare condition it can be in the gap between the old and new end position. Let's say loopInfo.endPosition 101, play-position at 100.5 and newloopInfo.endPosition = 100.
Without LoopSeekMode::MovedOut; the loop will be exited. I think that is undesired,
yes I think that might do it, I'll try that. ah, but we do want to emit a signal when the user changes the spinbox. |
Block signals in setLoop to prevent loop size widget updating from emitting more signals. Small refactor
… up past the loop end point.
This works around #11003 in which the adjustment of the spinbox creates a new loop, causing a sudden unexpected seek