Skip to content

[POC] Add downbeats to waveform #14835

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Conversation

alephlm
Copy link

@alephlm alephlm commented May 23, 2025

Current UI:

current_2deck

Modified UI:

downbeat_2deck

There are 3 new buttons:

  • toggle downbeats visibility
  • move downbeats forward
  • move downbeats backward.

Information, Implementation details and doubts

First of all, when reviewing take in consideration that I am mainly Javascript developer. Just tired of getting the down beat wrong, then I faced c++ here. And this implementation already solve almost all my use cases.

The main concept here is the creation of a new waveformrenderdownbeat class, this is basically a copy of waveformrenderbeat but only rendering the down beats. Doing this way is less disruptive and is not messing with old code.

  • It shows 4 and 8 beats differently, 8ths are generated with little triangle on top.

  • There is a new skin parameter for setting the downbeat color.

  • This will work only on 4/4 songs right now.

  • This will only work with [x] use acceleration

  • It needs a way to store the downbeatoffset after user move it.

__

  • I needed to get the absolute position of the beats, to do that I exposed m_beatOffset from beats.cpp. Is there already other way to do this?
  • I am adding the downbeat into waveformwidget.cpp. To control visibility I am just toggling the opacity. There is maybe a quicker anyway to do? Directly removing it instead?

@acolombier
Copy link
Member

Great to see some traction on this much needed feature!
You might want to have a look at #13330 to see points that will be expected for such a feature to be considered ready for merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants