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

Velocity automation #356

Merged
merged 44 commits into from
Feb 22, 2017
Merged

Conversation

elpescado
Copy link
Contributor

@elpescado elpescado commented Feb 26, 2016

Overview

This branch allows users to adjust velocity of played notes via a timeline called "automation path":
screenshot
audio

That path allows to create more complex songs using fewer patterns. It works especially nice with multi-layered drumkits.

Features

  • Support for built in playback (sampler)
  • Support for MIDI export
  • Saving and loading songs
  • Full undo&redo support
  • Unit tests

Prerequisites

This branch makes conserative use of C++11 features so it needs C++ support enabled (see pull request #263).

Regressions

This branch should be backwards compatible. Songs created before will get constant velocity adjustment=1.0, which means note velocities will remain unchanged.

How to test

  • Check whether it's possible to add points to velocity automation path, move and delete them
  • Check whether velocity is adjusted in playback in Song mode
  • Check whether notes are played with their nominal velocity in Pattern mode
  • Check whether note velocity is adjusted according to velocity path in exported audio file
  • Check whether note velocity is adjusted according to velocity path in exported MIDI file
  • Check whether after saving song with velocity automation path it's possible to load that song file, and velocity automation is correctly loaded

elpescado added 24 commits June 13, 2015 22:16
@trebmuh
Copy link
Member

trebmuh commented Feb 28, 2016

This looks awesome ! I'll be trying to test it as soon as possible.

Does the velocity changes in the pattern part are still working ? How do you handle those ?

@elpescado
Copy link
Contributor Author

Yes, it's still working. Final note velocity is computed as note own velocity * velocity adjustment from timeline * humanize factor. Range for velocity adjustment is [0, 1.5].

@trebmuh
Copy link
Member

trebmuh commented Feb 28, 2016

Hi @elpescado , I've been trying to build hydrogen from a fresh git clone ( 956fd11 ) with the addition of the 2 patches : https://patch-diff.githubusercontent.com/raw/hydrogen-music/hydrogen/pull/356.patch and https://patch-diff.githubusercontent.com/raw/hydrogen-music/hydrogen/pull/263.patch
but it fails building.
What would be your suggestion?

@trebmuh trebmuh mentioned this pull request Feb 28, 2016
@elpescado
Copy link
Contributor Author

What errors are you getting?

One thing I can think of is that this branch requires C++11 support from compiler. I created pull request which enables it, but it hasn't been merged yet: #263

@elpescado
Copy link
Contributor Author

By the way, what OS/distro and which version of gcc/clang are you using? I have successfully built this branch on Debian unstable (with #263 applied before).

@trebmuh
Copy link
Member

trebmuh commented Dec 9, 2016

@elpescado @0-wiz-0 : I'm not too sure to understand what happened here. There looks to be just a merge with current master branch, but I wonder if you reckon it would need another testing round. Please, advise.

@elpescado
Copy link
Contributor Author

Hi, I merged current master to keep this branch fresh, so I don't think it needs more testing.

@trebmuh
Copy link
Member

trebmuh commented Dec 9, 2016

Ok, thanks.

@mauser : what do you think about merging it to master (only being curious here) ?

@mauser
Copy link
Member

mauser commented Dec 11, 2016

Hi!

Yes, i can merge this soon! (No time to test at this moment). It would be great if a label/description could be added to the left of the automation path (or a combo box later) which tells the users what this blue line is all about.

@trebmuh
Copy link
Member

trebmuh commented Dec 12, 2016

@elpescado : would you be able to add a "volume automation" label or something around this line?

@pvint
Copy link
Contributor

pvint commented Dec 13, 2016

I've been following this for a while, and I will admit that I while haven't tried it yet, this sounds really great. I will really try to test this out soon.

@elpescado
Copy link
Contributor Author

elpescado commented Dec 13, 2016

Adding label is a great idea, I'll add it. I haven't really thought about it, for me it was obvious, but for first-time user, showing just blue line must indeed be confusing.

@elpescado
Copy link
Contributor Author

Hi, I added label (in form of LCDCombo) to velocity path view and fixed bug that caused crash when removing points in path.

@elpescado
Copy link
Contributor Author

Hi,

Is there anything that is blocking this pull request? In case anyone is interested in testing this feature, I have added "How to test" section in PR with test scenarios that came to my mind.

@mauser
Copy link
Member

mauser commented Feb 17, 2017

Hi,

tried to reach you via mail this week, but i suppose sth. went wrong if you haven't received it. I was busy for some time but will work on this one next week, maybe we can also put a first beta1-version out then...

@mauser
Copy link
Member

mauser commented Feb 17, 2017

... and of course it would be much appreciated if someone wants to test this feature :)

@mauser mauser merged commit b74b0de into hydrogen-music:master Feb 22, 2017
@mauser
Copy link
Member

mauser commented Feb 22, 2017

Thanks @elpescado , the branch is merged now! I will propably make the automation area hideable via the "View" menu or sth. similiar

@trebmuh
Copy link
Member

trebmuh commented Feb 27, 2017

@elpescado
Copy link
Contributor Author

Whoops, that's embarassing:| I'll have to look into this.

@trebmuh
Copy link
Member

trebmuh commented Feb 27, 2017

@elpescado : no worries, let me know when/where I can help (testing mainly).

@elpescado
Copy link
Contributor Author

elpescado commented Feb 27, 2017

Hi,

I have managed to find some time today. Fortunately, the "dots from outer space" bug turned out to be easy one - being able to drag points outside left margin was beacuse of comparing signed and unsigned integers. I have fixed it in my branch, elpescado:bugfix/automation (commit dbeaad7).

I'll look into "dots eating dots" bug when I find some more time. I'll create a pull request then.

@trebmuh
Copy link
Member

trebmuh commented Mar 1, 2017

Hi @elpescado : thanks for having tackled the "dots from outer space". That was the one I referred in my message. I'll try to find sometime soon to rebuild hydrogen with your fix and will report back to you.

About the "dots eating dots", well, ... I thought it was a feature !

Cheers.

@elpescado
Copy link
Contributor Author

I am confused now;) Dots-eating-dots were by design, but after your report I thought it wasn't fortunate design. I have to think about it more... In the meantime, I have created pull request for the first bugfix.

@trebmuh
Copy link
Member

trebmuh commented Mar 2, 2017

I am confused now;) Dots-eating-dots were by design, but after your report I thought it wasn't fortunate design. I have to think about it more...

So it's fine. I think it is a good feature. No need to change it.

In the meantime, I have created pull request for the first bugfix.

I'll be rebuilding hydrogen with adding this patch and will report back to you after.

Thanks!

@trebmuh
Copy link
Member

trebmuh commented Mar 2, 2017

Rebuilt and tested. All good, thanks @elpescado !

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.

6 participants