Lykos153 / mixxx

Mixxx is Free DJ software that gives you everything you need to perform live mixes.
http://mixxx.org
Other
4 stars 4 forks source link

Add vinyl button #21

Closed iRet closed 3 years ago

iRet commented 3 years ago

Issue #9

Improve scratch / slip button implementation:

Both are disabled by default

upCASE commented 3 years ago

Hi iRet,

to be honest, I don't like the new behavior. Slip mode should be a feature that's always ready to use, not just a mode you have to explicitly enable. Scratch mode is the default on most controllers with sensitive touch pads, so I wouldn't change that as well.

Changing it this way would break with the defaults used on other controllers ans confuse people. You could implement permanently enabling slip mode by using shift instead of having to double tap the slip button, but single tapping and holding the button should stay a temporary action. Enabling/disabling scratch mode should not be the purpose of this button IMHO. Maybe it would be a good idea to check how the Roland 505 mapping handles this, as well as checking the docs for other tools like Serato.

iRet commented 3 years ago

I can make scratch enabled by default. There reason I made it disabled to work around the led issue.

As for slip. I didn't notice there the is also a hold feature. I can take a look.

Enabling/disabling scratch mode should not be the purpose of this button IMHO.

I'm politely disagree, main purpose of vinyl button is to toggle scratch mode in serato.

Maybe it would be a good idea to check how the Roland 505 mapping handles this, as well as checking the docs for other tools like Serato.

Roland 505 mapping binds unshifted tap to the vinylcontrol_enabled that in my opinion is wrong. Vinyl control has nothing to do with the scratch mode and in use for DVS

upCASE commented 3 years ago

Hmm... Although I liked the old behavior better and prefer to have scratch enabled by default, this is only my opinion. I guess if you enabled scratch again, I'm more than happy with any changes you make :-)

iRet commented 3 years ago

The reason why I decided to switch to the serato behavior of vinyl button - inconsistency of the double tap approach. It gets out of sync with the led if you toggle it multiple times in a row. May be we can fix it the other way. My goal was to be able to disable the scratch since I often touch it accidentally and it is way too sensitive.

TL;DR I'm new to dj controllers, only used standalone decks before and a little bit overwhelmed with the tons of features available. I have spent lots of time hunting these issues even though I am a software engineer. Especially I disappointed with the tons of features bound to the hotcue buttons. I was trying to port the 505 mapping at first, but decided to postpone with this and fix what we already have instead.

upCASE commented 3 years ago

Hi @iRet,

I'm not a Pro myself, just DJing at times for personal fun. I guess there is no absolute right or wrong in this, so all progress with this mapping is fine with me :D

Using the 505 mapping as a fresh new base for the 202 was my intention as well, but it seems not feasible to me. Besides sadly I don't have much time for maintaining this mapping... Maybe it's best to approach it like you mentioned: Fix what's there.

The layering of hotcue buttons is very common, but I must confess that on the 202 (and I guess 505 as well) with 4 Layers it can be quite confusing, especially since the labels are not very helpful... But this is something you'll get accustomed to, as most of the time you only need the first 2 layers. I added e.g. a slicer mode to mimic Serato, but it's not very stable and Mixxx lacks visual support for this.

About the changes: So scratch is enabled by default and the vinyl button toggles it. Slip mode is toggled using shift & vinyl. I just had a quick try and it seemed to work and felt good. However I was under the impression that after some fiddling I produced a state where the LED did not match the behavior. I wasn't able to reproduce this after a restart, so maybe it was just in my had and I need another cup of coffee... If you don't plan on changing more stuff in this, I'll check again tonight and merge this 👍

iRet commented 3 years ago

@upCASE I'm hesitating between sticking to the current approach just binding scratch disable to shift+vinyl and the Serato way where vinyl press toggles scratch mode. In any event having vinyl led dedicated for scratch enabled seems reasonable to me. Slip mode has its own icon in UI. Vinyl mode has nothing.

Is temporary enabling slip mode while vinyl is pressed something standard for the Mixxx?

Personally I don't use slip, but I'm a huge fan of consistency. I would stick to the most obvious and predictable way.

upCASE commented 3 years ago

You're right: Having the LED indicate this would be correct I guess. But: I suppose the Labels might be misleading. Actually I think that the button is specifically meant for enabling slip mode, not vinyl.

Is temporary enabling slip mode while vinyl is pressed something standard for the Mixxx? Very good question... I fear not as I never observed this. From some videos I watched Serator and Recordbox do exactly this.

https://youtu.be/hjDeRDiWn0E?t=78 Might not be the best video, but form others I watched this seems to be fairly "standard" behavior. The button controls slip in the first place (maybe shift+tap might then be use to enable/disable scratch?).

https://youtu.be/hjDeRDiWn0E?t=224 What you refer to is this behavior, which I would expect for scratch, hotcue and loop mode. But from what I observe this is not what happens. This seems to be somehting which the controller script must implement. I might be wrong though and I'll see ig I can find the time and doublecheck this in Mixxx. There appears to be no clear answer, but given that most people use slip this way, I guess this is a whole new level of "problems". Maybe I'll check what the 505 does in that regard.

upCASE commented 3 years ago

https://manual.mixxx.org/2.2/en/chapters/user_interface.html?highlight=slip Apparently ist should work just fine. Maybe this was one of my facepalm moments. I'll try again...

Edit: Ah, no! "Once disabled".... it should resume. So this is intended behavior in Mixxx, but I'd say that it renders the feature more or less useless. Actually it's quite odd as I found a very old video of Mixxx, which shows the exact thing I would have expected: https://www.youtube.com/watch?v=Mz0hqEvutak I implemented this in some modes, but for scratch this apparently never worked...

upCASE commented 3 years ago

Seems to have been discussed for the 505 as well: https://mixxx.discourse.group/t/roland-dj-505/17916/34

iRet commented 3 years ago

Hi @upCASE Thanks for the links, I didn't know that slip mode also affects hotcues. Using slip for hotcues make much more sense to me rather than for scratching. I was also thinking on making the function switchable as @Lykos153 mentioned here #9

That would be awesome if mixxx implements some UI allowing to control global variables. For instance I would be happy to have scratch feature disabled by default without the need to modify mappings.

I think I'll just revert the slip toggle code and add use shift+slip for scratch control, to make the changes less breaking.

upCASE commented 3 years ago

Hi @iRet using the combination you mention would be a good way to do this.

The hotcue behavior is not implemented in Mixxx, at least I didn't get it working. So having the hold button = temporary slip is currently the only way to work with hotcues in a meaningful way. I'll see if I can implement this for hotcues, but given the fact that I don't find much time for this I don't have high hopes.

Anyway, once you do your changes I'll happily review them.

iRet commented 3 years ago

@upCASE ready for review!

It's turned out that shifted control has its own virtual led state. When shift is pressed vinyl led indicates shifted midi message state. So I'm leaving slip mode led untouched and using "virtual led" instead.

upCASE commented 3 years ago

Hi @iRet ,

sorry, took me a while to find the time an try it out. Work nicely, thanks! Are you done with this?

iRet commented 3 years ago

@upCASE yeah let's merge it