feat: add support for scaling BPM by different ratios

[mxmilkiib]

This change introduces the ability to scale the BPM of a track by different ratios using ControlPushButton objects. Each button is connected to a lambda function that calls the slotScaleBpm method with the corresponding BPM scale factor.

[daschuer]

Unfortunately clang-format is still complaining.
I suggest to install pre-commit locally to format every commit automatically. https://github.com/mixxxdj/mixxx/wiki/Using-Git#set-up-automatic-code-checking
It will also install clang-format.
You may format your whole PR
By git clang-format upstream/main
(Make sure not to format the whole code base.)

[mxmilkiib]

That syntax gives me this though;

In file included from /home/milk/src/mixxx/src/engine/controls/bpmcontrol.cpp:1:
/home/milk/src/mixxx/src/engine/controls/bpmcontrol.h:149:39: error: template argument 1 is invalid
  149 |     std::unique_ptr<ControlPushButton m_pBeatsHalve;
      |                                       ^~~~~~~~~~~~~
/home/milk/src/mixxx/src/engine/controls/bpmcontrol.h:149:39: error: template argument 2 is invalid
/home/milk/src/mixxx/src/engine/controls/bpmcontrol.h:149:10: error: ‘<expression error>’ in namespace ‘std’ does not name a type
  149 |     std::unique_ptr<ControlPushButton m_pBeatsHalve;
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
/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:97:5: error: ‘m_pBeatsHalve’ was not declared in this scope; did you mean ‘m_pBeats’?
   97 |     m_pBeatsHalve = std::make_unique<ControlPushButton>(ConfigKey(group, "beats_set_halve"), false);
      |     ^~~~~~~~~~~~~
      |     m_pBeats

[ronso0]

std::unique_ptr<ControlPushButton m_pBeatsHalve;

missing closing >

[mxmilkiib]

with closing bracket;

/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:98:12: error: no matching function for call to ‘BpmControl::connect(std::unique_ptr<ControlPushButton>&, void (ControlObject::*)(double), BpmControl*, BpmControl::BpmControl(const QString&, UserSettingsPointer)::<lambda(int)>)’
   98 |     connect(m_pBeatsHalve,
      |     ~~~~~~~^~~~~~~~~~~~~~~
   99 |             &ControlObject::valueChanged,
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  100 |             this,
      |             ~~~~~
  101 |             [this](int value) {
      |             ~~~~~~~~~~~~~~~~~~~
  102 |                 if (value > 0) {
      |                 ~~~~~~~~~~~~~~~~
  103 |                     slotScaleBpm(mixxx::Beats::BpmScale::Halve);
      |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  104 |                 }
      |                 ~
  105 |             });
      |             ~~
In file included from /usr/include/qt6/QtCore/QObject:1,
                 from /home/milk/src/mixxx/src/control/controlobject.h:3,
                 from /home/milk/src/mixxx/src/engine/controls/bpmcontrol.h:5,
                 from /home/milk/src/mixxx/src/engine/controls/bpmcontrol.cpp:1:
/usr/include/qt6/QtCore/qobject.h:198:9: note: candidate: ‘static QMetaObject::Connection QObject::connect(const typename QtPrivate::FunctionPointer<Prototype>::Object*, Func1, const typename QtPrivate::ContextTypeForFunctor<Func2>::ContextType*, Func2&&, Qt::ConnectionType) [with Func1 = void (ControlObject::*)(double); Func2 = BpmControl::BpmControl(const QString&, UserSettingsPointer)::<lambda(int)>; typename QtPrivate::FunctionPointer<Prototype>::Object = ControlObject; typename QtPrivate::ContextTypeForFunctor<Func2>::ContextType = QObject]’
  198 |         connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal,
      |         ^~~~~~~
/usr/include/qt6/QtCore/qobject.h:198:75: note:   no known conversion for argument 1 from ‘std::unique_ptr<ControlPushButton>’ to ‘const QtPrivate::FunctionPointer<void (ControlObject::*)(double)>::Object*’ {aka ‘const ControlObject*’}
  198 |         connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
/usr/include/qt6/QtCore/qobject.h:246:9: note: candidate: ‘template<class Func1, class Func2> static QMetaObject::Connection QObject::connect(const typename QtPrivate::FunctionPointer<Prototype>::Object*, Func1, Func2&&)’
  246 |         connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal, Func2 &&slot)
      |         ^~~~~~~
/usr/include/qt6/QtCore/qobject.h:246:9: note:   template argument deduction/substitution failed:
/home/milk/src/mixxx/src/engine/controls/bpmcontrol.cpp:98:12: note:   candidate expects 3 arguments, 4 provided
   98 |     connect(m_pBeatsHalve,
      |     ~~~~~~~^~~~~~~~~~~~~~~
   99 |             &ControlObject::valueChanged,
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  100 |             this,
      |             ~~~~~
  101 |             [this](int value) {
      |             ~~~~~~~~~~~~~~~~~~~
  102 |                 if (value > 0) {
      |                 ~~~~~~~~~~~~~~~~
  103 |                     slotScaleBpm(mixxx::Beats::BpmScale::Halve);
      |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  104 |                 }
      |                 ~
  105 |             });
      |             ~~
