Conversation
|
Tested with Carla. Before: After: Nice fix! |
|
@AW1534 In your "After" recording, it appears the current bar (BBT) still isn't working correctly. When you drag the play head, the current bar in the display does not follow - it snaps back to the beginning. |
I see that now. Although the frame and time are working as expected, I had ignored BBT because I was unsure what it was and wrongfully assumed it would be derived purely from frame, similarly to how time is. Okay so after a little research, it appears on the Carla vst2 host, BBT is derived from the bar start position and PPQ (Position in quarter notes). Also, the kVstPpqPosValid flag needs to be set. Judging by the fact that the BBT was also broken in the same way before the PR, I’m assuming the bar start position and/or PPQ aren’t being sent or aren’t being sent correctly. Judging by the nature of the bug, my gut says PPQ is what’s causing this. Anywho, since this doesn’t seem to be a regression and at least improves the frame based transport, I think it’s fair to merge this PR and open a separate issue (or issues), rather than inflating the scope of this PR? |
|
|
||
| // Update VST plugins with the new position so they stay in sync when skipping around, | ||
| // even when playback is paused. | ||
| if ((m_playing && m_playMode == playMode) || (!m_playing)) { | ||
| const auto& tl = m_timelines[static_cast<std::size_t>(playMode)]; | ||
| m_vstSyncController.setAbsolutePosition( | ||
| tl.ticks() + tl.frameOffset() / static_cast<double>(Engine::framesPerTick()) | ||
| ); | ||
| } |
There was a problem hiding this comment.
@regulus79 @sakertooth I'm going to run an idea by you two about how we could fix this problem more robustly.
For VST sync, from what I've gathered from this PR, it apparently needs to function the same regardless of whether the song is playing or not. But according to the comment a few lines above here, we've restricted the playbackPositionJumped signal emission to when the song is playing due to how the LFOs work.
My idea is that we remove the restriction on emitting the signal only while the song is playing since that is unintuitive for anyone relying on that signal (and LFOs are not the responsibility of Song), then in the LFO's slot we could add an early return if the song is not playing when it receives the position jumped signal (fixing the problem with the LFOs), and then VST sync would work even when the song is not playing.
And maybe we could also remove the connect() a few lines below this since it involves the Song connecting to its own signal, which seems very strange. Maybe something like this would work instead:
const auto onPositionJumped = [this, playMode = static_cast<PlayMode>(i)] {
// Only emit the signal when the active timeline jumps
if (m_playMode == playMode)
{
emit playbackPositionJumped();
m_vstSyncController.setPlaybackJumped(true);
const auto& tl = getTimeline(playMode);
m_vstSyncController.setAbsolutePosition(
tl.ticks() + tl.frameOffset() / static_cast<double>(Engine::framesPerTick())
);
}
};
connect(&m_timelines[i], &Timeline::positionJumped, this, onPositionJumped);(NOTE: I haven't tested this yet)
There was a problem hiding this comment.
I think that solution sounds reasonable. The nested logic within the lambda and the connect never felt quite right to me, although maybe it's fine if it's not too complex. Putting the responsibility of checking whether the song is playing on the LFO sounds like it would make Song simpler, which is good.
Resolves #8343
Bug Summary:
When playback is paused, VST plugins loaded via VeSTige lose synchronization with the host's timeline. Specifically,
m_currentSamplePosinRemoteVstPlugincontinuously increments bybufferSize()regardless of playback state, pushing incorrect and constantly advancing time information to the VST.Additionally, when the user drags the playhead on the LMMS timeline while playback is stopped,
VstSyncControllerwas not being updated, preventing the VST from receiving the new timeline position (Bars/Beats).Changes Made:
plugins/VstBase/RemoteVstPlugin.cpp:m_currentSamplePos += bufferSize()increment with an absolute timeline calculation derived fromsyncData->ppqPosandbpm.samplePosvalues tied to the timeline instead of raw frame accumulation.src/core/Song.cpp:Timeline::positionJumpedto updatem_vstSyncController.setAbsolutePosition(...)even when playback ispaused/!m_playing.