Zulko / pianoputer

Use your computer keyboard as a "piano".
http://zulko.github.io/blog/2014/03/29/soundstretching-and-pitch-shifting-in-python/
Other
324 stars 92 forks source link

misc updates + v2.0.0 release that can be distributed via pypi #16

Closed spacether closed 3 years ago

spacether commented 3 years ago

2.0.0

Updates From PR Feedback

Zulko commented 3 years ago

Thanks for this! Some requests before I merge:

It looks like you removed the typewriter.kb layout which to me is the best idea in that repo, it is what makes it possible to play the Turkish march. Could you keep that file in there and refer to it in the Readme? A picture of the layout is available there.

The keyboard pictures are a welcome feature. Could you change the yellow to a white/darkgrey pattern so that the piano layout becomes easier to grasp? (keys that are not part of the layout can stay light-grey).

Changes pitch shifting to use librosa, this works for mono and stereo files

Awesome, I'm pretty sure that librosa feature didnt exist a few years back. Apparently they use the same algorithm, with maybe some more quality in the execution (does it work better?). Did you remove the former pitch-shifting code completely? Asking because ther code was referred to by this blog post which now has a dead link. Would it be easy to have the old algo somewhere (available as an option, or in a separate file?)

spacether commented 3 years ago

It looks like you removed the typewriter.kb layout which to me is the best idea in that repo, it is what makes it possible to play the Turkish march. Could you keep that file in there and refer to it in the Readme? A picture of the layout is available there.

The keyboard was not removed, it was renamed to keyboards/azerty_49keys.txt. In the readme I include a description on how to use it in the changing keyboards section. Also, when that keyboard is specified, it keyboard layout image is loaded from here https://github.com/spacether/pianoputer/blob/misc_improvements/pianoputer/keyboards/azerty_49keys.png Does this meet your needs? While the azerty keyboard is great, it only works for azerty keyboards. Most of the keyboards in the world use a qwerty keyboard: The QWERTY layout is, by far, the most widespread layout in use. So I want to optimize the experience for them. When an average user goes to install this through pypi and wants to play the piano on the keyboard, should the default keyboard that we load work for their qwerty layout?

The keyboard pictures are a welcome feature. Could you change the yellow to a white/darkgrey pattern so that the piano layout becomes easier to grasp? (keys that are not part of the layout can stay light-grey).

Not sure what your exact asks are here. This is what I think you are asking, can you confirm this?

librosa... (does it work better?)

I don't know which works better, when doing the update I didn't write back to back audio files for comparison.

Did you remove the former pitch-shifting code completely? Asking because ther code was referred to by this blog post which now has a dead link. Would it be easy to have the old algo somewhere (available as an option, or in a separate file?)

Yes I removed the pitch shifting code because the librosa code does it more simply in fewer lines. If you prefer that I include the old pitch shifting code I can do so. My preference is to keep it in the historic commits because using librosa simpler and makes maintenence on this repo easier for pitch shifting code. The current master branch has 2 code paths for pitch shifting, stereo, and mono. Your suggestion of keeping old pitch shifting code increases us to 3 pitch shifting code paths (librosa, legacy stereo, legacy mono). How about you blog post links to files this commit? That way your links will keep working.

Zulko commented 3 years ago

keyboards/azerty_49keys.txt

Sorry I missed that, I only opened the azerty_45 one and assumed that azerty_49 would be a variant. Could you change the names to azerty_piano_45keys / azerty_typewriter ? (I called the second layout typewriter because it is designed so the two hands never cross). I'm thinking of ways to make the 49keys layout picture more informative (right now almost all the keys are yellow). Maybe by marking the separation between the zone of each hand (vertical bar between T-Y, G-H, B-N).

Not sure what your exact asks are here.

The idea is to make the piano keyboard layout clear in that 45keys picture. So the white keys of the piano would appear white. The black keys of the piano would appear dark-grey (not completely black so we can still see the letters) and the rest of the keys would be grey.

How about you link to files this commit in your blog post, that way your links will keep working.

That will sound lazy but I'm not even sure I can update this blog anymore, it was built with a now-mostly-deprected framework years ago. I don't want that to be in the way of this repo becoming a bit more than a blog illustration though. Let's move on with your code and I'll figure out my blog later.

Zulko commented 3 years ago

While the azerty keyboard is great, it only works for azerty keyboards. Most of the keyboards in the world use a qwerty keyboard: The QWERTY layout is, by far, the most widespread layout in use. So I want to optimize the experience for them. When an average user goes to install this through pypi and wants to play the piano on the keyboard, should the default keyboard that we load work for their qwerty layout?

Is there a default keyboard? If so I am fine with it being qwerty if that makes your life easier. One caveat on QWERTY keyboards is that there are subtle differences between UK/US/... and Mac/PC/... keyboards when it comes to the special keys (=non-letters). I think the idea here is that users with different keyboards would contribute different layouts that work for them.

spacether commented 3 years ago

