Skip to content

Adding CUP button and CUP mode.#1001

Closed
williamlemus wants to merge 35 commits intomixxxdj:masterfrom
williamlemus:master
Closed

Adding CUP button and CUP mode.#1001
williamlemus wants to merge 35 commits intomixxxdj:masterfrom
williamlemus:master

Conversation

@williamlemus
Copy link
Copy Markdown
Contributor

https://bugs.launchpad.net/mixxx/+bug/1535468

The CUP mode has been added. It works the following way:

  • Press, pause at the cue point
  • release, play

It is available as a Cue mode, or as a button that can be learned.

Comment thread src/engine/cuecontrol.cpp Outdated
// This is how CUP button works:
// If playing, press to go to cue and stop.
// If stopped, press to set as cue point.
// On release, start playing from cue point.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

indentation problem between the first two lines and the next two lines.

Copy link
Copy Markdown
Member

@daschuer daschuer Sep 4, 2016

Choose a reason for hiding this comment

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

// If stopped, press to set as cue point.

I am not sure if that is desired. Do you have a reference for this feature?

EDIT: I have found some, so the set behaviour is correct.

@sblaisot
Copy link
Copy Markdown
Member

sblaisot commented Sep 3, 2016

Thanks fo this PR.

Did you sign the mixxx contributor agreement ? https://docs.google.com/a/mixxx.org/spreadsheet/viewform?formkey=dEpYN2NkVEFnWWQzbkFfM0ZYYUZ5X2c6MQ

Please also update the mixxxcontrols wiki page when this PR will be merged.

@daschuer
Copy link
Copy Markdown
Member

daschuer commented Sep 4, 2016

Thank you for this Pull Request.

Unfortunately this branch does not merge cleanly with our master branch. It is a good practice to keep your own master branch equal with the upstream master branch of mixxxdj/mixxx.

This can be done from the command line like that:

# copy your changes into a feature branch 
git checkout master
git checkout -b CUP_feature

# adding mixxxdj/mixxx as upstream, if not already done:
git remote add upstream https://github.com/mixxxdj/mixxx.git

# set your master to upstream master 
git fetch upstream
git checkout master
git reset upstream/master --hard

# rebase your changes onto your new master
git checkout CUP_feature
git rebase master
# this will conflict. meld is a nice tool to resolve these conflicts side by side 
sudo apt-get install meld 

# resolve conflicts:
git mergetool
# add changed files 
git add <changed files list>
git rebase --continue  

# publish your changes
git push 

# now you have to issue a new pull request here on GitHub for your new branch.

Comment thread src/controllers/controlpickermenu.cpp Outdated
addDeckControl("play_stutter", tr("Stutter Cue"),
tr("Stutter cue"), cueMenu);
addDeckControl("cue_play", tr("Cue (CUP Mode)"),
tr("Cue button (CUP mode)"), cueMenu);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMHO this the CUP button is a button of its own, not a cue button in a cue mode.
How about:

addDeckControl("cue_play", tr("CUP (Cue Play)),
                          tr("Goto cue point and play after release"), cueMenu);

@daschuer
Copy link
Copy Markdown
Member

daschuer commented Sep 4, 2016

http://www.reloop.com/reloop-digital-jockey-2-master-edition#
in Tutorial Video 9, they say that CUP also sets the cue point when paused.

@daschuer
Copy link
Copy Markdown
Member

daschuer commented Sep 4, 2016

Not sure, but here it is nothing about setting a cue point with CUP
http://www.reloop.com/media/custom/upload/Reloop-Reloop_Beatmix2_VirtualDJ8_Operation_Guide.pdf

@daschuer
Copy link
Copy Markdown
Member

daschuer commented Sep 4, 2016

OK, after searching the web some more, I think your implementation with cue set feature is just right.

@williamlemus
Copy link
Copy Markdown
Contributor Author

I will make a new pull request(rebased from the upstream master branch) with the following changes:

  • Change tab to spaces
  • Reword the addDeckControl to:

addDeckControl("cue_play", tr("CUP (Cue Play)),
                          tr("Goto cue point and play after release"), cueMenu);
  • Remove the quantization:
            // If quantize is enabled, jump to the cue point since it's not
            // necessarily where we currently are
            if (m_pQuantizeEnabled->get() > 0.0) {
                lock.unlock();  // prevent deadlock.
                // Enginebuffer will quantize more exactly than we can.
                seekAbs(m_pCuePoint->get());

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.

7 participants