Skip to content

Conversation

dhanuarf
Copy link
Contributor

This PR adds semitone increment and decrement when changing pitch in playback option.

The UI is slightly modified, padding is added on the left side just to create some kind of "separation" between Playback speed and Pitch.

Slight downside: stepSize in Pitch slider is removed to accommodate the changes.

screen-20250728-235845.mp4

@dhanuarf

This comment was marked as outdated.

@dhanuarf dhanuarf changed the title feat: add semitone increment & decrement in playback pitch changing option feat: add 'semitone' function when changing playback pitch Jul 29, 2025
@dhanuarf

This comment was marked as outdated.

@dhanuarf

This comment was marked as outdated.

@Bnyro
Copy link
Member

Bnyro commented Jul 31, 2025

I'm not sure if it's a good idea to keep the old pitch setting because it might be confusing to have different ways to set the pitch.

Perhaps using semitones is more intuitive, so at least in my opinion we should drop the old preset values and change the slider to use semitone steps as well instead of using the pitch percentage.

Opinions @dhanuarf @FineFindus ?

@FineFindus
Copy link
Collaborator

Perhaps using semitones is more intuitive, so at least in my opinion we should drop the old preset values and change the slider to use semitone steps as well instead of using the pitch percentage.

Agreed, having two UI elements for editing the same value is pretty bad UX. I think having the slider is better, considering the value is discrete and bounded (i.e. it cannot be freely edited).

On a more general note, what's the use case for changing the pitch? I assume it would be for manual adjustment when changing speed, but maybe that should better be done automatically?

@dhanuarf
Copy link
Contributor Author

dhanuarf commented Aug 1, 2025

Perhaps using semitones is more intuitive, so at least in my opinion we should drop the old preset values and change the slider to use semitone steps as well instead of using the pitch percentage.

I do agree of removing the preset. But changing the slider back to discrete with semitone steps, I personally prefer the slider to be continuous. But this is your project so the decision is up to you.

The reason I prefer continuous slider is because for more accurate pitch tuning. There are some songs that are tuned differently than the typical tuning. For example, standard tuning is typically tuned with the note A4 at 440hz. But there are some songs where the note A4 is tuned to 432hz. So if your musical instrument is tuned with A4=440hz, the songs sound a bit out of tune.

With a semitone calculator like https://www.omnicalculator.com/other/semitone, you can calculate the semitone difference and then enter the value in the app.

But this feature probably will only useful for some music nerds, not general people.
So yes, in most app, for general use to most people, changing the semitone in +-1 increment is good enough.

So again, the decision is yours, I'll just follow along.

@dhanuarf
Copy link
Contributor Author

dhanuarf commented Aug 1, 2025

On a more general note, what's the use case for changing the pitch? I assume it would be for manual adjustment when changing speed, but maybe that should better be done automatically?

I'm not sure about general cases, but at least in my case, as someone who plays musical instrument, I like changing the playback pitch when I want to play along to some songs that have different tuning than my instrument. Or maybe there are others who want to sing along but the song is out of their vocal range so they adjust the playback pitch to fit their vocal range better.

Or maybe there are some others who are sensitive to high-pitched sound, so they tune down the playback pitch for comfortable listening.

Or maybe there are some people who just like to change the playback pitch just for gigs and giggles, I'm not sure.

But yeah, the ability to change the playback speed is enough for generic usage.

@dhanuarf
Copy link
Contributor Author

dhanuarf commented Aug 1, 2025

deacd72 update:
The playback pitch slider value is now in semitones, but the slider itself is still in continuous mode.

Also, should we change back the playback speed to decimal values or keep them at percentages?

@Bnyro
Copy link
Member

Bnyro commented Aug 1, 2025

But changing the slider back to discrete with semitone steps, I personally prefer the slider to be continuous. But this is your project so the decision is up to you.

I agree, semitones might be too big steps for some people, so a continuous slider makes sense.

Also, should we change back the playback speed to decimal values or keep them at percentages?

I prefer decimal values because that's the way most other apps do it as well, so it's more intuitive.

@dhanuarf dhanuarf changed the title feat: add 'semitone' function when changing playback pitch feat: change 'playback pitch' option to semitone mode Aug 1, 2025
@dhanuarf
Copy link
Contributor Author

dhanuarf commented Aug 2, 2025

5a364ff changes the minus&plus button into musical flat&sharp button.

I tried to use the unicode version of these symbols to simplify things, but they look ugly. So I replace them to use icons.

Screenshot_20250802-100443_LibreTube Debug

Copy link
Member

@Bnyro Bnyro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

I've just adjusted the margins and padding a bit to make everything look more consistent, apart from that lgtm.

@Bnyro Bnyro merged commit 1dabdf1 into libre-tube:master Aug 2, 2025
3 checks passed
@dhanuarf
Copy link
Contributor Author

dhanuarf commented Aug 2, 2025

Actually, just right now I was just looking for a way to turn the slider into centered like in here: https://m3.material.io/components/sliders/overview, but cant seem find a way to do that.

@Bnyro
Copy link
Member

Bnyro commented Aug 2, 2025

Actually, just right now I was just looking for a way to turn the slider into centered like in here: https://m3.material.io/components/sliders/overview, but cant seem find a way to do that.

That's only possible after upgrading to MD3 Expressive afaik, see the WIP PR #7444

@dhanuarf
Copy link
Contributor Author

dhanuarf commented Aug 2, 2025

That's only possible after upgrading to MD3 Expressive afaik, see the WIP PR #7444

Ahh, so thats why.

I was so confused because this centered slider update is done in Dec 2023, and the material library we are using is 1.12.0 which is released in May 2024, got me wondered why wont this work

@dhanuarf dhanuarf deleted the add-semitone branch August 4, 2025 23:11
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.

3 participants