pocket-casts-android
pocket-casts-android copied to clipboard
Increase limit on playback speed to 5.0
Description
This PR increases the maximum playback speed from 3.0 to 5.0. The current maximum speed (3.0) can still be perceived as rather slow for podcasts with very slow speakers. Thus, this PR increases the upper bound from 3.0 to 5.0.
Testing Instructions
- Play a podcast
- Increase the playback speed to values higher than 3.0 (the previous maximum) and lower than 5.0 (the new maximum).
Screenshot
Checklist
- [x] If this is a user-facing change, I have added an entry in CHANGELOG.md
- [x] Ensure the linter passes (
./gradlew spotlessApply
to automatically apply formatting/linting) - [x] I have considered whether it makes sense to add tests for my changes
- [x] All strings that need to be localized are in
modules/services/localization/src/main/res/values/strings.xml
- [x] Any jetpack compose components I added or changed are covered by compose previews
I have tested any UI changes...
- [ ] with different themes
- [ ] with a landscape orientation
- [ ] with the device set to have a large display and font size
- [ ] for accessibility with TalkBack
This is unlikely to be merged as it leads to too much tapping, can you do something about it , I am also working on a redesigned playback FX player to justify a speed increase to 5.0, if you can implement that then i would say we can go to 5x 😉
https://github.com/Automattic/pocket-casts-android/pull/1150
🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏🙏
@MiSikora What are your thoughts on this?
If you're asking about my opinion I feel that it is too much but I don't have any problems with having it in the app. I'll ask internally if it is something that we want to support and get back to you.
However, this code on its own isn't enough. It needs changes that will support speed toggling from notification center and Tasker support for these settings.
@zimmerrol We can add it. However, as I mentioned, you need to add support for notifications center and Tasker plugin.
Notifications are handled here
https://github.com/Automattic/pocket-casts-android/blob/2ea07252cb1b4afc99df966159b87023a0d77664/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaSessionManager.kt#L435-L462
https://github.com/Automattic/pocket-casts-android/blob/2ea07252cb1b4afc99df966159b87023a0d77664/modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/MediaSessionManager.kt#L738-L750
And Tasker is handled here
https://github.com/Automattic/pocket-casts-android/blob/2ea07252cb1b4afc99df966159b87023a0d77664/modules/features/taskerplugin/src/main/java/au/com/shiftyjelly/pocketcasts/taskerplugin/controlplayback/config/ViewModelConfigControlPlayback.kt#L46-L48
Great, thanks for looking into this @MiSikora. I'll implement the necessary changes!
@mebarbosa I noticed that you recently (#1862) updated all playback speed resources to ensure they all have the same font family; since we now need resources for the added speed values, I was wondering whether you could tell me how you created these images (based on what template) so that I can create the missing ones.
Great, thanks for looking into this @MiSikora. I'll implement the necessary changes!
@mebarbosa I noticed that you recently (#1862) updated all playback speed resources to ensure they all have the same font family; since we now need resources for the added speed values, I was wondering whether you could tell me how you created these images (based on what template) so that I can create the missing ones.
@zimmerrol thanks for the ping and working on this! ❤️ I think our design @david-gonzalez-a8c could help us with this. David generated all images for me
@mebarbosa see my comment above, David actually gave me some feedback from a design I did a few months ago 😅 😉
@david-gonzalez-a8c Did you have a chance for looking at this PR or can you please send me the templates you created/used? So that we can finalize this PR?
Hey there, @zimmerrol
Let me know how these look and if you need a different file structure or naming:
Thank you!
@david-gonzalez-a8c I'm still a tad unsure going up to 5x speed would be good without a redesign on safety grounds & tapping +- is very laborious beyond even 3x speed 😅 just a reminder of our chat from last year https://github.com/Automattic/pocket-casts-android/pull/1150
@MiSikora I implemented your suggestions above and think this should complete the PR. Is there anything else that needs to be done before this can get merged?
Here is an example of how the new speeds are correctly displayed in the notifications:
These tests are failing
[2024-06-07T08:43:11Z] au.com.shiftyjelly.pocketcasts.utils.featureflag.DoubleTest > roundedSpeed respects upper boundary FAILED
[2024-06-07T08:43:11Z] junit.framework.AssertionFailedError at DoubleTest.kt:23
[2024-06-07T08:43:11Z]
[2024-06-07T08:43:11Z] au.com.shiftyjelly.pocketcasts.utils.featureflag.DoubleTest > roundedSpeed adjusts above upper boundary FAILED
[2024-06-07T08:43:11Z] junit.framework.AssertionFailedError at DoubleTest.kt:38