/usr/include/qt6/QtCore/qobject.h:177:36: note: candidate: ‘static QMetaObject::Connection QObject::connect(const QObject*, const char*, const QObject*, const char*, Qt::ConnectionType)’
  177 |     static QMetaObject::Connection connect(const QObject *sender, const char *signal,
      |                                    ^~~~~~~
...

[mxmilkiib]

Aah cmake cache again.. edit: nope..

[ronso0]

Yes, it's a unique_ptr now so we need to get() the value first (the ControlPushButton in this case).
This should work:

connect(m_pBeatsHalve.get(),
       &ControlObject::valueChanged,
       this,
       [this](int value) {
           if (value > 0) {
               slotScaleBpm(mixxx::Beats::BpmScale::Halve);
           }
       });

[mxmilkiib]

The style changes mentioned are inconsistent, I could do the extra line breaks, but it's not asking for them in all cases..?

[ronso0]

Builds are currently failing because the explicit delete calls are still in the destructor BpmControl::~BpmControl but not applicable to the now unique_ptr.
Please remove those and amend to the respective commits (each commit must build in order to allow git bisect).

In case you still can't build locally I can provide fixups and push to your branch.

[mxmilkiib]

Can do, will be afk for a couple of days tho

[ronso0]

ping clang-format wants to break some long lines.

Unfortunately clang-format is still complaining. I suggest to install pre-commit locally to format every commit automatically. https://github.com/mixxxdj/mixxx/wiki/Using-Git#set-up-automatic-code-checking It will also install clang-format. You may format your whole PR By git clang-format upstream/main (Make sure not to format the whole code base.)

[ronso0]

Why did you change this?
We use the full path for all other includes.

[mxmilkiib]

Ah that was me trying reacting to the red squiggly lines the MS IntelliSense extension in VSCodium put under the path saying the file could not be found.

It's frustrating because I tried the other Clang based debugger mentioned on Zulip a while back and it sucks compared to the MS IntelliSense extension re advising what the cause of and solution for most a true problems are.

I'll fix and force push again in a bit.

[mxmilkiib]

ping clang-format wants to break some long lines.

"The style changes mentioned are inconsistent, I could do the extra line breaks, but it's not asking for them in all cases..?"

Unless I misread then, and I havn't relooked yet - I'll do so after I've done the other thing - it was wanting me to remove line breaks, and for some reason only wanting me to add line breaks to some of the code I'd added thus I assumed that adding further linebreaks would hit me with a requirement to remove those new line breaks too. Maybe I was wrong. I'll suffer trying to fix it multiple times again and it continually giving me inconsistent output until I rage quit and go to bed.

Also, running locally give no problematic output. I guess either I'm doing something wrong or the instructions are inadequate.

Well, it is giving a silly number of "skipped", obviously part of the problem, given it should be giving me an error, but I'm not sure how I've fluffed the process.

[ronso0]

It's skipping because you have no file changes for pre-commit to check.
clang-format has a line length threshold above which it proposes line-breaks (which may sometimes look rather strange to humans).

You can do
git log -1 (just to note the commit message)
git reset HEAD^1
git add --all
git commit
Then pre-commit should run again on the changeset.

(by the way I very much recommend the zsh git plugin, it has many useful shortkeys)

[ronso0]

now it wants to move the header include...

please revert and re-commit as described above, actually pre-commit should only act on changed lines.
then force-push.

[ronso0]

...and trailing whitespaces 😆
You can also download the pre-commit patch from the CI summary (at the bottm of the Summary page https://github.com/mixxxdj/mixxx/actions/runs/8242566098?pr=12934), apply that (git commit --amend) and push again.

[ronso0]

I'm the only reviewer until now and I'm fine with cherry-picking / rebasing and amend the commit of #12941.

[mxmilkiib]

Cool cool.

I was using Git Butler to "undo" and force push the PR commits, now it has stopped working, as I made a mess trying to get multiple feature PRs into the same branch so removed the repo dir and recloned but apparently broke some part of a workflow. I learnt some git years ago but have forgotten most, and never rebased nor force pushed back then, let alone cherry picked commits, so am rather lost in how to explain to myself what I need to do.

I also retrieved both my PRs with gh pr checkout then tried git cherry [sha of main commit in first PR], but that just returned another SHA looking string, and I can't tell if anything changed.

[mxmilkiib]

Ah, git cherry-pick not git cherry.

Lets see how wrong I get this..

[mxmilkiib]

A strange sort order;

Needs a new issue?

[ronso0]

I assume this is the MIDI Input table. Actions are sorted by control, and IIRC that is
D ouble
F our thirds
H alf
T hree fourths
T wo thirds etc

Yeah please file a new issue!

[ronso0]

LGTM, except one tiny yet significant character

[mxmilkiib]

Please take this off my hands, I don't have the capacity to focus on fixing and finishing this PR. Thanks.

[ronso0]

I have just rebased, fixed and force-pushed to your branch.
Hopefully also clang-tidy is happy now.

[ronso0]

I have just rebased, fixed and force-pushed to your branch. Hopefully also clang-tidy is happy now.

Since I worked on this, too, someone else needs to review and merge.

[ronso0]

I've split the commit into the std::unique_ptr changes and the actual feature commit.

[daschuer]

We have a conflict unfortunately

[ronso0]

We have a conflict unfortunately

resolved. There'll be some more #12104 when this is merged 💦

[daschuer]

Thank you.

[ronso0]

Thanks @mxmilkiib for starting this, and their patience.