Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't disable loop cues when latching it via play. lp1942656 #4265

Merged
merged 1 commit into from
Sep 6, 2021

Conversation

daschuer
Copy link
Member

@daschuer daschuer commented Sep 5, 2021

This fixes also a segfault that happens when deleting a cue in preview state.

This fixes also a segfault that happens when deleting a cue in preview state.
Comment on lines -1924 to +1936
updateCurrentlyPreviewingIndex(Cue::kNoHotCue);
int oldPreviewingIndex =
m_currentlyPreviewingIndex.fetchAndStoreRelease(
Cue::kNoHotCue);
if (oldPreviewingIndex >= 0 && oldPreviewingIndex < m_iNumHotCues) {
HotcueControl* pLastControl = m_hotcueControls.at(oldPreviewingIndex);
mixxx::CueType lastType = pLastControl->getPreviewingType();
if (lastType != mixxx::CueType::Loop) {
CuePointer pLastCue(pLastControl->getCue());
if (pLastCue && pLastCue->getType() != mixxx::CueType::Invalid) {
pLastControl->setStatus(HotcueControl::Status::Set);
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this code go in updateCurrentlyPreviewingIndex instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does not work here, because we need to keep the active state of the loop cue here for latching. Using updateCurrentlyPreviewingIndex in both cases was the root cause of this bug.

@Be-ing Be-ing requested a review from Holzhaus September 6, 2021 01:50
@Be-ing
Copy link
Contributor

Be-ing commented Sep 6, 2021

Thank you for working on this. I confirm it does fix the bug. I am not certain this is the best place to put the fix in the code.

@daschuer
Copy link
Member Author

daschuer commented Sep 6, 2021

Merge?

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