Add code for the PianoBooster metronome

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

Add code for the PianoBooster metronome

Louis B.
Administrator
This post was updated on .
Hi all,

For the background on this please read this post http://piano-booster.2625608.n2.nabble.com/Preview-of-the-new-piano-booster-metronome-build-td7572912.html 

And also see these two issues https://github.com/pianobooster/PianoBooster/issues/71 and  https://github.com/pianobooster/PianoBooster/issues/292

The idea with the metronome is that it can also double up as complete drum track that is easy to customize.  please see the files in this dir metronome-patterns
Reply | Threaded
Open this post in threaded view
|

Issue #292 Reply (contributing)

mark-Dutton
Hi Louis,

Might take a look at the code and get familiar with it first but if no. 2 is a wip could look at adding something to that. Have downloaded the wip/metronome branch onto linux and got it up and running.
Is it just the GUI + customization settings of the metronome that needs to be done?

Reply | Threaded
Open this post in threaded view
|

Add code for the PianoBooster metronome

Louis B.
Administrator
Might take a look at the code and get familiar with it first but if no. 2 is a wip could look at adding something to that. Have downloaded the wip/metronome branch onto linux and got it up and running.
Is it just the GUI + customization settings of the metronome that needs to be done?
I have just merge the latests dev code into the metronome branch.

I have found a draft UI that I created for the metronome using  QT 5 designer . However before I post it to you I would be interested to see your ideas of how it could work. Just a pencil sketch is fine or perhaps a screenshot from another app the implements a metronome in a way that you like.  Otherwise I can just post it to you if you prefer.