Sorry I missed that, I only opened the azerty_45 one and assumed that azerty_49 would be a variant. Could you change the names to azerty_piano_45keys / azerty_typewriter ? (I called the second layout typewriter because it is designed so the two hands never cross). I'm thinking of ways to make the 49keys layout picture more informative (right now almost all the keys are yellow). Maybe by marking the separation between the zone of each hand (vertical bar between T-Y, G-H, B-N).

Sure, happy to change the file names. I am not seeing a 45 key azerty layout. Also I am not seeing a second keyboard layout in the master branch of your repo. Should there be two azerty layouts?

The idea is to make the piano keyboard layout clear in that 45keys picture. So the white keys of the piano would appear white. The black keys of the piano would appear dark-grey (not completely black so we can still see the letters) and the rest of the keys would be grey.

Ah, I understand now, thanks. Happy to change the key colors. Doing that poses a difficulty. The wav file sets what the anchor key is at. So that AND the key file determine where the black keys are at. I see a couple of solutions.

What do you prefer?

Is there a default keyboard? If so I am fine with it being qwerty if that makes your life easier. One caveat on QWERTY keyboards is that there are subtle differences between UK/US/... and Mac/PC/... keyboards when it comes to the special keys (=non-letters). I think the idea here is that users with different keyboards would contribute different layouts that work for them.

Yup, the qwery keyboard that I included in the readme is now the default. Ah, good to know that, thanks. Yup I think that having users contribute their own keyboards would be great.

Future thoughts: One limiting factor that came up was displaying a keyboard I think that a keyboard plotter should be added, then we can stop using static image (png) assets One could load a file containing keyboard info including:

Zulko commented 3 years ago

Some questions/remarks:

spacether commented 3 years ago

To me it's not that important that a particular key of the keyboard corresponds to a fixed note.

This makes sense to me also. I understand that users may want to load in audio files that are different notes than c, or frequencies that are in between or above or below piano key notes.

About the keyboard file though, no data exists in the keyboard file describing what keys are black and white. That's why I suggested adding an anchor key descriptor like 'c'. That defines what keys are black and white. Does that make sense to you?

Not all original samples will be at the same height on a keyboard.

Yup, I understand that.

It is possible to have the keyboard layouts defined in JSON/CSV files and have them programatically rendered in Python (e.g. with pygame, which would also allow to highlight the keys pressed). But for this kind of features I am more pursuing the JS version of the pianoputer.

That is only possible if a keyboard plotter is added like I described in this comment The information that we are missing is:

Could you describe how the qwerty layout is organized? Is "Y" not a key?

Sure thing. Y is not a key, that's right. The keyboard image is now updated to show black and white keys here The row above the space bar (left shift to right shift) includes white keys c to g. If you want to play keys lower than that move your left hand to the top left of the keyboard and continue down on white key t. If you want to play keys higher than that move your right hand to the top right of the keyboard and continue up on white key u. This qwerty layout attempts to preserve keyboard shape and keep the hands away from eachother.

spacether commented 3 years ago

closing and reopening to try to fix line counts. I marked svg as binary

Zulko commented 3 years ago

I haven't had time to think too much about it but your piano layout sounds very interesting (do you have any videos of it in action?) and I don't want to get in the way of trying new things. The rest looks very clean and understandable to me, so feel free to merge.

For the future, would you be interested in getting maintainer access to this repo? (and maybe mark yourself as main developer since v2.0?)

spacether commented 3 years ago

Hey there. My piano skills are not great. If I have time in the future I can record a new video. Because my repo is a fork of yours only you are able to merge it. Can you merge it? Yes, in the future I would be happy to be listed as a maintainer for your pianoputer repo and on pypi if you want. Also, if you are okay with waiting a couple of more days I am building a library to plot keyboards here: https://github.com/spacether/keyboardlayout And we can use that in this library to dynamically color keys.

spacether commented 3 years ago

Converting to draft while I integrate keyboardlayout

spacether commented 3 years ago

So I am having trouble testing the azerty keyboard. On my windows mac I am able to switch to a French PC layout but I am not sure that it is working correctly for the number keys. Pygame sees them as 1,2,3 and all other program sees them as the expected &,é," @Zulko can you help me by testing keyboardlayout? I need to know if your & é ” ’ ( - è _ ç à ) = keys (number keys for qwerty) show up as red when you press them when running python samples/pressed_keys_pygame.py azerty_laptop from this branch? Once I have this answer I can finish getting the azerty layout plotting working.

Zulko commented 3 years ago

I can help in ~1 week, I think it's fine if it's broken in the meantime. This looks globally good to me so I'll make you maintainer and feel free to merge when you feel it's ready! :+1:

spacether commented 3 years ago

Azerty layout has been updated: azerty

spacether commented 3 years ago

@Zulko I am almost ready to merge. Who do you want to distribute it on pypi, you or me? You are marked as the author and I am marked as the maintainer here

spacether commented 3 years ago

All set, merging