Option to adjust playback speed (tempo) via gesture on main player screen

[klaviartur]

Add third gesture area in the middle third of player screen
Add third gesture option: tempo / playback speed
Add settings item for middle gesture

[asandikci]

Things before merge to extended branch

• Understand code (maintainer / @asandikci)
• build with CI
• Test apk (maintainer)
  • ensure this PR does not cause any error
    • couldn't find any further bug / fatal error
  • ensure this PR does not cause any incompatibilities (between versions)
    • couldn't find any related bug
  • Test other sides and few combinations (left/right)
    • couldn't find any related bug
• allow users to change settings related with this PR
• [solve bug]: resetting speed when menu opens while unhook unchecked
  • solved, thanks !
• Mark related settings with a symbol (E)
  • (E) refers to extended
• How to handle translations? (maintainer)
  • closing because already noted in Find a way to properly handle new translations #28

Open related issues after merge

• [feature request]: both pitch and tempo should change with middle gesture if unhook unchecked
  • added with this PR, thanks !

[klaviartur]

Hey, thanks again for the quick reply!

Unfortunately you are right and this bug is there.
I personally view the coupling of pitch and tempo as a legacy feature. I know of no single prson who uses it. It's not a coincidence that decoupling is the default setting.
And the coupling is broken anyway the moment you exceed 2x, which corresponds to the limit of 12 half-tones, and 0.5x respectively, which is -12.

Also I wonder, if you are planning to respect the coupling in the hold-to-speed-up feature, you seem to plan to implement.

If it was for me to decide, I would get rid of this check box entirely.

But to the point:
My initial main goal for opening this PR was to have it merged to main and thereby have it available in the automatic updates that will hopefully eventually be coming via Izzy-On-Droid repo, as opposed to rebuilding my own forked apk everytime something changes upstream.

Therefore I did take about 2 days of my time to code it cleanly with all bells and whistles (except for pitch coupling apparently).
If it was only for my personal use, I could have stopped after 3 hours.
Fact is, i don't have 2 more days to fix the bug, at least not shortly, and also no motivation to do so, as my main goal of automatic updates will be missed anyway.

As I mentioned earlier, this is my very first PR and I was hoping for a hole-in-one kind of thing. Too bad.

I don't know, what that means for this PR. Probably you'll just close the case. I would if I were you.

So it seems I will have to build ma own apk once in a while.

If you see any other route, this PR could go, feel free to contact me :)

PS: Maybe I would take 2 days to exterminate that checkbox, if you asked me to do so ;)

[asandikci]

Okay, okay. I agree the unhook checkbox is not very useful. But solving mentioned bug is more meaningful than removing this button completely.

Sorry for being annoying but I want to keep this app stable ass possible even for extended branch. So at least this bug should be solved.

Also I plan to use a release cycle like florisboard project: release both versions same time. Master branch publishes to F-droid repo, extended branch publishes to Izzy-on-Droid repo (ofc if izzysoft accepts this). So there will be automatic updates for both versions, don't worry. You can also use your own build too.

[klaviartur]

Okay, done.

Fixing the Bug did also auto-implement the feature ;)

What is not done, is

• Mark related settings with a symbol (E)

as I don't understand, what I have to do.

Also I don't know how to push this new commit to existing PR.
Please help!

Edit: Seems, I have done it somehow.

PS: The Speed still resets to 2x (if it was higher) on menu open when unhook is unchecked and pitch control mode is semitones, but this is true for setting speed via menu instead of gesture, too. And 0.5x respectively.
Even in upstream, so it is a feature, not a bug, apparently ;)

I think, speed should be confined to 0.5x-2.0x when unhook is unchecked and pitch control mode is semitones, but as already stated, all this hooking nonsense belongs in the trash anyway ;)

[klaviartur]

Side quest:
Now that this is done properly (hopefully at least), how about me opening identical PR in tubular directly?

Theoretically this would even work in NewPipe directly, but they don't take feature request PRs currently.

This way this feature could end up in your main after all, couldn't it? If @polymorphicshade accepts it, of course.

[asandikci]

Fixing the Bug did also auto-implement the feature ;)

🥳

What is not done, is

• Mark related settings with a symbol (E)

as I don't understand, what I have to do.

I want to mark features that only includes in extended branch with a symbol. So users can understand this is not a well-tested feature. Also if anyone ask for what is the difference with NewPipe or other fork, I wanna just say: "look for marked items in settings page". So one thing you should do is just mark new items. I mean just add (E) suffix to translations in settings page to item title or selections.
Examples:

• Middle gesture action -> Middle gesture action (E)
• Tempo -> Tempo (E) nvm it

PS: The Speed still resets to 2x (if it was higher) on menu open when unhook is unchecked and pitch control mode is semitones, but this is true for setting speed via menu instead of gesture, too. And 0.5x respectively.
Even in upstream, so it is a feature, not a bug, apparently ;)

I agree, this is not a bug, just an undocumented feature 😉

[asandikci]

Side quest: Now that this is done properly (hopefully at least), how about me opening identical PR in tubular directly?

Theoretically this would even work in NewPipe directly, but they don't take feature request PRs currently.

This way this feature could end up in your main after all, couldn't it? If @polymorphicshade accepts it, of course.

Yea, you can open a PR in Tubular too. Unfortunately polymorphicshade is not very active recently but it could be merged if he/she came back.

If he/she merge I'll merge it main too. Even if no, after we ensure there isn't any bug (and don't breaks any long-term compatibility) we can merge directly to main

[asandikci]

Everything looks good for now. I'll merge after everything made in #24 (comment)

update: I reviewed the code. It was written clearly, thank you!

[asandikci]

add (E) suffixes please. Ready to merge
update: added myself

[asandikci]

Merged, thanks!

[klaviartur]

Oh, okay. You were too fast for me. :)
Bummer, I was hoping to be able to include some minor improvements based on experiences from active usage of this feature with this PR.
Will see, maybe new PR

[asandikci]

you can always open a new pr 👍🏻

[asandikci]

I wanted to merge quickly because upstream newpipe has been released a new version. I wanted to include this PR too and releasing new versions now

[funsafe-ptr]

@klaviartur Thanks for this 🎉 I change tempo often, so this is really nice. 😊 Also, maybe always use the lowest step for gestures, regardless of the setting. ✨

[klaviartur]

@funsafe-ptr

I change tempo often

Unsurprisingly, so do I 🙃
I'm really glad that this means something to someone other than myself. So thanks for the feedback.

As for the step, that was a deliberate decision.
I know, there are a lot of Monk-like people like me out there, who could not stand to watch on 2.01x and would be forced to carefully and painstakingly fine tune to the nearest acceptable step all the time.

Also, what is the downside of respecting the setting?

[funsafe-ptr]

Well, the other gestures (brightness and volume) are smooth. It's a bit surprising at first when the step is 25% and above. 🤔 However, after using it many times, this isn't really a problem, because I can set the step to 1%. 👍 10% is okay as well. 🌟

[asandikci]

Actually I started to using this feature very frequently. Thanks again @klaviartur !