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

add 'LoadTrackFromDeck' and '~Sampler' controls #11244

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Feb 3, 2023

this just loads the track from the source deck.

Background:
for my controller mapping I abuse the clone controls to copy tracks into parking slots (samplers 1-8 or decks 3/4) and recall them later on.
Currently, I need to stop the parking deck immediatley after cloning to suppress undesired sounds. With this simple 'Load' control I don't need to care.

@ronso0 ronso0 force-pushed the load-track-from-group-control branch from 65cc54a to 0f1e2c1 Compare February 3, 2023 22:23
@@ -650,6 +666,34 @@ void BaseTrackPlayerImpl::slotCloneChannel(EngineChannel* pChannel) {
slotLoadTrack(pTrack, false);
}

void BaseTrackPlayerImpl::slotLoadTrackFromDeck(double d) {
int deck = static_cast<int>(d);
if (deck >= 1) {
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to check for the lower bound and not for the upper? I consider to remove the check.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've removed it.
If the group name is invalid it'll fail in loadTrackFromGroup anyway.

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

No objections, to add this. Just some comments.

src/mixer/basetrackplayer.cpp Outdated Show resolved Hide resolved
return;
}

TrackPointer pTrack = pChannel->getEngineBuffer()->getLoadedTrack();
Copy link
Member

Choose a reason for hiding this comment

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

A null check for pChannel->getEngineBuffer() can't hurt here.

Copy link
Member Author

Choose a reason for hiding this comment

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

The BaseTrackPlayerImpl constructor creates a new EngineDeck which creates the EngineBuffer, and not even there is a a check for pEngineBuffer, so why here? if it was missing for whatever reason Mixxx would have fallen apart long before LoadTrackFromDeck could be triggered.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, right. So this is a perfect use case of 'gsl::not_null<>. But probably a topic for an extra pull request.

src/mixer/basetrackplayer.cpp Outdated Show resolved Hide resolved
@ronso0 ronso0 force-pushed the load-track-from-group-control branch from 0f1e2c1 to 2a02687 Compare February 6, 2023 01:05
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you.

@daschuer
Copy link
Member

daschuer commented Feb 6, 2023

Can you also take care to add these new controls to the manual?

@daschuer daschuer merged commit 56657a9 into mixxxdj:2.4 Feb 6, 2023
@daschuer daschuer added this to the 2.4.0 milestone Feb 6, 2023
@daschuer daschuer added the changelog This PR should be included in the changelog label Feb 6, 2023
@ronso0 ronso0 deleted the load-track-from-group-control branch February 6, 2023 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog This PR should be included in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants