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 controls to manipulate the bpm via skin/midi controller #9133

Closed
mixxxbot opened this issue Aug 23, 2022 · 18 comments · Fixed by #12934
Closed

add controls to manipulate the bpm via skin/midi controller #9133

mixxxbot opened this issue Aug 23, 2022 · 18 comments · Fixed by #12934

Comments

@mixxxbot
Copy link
Collaborator

Reported by: index82
Date: 2018-02-13T17:35:16Z
Status: Confirmed
Importance: Wishlist
Launchpad Issue: lp1749252
Tags: bpm, skin, usability


like e.g. beats_adjust_faster, beats_adjust_slower ...

Options:

  • double BPM
  • halve BPM
  • 2/3 BPM
  • 3/4 BPM
  • 4/3 BPM
  • 3/2 BPM
@mixxxbot
Copy link
Collaborator Author

Commented by: esbrandt
Date: 2018-02-21T08:07:53Z


Could you elaborate a bit more what you are requesting here?

Some skins e.g. Tango and Deere do have controls to edit a beatgrid, and all skins have the ability to Change the BPM by right-clicking a track in the library, or by editing the BPM tab in the track properties editor.

Beatgrid editing controls are also available in the MIDI controller wizard, under the BPM menu.

@mixxxbot
Copy link
Collaborator Author

mixxxbot commented Aug 23, 2022

Commented by: index82
Date: 2018-02-23T21:14:06Z


I checked the midi controller wizard now but didn't find the requested controls.
The library contextmenu is usable but not very fast. (find title; right click; hover and move; left click)

Mixxx has

  • beats_adjust_faster
  • beats_adjust_slower
  • beats_translate_curpos
  • beats_translate_match_alignment
  • beats_translate_earlier
  • beats_translate_later

Mixxx hasn't

  • beats_set_halve

  • beats_set_double

  • beats_set_twothirds

  • beats_set_threehalves

  • beats_set_threefourths

  • beats_set_fourthirds
    (I don't know the correct terms.)

    OK, in this case it's a personal wish.
    I don't want these controls as ui elements in the official mixxx skins.
    But it would be good to have the possibility and to fill the small gap of controls. ;)

My idea is to speed up the workflow for preparing tracks/beatgrids. And it makes realy fun to work with.
FYI: clone my repo and test the editdeck (WIP) https://github.com/22degrees/RoundCorners
The "EDIT"-Button in the left upper corner in each deck.

@mixxxbot
Copy link
Collaborator Author

Commented by: mxmilkiib
Date: 2020-08-24T21:08:41Z


MIDI control for fixing incorrect BPM detection would be incredibly welcome.

The default skin now doesn't have these options in the main UI body, one has to go back into the media library to use a submenu, which is a bit of a workflow nightmare.

The pads on many controllers now would be perfect for binding such to.

@mixxxbot
Copy link
Collaborator Author

Commented by: ronso0
Date: 2020-08-25T13:52:40Z


FYI
in Mixxx 2.3 there's no need to go back to the library anymore: you can open the track context menu by right-clicking on a track property in decks: title, artist, comment (Tango only). There you have a 'Adjust BPM' menu with all options.

You can also double-click on artist/title fileds in the decks to open the Track Properties window. Besides the track metadata, it has a second tab 'BPM' with all BPM options where you can also tap the BPM or enter a value manually.

@mixxxbot mixxxbot transferred this issue from another repository Aug 24, 2022
@ronso0 ronso0 added skins and removed skin labels Jun 6, 2023
@ronso0
Copy link
Member

ronso0 commented Nov 4, 2023

This is a kind of a no-brainer:
just add the controls and slots like in DlgTrackInfo to BpmControl
40a7de1 for a starter. I have no intention to work on this, so feel free to adopt that commit.

@ronso0 ronso0 added the easy label Nov 4, 2023
@mxmilkiib
Copy link
Contributor

mxmilkiib commented Feb 21, 2024

This would be very handy/essential for keyboard+mouse free library management.

What is the actual status of what is in the 40a7de1 commit? Before I go yak shaving on implementing this. Is it purely an example with just one of the fractional BPM Adjustments? I guess some mindful copying+pasting and rewordings could fulfil this?

