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

fixes in common-controller-scripts.js #1072

Merged
merged 6 commits into from
Dec 13, 2016

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Dec 12, 2016

This is my first fix on github -woohoo!- so please tell me when I'm doing something...not right.

In the first fix I just corrected the use of 'numberOfBeats' for loopMove function.
The second one is about script.brake() and considers the actual playback speed:
Until now, a pitched track would jump to 100% speed, then slow down
see https://bugs.launchpad.net/mixxx/+bug/1571442

res/controllers/common-controller-scripts.js Show resolved Hide resolved
// disable on note-off or zero value note/cc
engine.brake(parseInt(group.substring(8,9)), ((status & 0xF0) != 0x80 && value > 0));
// calculate current playback speed
var currentRate = engine.getValue(group,"bpm") / engine.getValue(group,"file_bpm");
Copy link
Contributor

Choose a reason for hiding this comment

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

Would engine.getValue(group, "rate"); work?

Copy link
Member Author

Choose a reason for hiding this comment

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

In the wiki I see that 'rate' ranges from -1...0..1 (which I read as "0 = normal speed") while in brake() 'rate' '1' means normal speed, -10 would be 10x backwards
So I guess no. And I was to lazy to even think about the conversion.. ;)
But the identical naming is irritating, though. Maybe in brake() 'rate' should be 'speed'?

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, I was asking in case it would be easier to just get the rate value without having to manipulate it at all. I'm okay with the naming as is.

@Be-ing
Copy link
Contributor

Be-ing commented Dec 12, 2016

Thanks for fixing these. :)

@@ -44,6 +44,7 @@ TerminalMix.init = function (id,debug) {
engine.softTakeover("[Channel"+i+"]","filterHigh",true);
engine.softTakeover("[Channel"+i+"]","filterMid",true);
engine.softTakeover("[Channel"+i+"]","filterLow",true);
engine.softTakeover("[Channel"+i+"]","rate",true);
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 was easy..

@Be-ing
Copy link
Contributor

Be-ing commented Dec 13, 2016

Okay, LGTM. If you have any more changes for the Terminal Mix mapping, make a separate pull request with a new branch for those.

@daschuer
Copy link
Member

Thank you very much :-)

@daschuer daschuer merged commit 7e167e6 into mixxxdj:master Dec 13, 2016
@ronso0
Copy link
Member Author

ronso0 commented Dec 13, 2016

Cool!

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.

3 participants