The work that needs to be done is as follows:
0. Agree on a UI design for the metronome
1. connecting the metronome QT 5 designer to the PB app.
2. adding a toolbar icon to turn the metronome on and off.
3. Customization settings of the metronome (I was thinking of adding a $BEAT_SOUND and a $BEAT_VOLUME to this file PianoBooster/src/metronome-patterns/Metronome_Clicks.txt
3. Getting a metronome volume slider to work. (This is tricky but I first need to revert the hacks I made to conductor.cpp)
Reply | Threaded
Open this post in threaded view
|

Re: Add code for the PianoBooster metronome

mark-Dutton
Update:
So I've added a simple metronome button to the GuiTopBar. I haven't really looked around too much but I thought this one went okay with the rest of the icons:


which I found on this website: https://www.iconfinder.com/icons/4937148/instrument_metronome_music_musical_pace_pendulum_sound_icon (might need to purchase before use)

Still have to implement the actual functionality of the metronome to the on_metronome_clicked function I added but I implemented a simple toggle where the background colour of the button is set to blue (using the QPalette) when metronome is on and just keeping the image the same.

For off:


For on:


And when you hover over the button it reads: "Start and stop Metronome"

The only problem is the background highlighting adds ugly looking borders as the icon is a rounded png. Might have to add another image with some blue highlighting or play around a bit more with the qt style sheets.
Reply | Threaded
Open this post in threaded view
|

Re: Add code for the PianoBooster metronome

Louis B.
Administrator
Yes I really like that icon thanks it fits the PB style well. Yes we have to purchase before use, which I am fine with. Alternatively googling for "open source icons metronome" comes with some good results.

I found that I have included GuiMetronomeDialog.ui in the sourcetree for that branch see this screenshot.



I thought the Style: would list out all the files in the metronome-patterns dir.

I am not sure about the OK and Cancel buttons they are rather old fashioned now. They could be replaced with a "Defaults" button. Also I am not sure if  "sub beat": is needed.

Please feel free to rework that GuiMetronomeDialog.ui it is just my ideas so far. I don't like the activate metronome check box. It should ideally be replaced with a matching toolbar Icon.
Reply | Threaded
Open this post in threaded view
|

Re: Add code for the PianoBooster metronome

mark-Dutton
So where exactly should the GuiMetronomeDialog be added in PB. Should it be added as a separate button in the setup dropdown toolbox?
Reply | Threaded
Open this post in threaded view
|

Re: Add code for the PianoBooster metronome

Louis B.
Administrator
I was thinking that it could be added to the menu -> Song -> Metronome. Underneath the menu -> Song ->  "Song details" menu entry. I was also thinking of adding new entry as follow menu -> Song -> Repeat Bars as the  repeat bars drop down arrow on the toolbar is rather difficult to find. ( I don't think we need a dropdown arrow on the tool bar which would brings up the Metronome dialog. )

I am going to be offline for the next few days but after that we could have a video call if you are interested. I will PM you about a possible call. (Please check your spam folder)

Louis
Reply | Threaded
Open this post in threaded view
|

Re: Add code for the PianoBooster metronome

mark-Dutton
This post was updated on .
Hi Louis,
I have added the metronome menu to the songs tool tab and got full functionality for customizable bar and beat sounds and velocities of the metronome, as well as the metronome on and off functionality.

For the metronome sounds I have added all the possible notes for the General MIDI Percussion channel (on Channel 9) available to choose from in the drop down widget for both 'bar' and 'beat' combo boxes.

For velocity, bar and beat sounds are customizable for the range 0, 127.

Just need to add functionality for preview sound, activate metronome tick box, volume control and style. If you were thinking of getting rid of sub beat I wasn't planning on implementing the functionality for that but will if you like. but both the sounds and velocities are fully working and you can get different instruments running the metronome at different velocities.

I have been running on Linux build and I could submit a pull request or email you the build for you to look at the progress made if you like but I plan on getting all the functionality for metronome done pretty soon (most likely next couple of days).

Here are a few screen shots:


click on metronome tab:


click on combo box for either bar or beat:


selection made:


Reply | Threaded
Open this post in threaded view
|

Re: Add code for the PianoBooster metronome

Louis B.
Administrator
Hi Mark,

Oh wow that looks great thanks so much. I am going to be away on the 6-8 April, but after that I could look at it. A PR to the metronome branch would probably be the easiest or for work in progress code I could look at your fork of PB if you push the code there and make it public.

Just so you know I plan to do a new release with the metronome feature after I have first done a couple of youTube videos about PB.
Reply | Threaded
Open this post in threaded view
|

Re: Add code for the PianoBooster metronome

mark-Dutton
Hi Louis,
So I have powered through and finally got all functionalities finished. I have submitted a pull request for you to review.
Changes made:
1. Metronome button on/off
2. Metronome Settings menu
3. Metronome Bar and Beat Sounds
4. Metronome Bar and Beat Velocities
5. Metronome Volume
6. Metronome Preview
7. Metronome Tick Box
8. Metronome Cancel button to revert changes back to previous state
9. Metronome ok button to save new changes

I have also made some major changes to the CConductor::calcBoostVolume() function as well as the piano slider and boost slider functions. The volume of each channel can now be adjusted separately using the boost slider, when you switch the active channel the boost slider will snap up to the current volume of that channel and you can make adjustments to it as you wish. This fixes wip's I saw within that function. Brief overview of how it works:

I set each channels volume to the midpoint of 0 and the maximum midi volume possible. So the volume will be set to MIDI_MAX_VOLUME/2 = 127/2 = 63 on initialisation. This means that all instruments will have equal volume but some volumes may be increased/decreased.

The volume can be changed for each channel using the booster slider,
and the slider value will be normalized/scaled between the values 127 and 0.
So if the slider is at -100 the volume for the active channel will be 0,
100 volume will be 127 and at its midpoint volume will be 63.

When the active channel is changed on the side bar, the boost slider will also snap to the saved value of that channel.
I also fixed piano volume slider which was hardcoded at 127. It follows the same normalization as other
sliders. The metronome slider follows this process as well.

It has been very enjoyable working on the code in the afternoons. If you have any questions about the code in the pull request I can go into more detail for the different segments.
Feel free to look at any of this code whenever you feel like it. I might leave off coding in the afternoons for a while as I need to catch up on the uni work but it has been fun working through this project.
Reply | Threaded
Open this post in threaded view
|

Re: Add code for the PianoBooster metronome

Louis B.
Administrator
OK Mark,

Thank you very much for doing this,

I have compiled and run the code and had a very brief look at your code changes in your PR.  I noticed a couple of things. Which I have marked on the source code on the "files changed" tab on your PR. see https://github.com/pianobooster/PianoBooster/pull/295

My guess is that you are not familiar with using git and unfortunately it is rather tricky and difficult to learn but it is worth the effort as git is used in most companies that are developing software.

Unfortunately I caused you problems when I said this: "I have just merge the latests dev code into the metronome branch" . What you should have done immediately is  `git stash` followed by `git pull` and then followed by the git pop commands. Maybe we can have a call about the best way to fix this as it is quite complicated.

Also regarding conductor.cpp I wrote "I first need to revert the hacks I made to conductor.cpp". Unfortunately only the code in conductor.cpp in the develop branch is valid and has the working functionality for the two sliders on the left hand side of pianobooster. The top left hand slider is for the "Adjust the volume of your piano" and the bottom slider  is for "Adjust the volume of the selected part". Unfortunately I made a temporary experimental hack to conductor.cpp in the metronome branch and broke the top slider and so it does need to be reverted before you make your changes. I haven't looked at or tested any of the volume changes you have made yet.

Finally when I tested the functionality by running the code in your branch I noticed a couple of things: the second time I started the metronome dialog box the drop down box for the beat had double the number of entries that it should have. And the preview button did not work for me.

You wrote "I might leave off coding in the afternoons for a while as I need to catch up on the uni work" Also i will not fully review this PR until later as I first want to post a couple of YouTube videos about PB before then.

Oh dear this is all sounding very negative but actually I am very pleased with what you have done.
Reply | Threaded
Open this post in threaded view
|

Re: Add code for the PianoBooster metronome

mark-Dutton
Hi Loius,

Yes this is true I am not very familiar with git and I apologize if any issues have arisen from this. The only files that
should have been updated are the source files, the cmake config file and the images folder. All of these files are within the src directoy
of the piano booster project if I am not mistaken. But I also had to add metronomedialog.cpp and metronomedialog.h classes to control
the metronome gui as is standard procedure with qt. That is to say no files outside of the src directory should have been modified.

To address the concern you raised with my changes to the boost slider/volume and piano slider/volume:

Firstly I would say if you do not like the changes I have introduced then they can be removed from the pull request but
I would like to give a bit more explanation:

I have a done a diff on the conductor.cpp file for the devlop branch and the wip/metronome
branch and the function: CConductor::calcBoostVolume(int channel, int volume)
follows the same logic minus any added metronome code that was wip. The slider is valid and works
on both branches, however there was a problem when you turned up the volume of the active channel,
the volume of all other channel decreased, so if the active channel volume was at max, all the other
channels were muted. I'm not sure if this is an intended feature but my change allows for each volume
to be changed independantly.

The changes I have introduced are agnostic of the previous method that was in the works.
Instead of adjusting the volume within this function I have simply
done a normalization on the slider value to fit its min/max values (which are -100 to 100) within
that midi main_volume range of 0 to 127. So when the boost slider value is changed, the callback function will
first normalize the slider value and then call the setChannelVolume(int volume) function
which will apply the new volume to the active channel that is selected.
The piano slider and metronome slider follow the same logic of scaling the slider value to the functionali midi range of 0 to 127.

Also in the develop branch, the CConductor::outputPianoVolume() function is identical to that of the
wip/metronome branch and the volume is still hardcoded at 127. All I have really done is change the volume variable within that function
to be set to the m_pianoVolume value.

As for the error you found:
"the second time I started the metronome dialog box the drop down box for the beat had double the number of entries that it should have."
Yes this is an error that I had overlooked but I should be able to fix with relative ease.

But for the preview button not working this is only when the metronome is not activated. This also
is a bug but only for when the metronome is not set to active. When it is active the preview button works.
And if you push the cancel button, the changes you made are rolled back even if you have previewed the
metronome using a different bar or beat sound / volume / velocity.

Part of this pull request was not to introduce an error free merge into the metronome branch but to show you all the functionality
working and get some feedback on it. The most valuable feedback I was looking for and intended to recieve before going
further along with this branch was whether the actual code that I have added to the code base and the features that are now working
are what you had in mind. Feedback such as your worries with the changes to CConductor::calcBoostVolume(int channel, int volume)
is what is most vital to me as I have been very much working on this code with a blindfold on and had to figure out what was/is the original
intention of certain functions/features.

With all of that said, I am eager to get this metronome branch fully functional and work with you and other github contributers to
achieve this. Though for now my input will be at a lot slower rate as I get my head back into the uni books.
Reply | Threaded
Open this post in threaded view
|

Re: Add code for the PianoBooster metronome

mark-Dutton
Hi Louis,

I also just remembered now that the metronome icon used in the pull request is still the same one that requires licensing. Please refer to the dev forum discussion we had but had intended to find a new image that was fully open source.

kinds regards,
Mark