@mxmilkiib
Copy link
Contributor

So, given the actions are already defined in wtrackmenu.cpp, is it sensible to reproduce the = in that commit? Should there be some DRY here? Should that function remain in the wtrackmenu file even though it would be used elsewhere?

If redefining is best, would that require a different w_ name to avoid a collisions, or is the scope different enough?

@ronso0
Copy link
Member

ronso0 commented Feb 22, 2024

What is the actual status of what is in the 40a7de1 commit?

40a7de1 is just a POC, I don't intend to continue that.

In order to extend that example for more scales, this

    connect(m_pBeatsDouble,
            &ControlObject::valueChanged,
            this,
            &BpmControl::slotDoubleBpm,
            Qt::DirectConnection);

would simply use a lambda

    connect(m_pBeatsDouble,
            &ControlObject::valueChanged,
            this,
            [this](value) {
                if (value > 0) {
                    slotScaleBpm(mixxx::Beats::Double);
                }
            });

(and drop void BpmControl::slotDoubleBpm())
Other BPM scale controls would be set up identically, just use different scales.

Does that make sense?

@ronso0
Copy link
Member

ronso0 commented Feb 22, 2024

So, given the actions are already defined in wtrackmenu.cpp, is it sensible to reproduce the = in that commit? Should there be some DRY here? Should that function remain in the wtrackmenu file even though it would be used elsewhere?

IMHO we don't need to care about the actions in WTrackMenu, redundancy is not an issue in that case.

@mxmilkiib
Copy link
Contributor

Does that make sense?

I don't quite understand.

I'm trying to add to the files, but I'm getting this, so I'm a bit lost;

image

If I right click m_pTranslateBeatsMove and go to the declaration, it goes to :47 in .h

What am I missing here?

@mxmilkiib
Copy link
Contributor

mxmilkiib commented Feb 28, 2024

Oh, I think it's just an misconfigured extension.. nm

I just disabled it n went with another clangd extension.

Now I'm getting;
image
on attempting to cmake .. scratch head thought that was fixed

@mxmilkiib
Copy link
Contributor

Why are there 5 connect() parameters in the original example and only 4 in the lambda example?

@ronso0
Copy link
Member

ronso0 commented Mar 4, 2024

The fifth parameter is the Qt::ConnectionType. If it's omitted it defaults to Qt::AutoConnection. Nothing to worry about for a POC.

@ronso0
Copy link
Member

ronso0 commented Mar 4, 2024

I suggest to open a draft PR where we can discuss code much easier than with screenshots ; )

@mxmilkiib
Copy link
Contributor

I shall, once I've been able to try build 2.5 :)

@mxmilkiib
Copy link
Contributor

Finally, right.

How is the scope meant to get this? And where exactly do I declare Halve as a member of Beats? Cheers!

/home/milk/src/mixxx/src/engine/controls/bpmcontrol.cpp: In constructor ‘BpmControl::BpmControl(const QString&, UserSettingsPointer)’:
/home/milk/src/mixxx/src/engine/controls/bpmcontrol.cpp:101:20: error: ‘value’ has not been declared
  101 |             [this](value) {
      |                    ^~~~~
/home/milk/src/mixxx/src/engine/controls/bpmcontrol.cpp: In lambda function:
/home/milk/src/mixxx/src/engine/controls/bpmcontrol.cpp:102:21: error: ‘value’ was not declared in this scope
  102 |                 if (value > 0) {
      |                     ^~~~~
/home/milk/src/mixxx/src/engine/controls/bpmcontrol.cpp:103:48: error: ‘Halve’ is not a member of ‘mixxx::Beats’
  103 |                     slotScaleBpm(mixxx::Beats::Halve);
      |                                                ^~~~~

@ronso0
Copy link
Member

ronso0 commented Mar 7, 2024

I shall, once I've been able to try build 2.5 :)

Actually you don't need to build it yourself, just commit, push and CI will build for you. Should be sufficient to spot errors and discuss code.

@mxmilkiib
Copy link
Contributor

mxmilkiib commented Mar 7, 2024

Some ints and BpmScale::s I think have sorted it..

#12934

@ronso0 ronso0 linked a pull request Mar 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants