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

Piano roll scale note indicators #477

Closed
wants to merge 1 commit into from

Conversation

polygon
Copy link

@polygon polygon commented Feb 26, 2020

My first Ardour contribution, hello everyone.

This patch allows to set the root note and scale on midi tracks (New "Signature" submenu in the context menu for a midi track right next to "Note Mode"). It will then replace the piano roll background colors to show which notes are in/out the current scale and where the root notes are. It's functional, but still pretty rough. I hope to get some feedback if this fits with Ardour, how to improve the code, and the functionality itself. Some points:

  • Should this be done per track as it is now or per MIDI segment in the track
  • I took NoteMode as a blueprint when implementing this. However, I am not sure if I need to have this in midi_track of libardour at all. Would it be fine to just save it in the gtk2_ardour GUI components?

Having the key and scale information might also aid further features such as a chord assist.

@pauldavisthefirst
Copy link
Contributor

This is exciting and interesting. However, we're on a push toward 6.0 right now, and there's no chance of this being included in that release. I hope you can be patient in awaiting our review and comments while the 6.0 process takes place.

@polygon
Copy link
Author

polygon commented Feb 29, 2020

Thanks for the response. I totally understand, at some point you just have to stop accepting new features to get the release out. I'll keep watching for 6.0 and once it comes around I'll update this feature to be based on the most recent source.

@x42
Copy link
Member

x42 commented Feb 29, 2020

Thanks for getting the ball rolling!

A few comments from briefly looking at this:

  • There is a static function with ParameterDescriptor::midi_note_name() that already offers translated note names.
  • I think hardcoding scales is not a good idea, Scales should be dynamically loaded and extensible. So I'd not define enums for root notes and scales. That is limiting down the road.
  • The code should eventually be consolidated with libs/surfaces/push2/scale.cc (that also currently hardcodes scales). There are plans to overhaul control surface code after 6.0 and make functions re-usable.

Adds the option to select Root Note and Scale Type for MIDI tracks.
Will when color the notes of the piano roll depending on whether or
not they are in the key. Also, the root notes will be colored
differently.
@polygon
Copy link
Author

polygon commented Dec 2, 2020

Oh, this seems to have gained some interest over the past months. I did some cleanups of the code, removed storing the scale information in the actual midi_track, since this seems to be unnecessary, and rebased everything on top of the current master branch.

Right now, I am offering just twelve options for the root note. This should be fine for indicating which notes are in the scale. But I could imagine this to also be useful as a basis for other tools, such as a chord assist. Bottom line is, would it be useful to actually differentiate between, e.g. F# and Gb as a root note?

@ProofOfKeags
Copy link

I hope this gets some more attention soon. This is currently the most frustrating part of using Ardour, for me right now.

As for #/b distinction: Useful? Yes. Strictly Necessary? No. When I collaborate with friends we use ableton and ableton casts Bb as A# even in Dm and while it looks weird, it doesn't impede my ability to compose the way that the lack of a piano roll does.

@pauldavisthefirst
Copy link
Contributor

We're not ignoring it, but both of the two main developers are deeply immersed in other much more major work right now. For myself, I won't take a look at this until after the nutempo2 branch is finished, merged and more or less ready for release.

@pauldavisthefirst
Copy link
Contributor

I've reimplemented all the functionality in this branch from scratch. Not because this was actually wrong, but I had some other goals in mind. In particular, the root/mode are properties of the track itself, not the GUI side of things. It's currently in a branch (mode-clamping) but will likely merge soon. I have no idea what the best color for the out-of-key lines are :(

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.

4 participants