Playback speed controls

[ghost]

Controls for both Android & iOS.
Introduces: allowPlaybackSpeedChanging and playbackSpeeds to ChewieController

[nstrelow]

Could you add a gif or screenshot on how it works?

Also duplicate of #367

[Ahmadre]

Controls for both Android & iOS.
Introduces: allowPlaybackSpeedChanging and playbackSpeeds to ChewieController

Thank you so much @kono0514 for your work 👍🏼. This PR is much more fast forward and I think we can work with it :). I've checked our your branch and tested everything here:

I encountered following points:

1. On android the 1.0 speed is called Normal and that's perfect 👍🏼, but on iOS it's called 1.0. Could you do that consistent and name both Normal for 1.0 speed?

2. The bottom bar gets full of controls and I think that could lead to worse User-Experience for especially dragging the seek-slider on smaller devices. But I already found a clean solution for that: Do it like Youtube and create an Overlay or AnimatedOpacity Container which pops up on Web/Desktop and for mobile the same bottom sheet appears like the Youtube-Mobile App and shows the available settings for the video.

Youtube Web

Youtube Mobile

As you can see on mobile devices the vertical_more IconButton is used for showing the video settings on the top right corner. Regarding the checks wether we're on mobile or not, I would rather stick to screen size then to TargetPlatform or Platform.isAndroid. You can try LayoutBuilder or other ways to differentiate :).

If we can have this is chewie this would be awesome 👏

[Ahmadre]

Ah yes and I am missing the "Cancel" Button on the bottom for the MaterialBottomSheet (on Cupertino it exists already):

And we should also be able to localize such labels like "Normal" and "Cancel". The best thing would be maybe to just give a parameter like for playback's "Normal" and "Cancel" buttons. But maybe you guys got a better idea here (@nstrelow)

[nstrelow]

https://material.io/components/sheets-bottom

A sheet should be dismissable by (I think) swiping down or clicking above the sheet>>

https://developer.apple.com/design/human-interface-guidelines/ios/views/action-sheets/
In iOS:

Provide a Cancel button if it adds clarity. A Cancel button instills confidence when the user is abandoning a task. Cancel buttons should always be included in action sheets at the bottom of the screen.

I do not think we need an explicit Cancel button. A confused user would probably just press normal or try to dismiss it.

Example for needing a cancel button: User presses delete by mistake -> cancel gives certainty nothing was deleted.

About translation
We currently have the blessing of having no strings at all in chewie.
I would love to keep it that way for now.
So no cancel and replace normal by 1.0 I would say.

[Ahmadre]

https://material.io/components/sheets-bottom

A sheet should be dismissable by (I think) swiping down or clicking above the sheet>>

https://developer.apple.com/design/human-interface-guidelines/ios/views/action-sheets/
In iOS:

Provide a Cancel button if it adds clarity. A Cancel button instills confidence when the user is abandoning a task. Cancel buttons should always be included in action sheets at the bottom of the screen.

I do not think we need an explicit Cancel button. A confused user would probably just press normal or try to dismiss it.

Example for needing a cancel button: User presses delete by mistake -> cancel gives certainty nothing was deleted.

About translation
We currently have the blessing of having no strings at all in chewie.
I would love to keep it that way for now.
So no cancel and replace normal by 1.0 I would say.

Brilliant! You solved all my headache @nstrelow 😂

Ok so again my updated thoughts:

1. Consistent 1.0's for both Material and Cupertino Controls
2. We definetly need a settings icon which should have an onTap handler.
   2.1 If we don't want to handle localization then we should have a new parameter onSettingsTap, so the user can decide what he wants to show (Overlay or BottomSheet is not needed here anymore) after the settings Icon is tapped.
3. We need the tap gesture handler which @kono0514 implemeted for the speed to trigger.
4. We should then have a builder method like "playbackSpeedBuilder" so that the user decides how he want to render the speed selection (on default the actual implementation is appearing).

🧐 🤔 These are my thoughts here to stay as cross-platform as we can. Thanks also for all of your inputs!

[ghost]

1. I agree. Gonna fix the inconsistency on speed labels
2. I don't think we need to expose onSettingsTap and playbackSpeedBuilder etc... If a user wants to change how the controls look and function, we do provide customControls for that.

What we should do is provide a sane default player controls with great functionality and UI out of the box. Then leave the rest for the user to modify using customControls.
And I also think the current "MaterialControls" doesn't look and feel "Material". It could be improved alongside with the Youtube-like settings icon on top right like @Ahmadre mentioned.

That's my two cents. What do you guys think?

[Ahmadre]

1. I agree. Gonna fix the inconsistency on speed labels

2. I don't think we need to expose onSettingsTap and playbackSpeedBuilder etc... If a user wants to change how the controls look and function, we do provide customControls for that.

What we should do is provide a sane default player controls with great functionality and UI out of the box. Then leave the rest for the user to modify using customControls.

And I also think the current "MaterialControls" doesn't look and feel "Material". It could be improved alongside with the Youtube-like settings icon on top right like @Ahmadre mentioned.

That's my two cents. What do you guys think?

Exactly! That was what I was searching for chewie: add this speed functionality now and provide in future a customControls parameter 👏🏻.

I already implemented in the past YouTube-like controls, I'll create a PR for that, so it comes out of the box.

Ok, but for now for this PR: LGTM, if you just improve the section which I've commented in your code 👍🏼 great job @kono0514 .

@nstrelow do you agree on the plan?

[nstrelow]

Yup, agree.

[ghost]

There is also one small bug in Material.
When the playbackSpeeds size is big and the sheet dialog occupies the whole screen. The dialog goes behind the status bar.

Workaround for that bug involves passing the main context to _PlaybackSpeedDialog and adding margin using a Container.
But there's an official fix coming flutter/flutter#66257

Until the official fix lands, should I implement the workaround or just wait?

[Ahmadre]

@kono0514 It's better to wait for the official fix, but it seems that the fix already landed and merged :) so all Master-Channels got it.

Regarding the change requests: it's fine! Then the concrete implementation stays and I saw you already changed the SingleChildScrollView and Wraps 👍🏼 good job builder methods are always faster 👍🏼 LGTM now

[Ahmadre]

@kono0514 I encountered that you've forked and got a separate implementation for video_player...that's not good:

Because we actually can't use your implementation here...

It would be good to have such speed functionality in video_player, but until then we can't use this in chewie @nstrelow

[ghost]

I think you're on an old version of video_player. setPlaybackSpeed was added in 0.11.0
Check your pubspec.lock

[Ahmadre]

I think you're on an old version of video_player. setPlaybackSpeed was added in 0.11.0

Check your pubspec.lock

Maybe you're right! I'll check that!

[creativecreatorormaybenot]

Just wanted to mention this: thank you so much for using the setPlaybackSpeed feature ✨ and integrating it into chewie.

(context)

I created the PR on video_player because we needed it internally and we also used "Normal" for the 1.0 speed :)
(sorry that we are not using chewie internally 😄)

[nstrelow]

@Ahmadre We probably need to set the minimum supported version of video_player to 0.11

Nice that this landed 😁

[anchal-td]

I encountered following points:
On android the 1.0 speed is called Normal and that's perfect 👍🏼, but on iOS it's called 1.0. Could you do that consistent and name both Normal for 1.0 speed?

Hi @Ahmadre, It will be very helpful and will look more consistent if you do 1.0 as Normal in android & 1.0 is iOS. Let us know if this is supported in any coming version. Thanks in advance for this awesome plugin.