Saved beat jumps #13216
Saved beat jumps #13216
Conversation
| } | ||
| if (pCue->getPosition() > m_lastProcessedPosition && | ||
| pCue->getPosition() <= currentPosition) { | ||
| auto delta = pCue->getPosition() - currentPosition; |
There was a problem hiding this comment.
That does not work well, because the jump is quantized by the engine buffer size. Normally you want to jump beat exact and which can be somewhere within the buffer.
So we need something like the looping engine.
How about actually using the looping engine. Isn't a jump just the same as a loop which disables itself after one iteration.
There was a problem hiding this comment.
Why does it not work? This is already a simplified version of LoopingControl::process, except that we do not need adjustedPositionInsideAdjustedLoop, but can seek to the "target" directly, (corrected with the engine delay to prevent desync, as done with that delta var)
There is a few drawback I can think about using the looping engine:
- Loop engine expects start < end, but jump may have from > to. Allowing start > end in
LoopingControlsounds wrong - They maybe be a loop defined (but not active) and using the
LoopingControlwill override it for a "one frame" loop (or will have to deal with state restoration, which sounds overkill)
Isn't a jump just the same as a loop which disables itself after one iteration.
To some degree, except it is more simple that a loop, and doesn't need to complexity around the loop (no concept of seekMode for example, or direction). Disabling it at the next iteration implies that is had to be enable at some point, which is why making the most of EngineControl seems a better approach. The engine is already able to perform jumps, so I'm not sure what is the value of using the LoopingControl for a simple unique unidirectional jump
There was a problem hiding this comment.
The seeking code in LoopingControl::process handled only the exceptional cases when adjusting the loop size. The looping itself is done here:
mixxx/src/engine/readaheadmanager.cpp
Line 49 in e32b1ed
Your current implementation does not work for reverse playing and jumps before the track, and small jumps, < delta.
It took quite some iterations to have the looping code rock solid and I have not a big interest to repeat these iterations with beat jumps again. So let's consider what is the best way for trying that code.
Loop engine expects start < end, but jump may have from > to. Allowing start > end in LoopingControl sounds wrong
I think we have discussed earlier to allow forward loops. The engine don't mind if it is a repeated jump backward or a single jump forward.
However we need to take care not to create surprises when we allow to edit jumps with looping controls. So I agree to not reuse them in the first attempt.
They maybe be a loop defined (but not active) and using the LoopingControl will override it for a "one frame" loop (or will have to deal with state restoration, which sounds overkill)
We have this issue already with saved loops.
For example I would love to have a saved loop at the end of the track to extend a short outro that is always active. Maybe we can solve both in one run.
There was a problem hiding this comment.
Your current implementation does not work for reverse playing and jumps before the track, and small jumps, < delta.
It does need to, because of the unidirectional aspect of a jump. IMO, you wouldn't expect to jump back to the from position if you play reverse above to
For example I would love to have a saved loop at the end of the track to extend a short outro that is always active
Definitely would like to see this behaviour too as I could use it as well. But perhaps that is out of scope for this feature?
There was a problem hiding this comment.
For the reverse case, my expectation was that it jumps form the "to" position to "from" just as if you would play backwards a forward recording of the jump. But It is also OK for the first version to not jump at all in reverse, which is is probably not yet the case . (not tested)
There was a problem hiding this comment.
The idea of this feature is to "automate" a beatjump_forward so not sure it would make sense to "undo" this jump if playing reverse.
I guess we could make it work with a minimal amount of change, without using the LoopingControl if that the behaviour with would expect.
|
Currently this behaves like a loop, i.e. the jump cue is not deactivated after the jump if the the target is behind to cue. |
|
Having to deactivate a special mode in order to get the hotcue mode feels.
IMO they can be very close (like the hotcue grid), to somehow mimic radio buttons? |
That's right, and it was on purpose. Do you think it would be best to have it deactivated on run? My thought for the usecase was, when a DJ prepare their track cues, and decide to say that a section should be jumped over, this is likely that they want not to play the section of the track, so we should keep it on at all time. If they decide live to actually play this section, they can trigger the hotcue (
You don't have to. Like for the saved loop, you can use the
I like the idea, happy to make it work like this once we are happy with the state of feature |
acolombier
force-pushed
the
feat/saved-jump
branch
2 times, most recently
from
4d7c46e
to
bf74e6e
Compare
acolombier
force-pushed
the
feat/saved-jump
branch
from
bf74e6e
to
7759e18
Compare
|
I have upgraded the button to behave like a radio group. Also, I adjusted slightly the delete button to put the emphases that they are not part of the radio group. Kooha-2024-05-09-16-45-06.mp4 |
src/widget/wcuemenupopup.cpp
Outdated
| const mixxx::BeatsPointer pBeats = m_pTrack->getBeats(); | ||
| const auto position = mixxx::audio::FramePos::fromEngineSamplePos( | ||
| m_pPlayPos.get() * m_pTrackSample.get()); | ||
| if (position == m_pCue->getPosition()) { |
There was a problem hiding this comment.
For some reason this doesn't work for me:
when clicking Jump while at the cue position a zero-sized jump is created. When activating thus jump cue I run into an inifinite tiny loop.
There was a problem hiding this comment.
in the display start and end seem to be idetical.
Maybe a rounding issue with fromEngineSamplePos()?
There was a problem hiding this comment.
This should be fixed now
|
I think at least in LateNight we need better styling for the 'active state' of loop/jump cues. I'll tr ysomething. |
|
I've made some minor UI tweak - does that look better @ronso0 ? Also, I have made the saved beat jump to disable itself if jumping backward, to prevent them making unwanted loops. My use case for this feature was the ability to jump above certain part of track with the low interest, so I would appreciate we don't disable jump forward when possible so a track can be prepared with auto forwarded sections. If you think it should be different, I can add a setting in preference. |
|
For some reason I can't assign a "free" jump target anymore, it'll always use the current jump size, both for righ- and left-click. |
Strange, I can't reproduce that anymore. |
|
That weird indeed. It sounds like a bug I fixed when I rebased main yesterday tho, couldn't it be some cached sources? |
|
What I find a bit disturbing is that the jump range in the overview always appears as 'active', unlike saved loops which are less opaque if disabled. |
Yep, agreed. This is why this is still a draft ;) I've paused that work because implementing the above requires to touch on lot of waveform renderers atm, which we are hopefully being retired as part of #13226. |
|
Just an idea I had while reading https://mixxx.discourse.group/t/timing-a-fireworks-show/29693/3 |
Might stem from a previous version where the end was not cleared after changing back to hot/jump cue. |
This addresses the feature request #13161
Currently, the activating the saved jump result in enabling or disabling the auto jump (different icon used to provided feedback on jump status)
Kooha-2024-05-06-18-58-03.mp4
TODO:
Depends on #13215