Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[video_player_avfoundation] fix video playback speed resetting on iOS 16+ #8274

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

sha3rawi33
Copy link

This PR fixes the unwanted behavior of video playback speed resetting on iOS16+.

Current unwanted behavior: Video playback speed resets to 1.0 ignoring any speed the user has already set. Like when you play a video, and set the speed to 2.0x, when you seek the video playback speed will change to 1.0x again.

PR's new behavior: the playback speed will remain set when seeking forward/backward/drag to position. Like when you set the playback speed to 2.0x(or any other speed) and then seek forward/backward or drag to any video position; the playback speed will keep playing on 2.0x and will not reset to 1.0x

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

Copy link

google-cla bot commented Dec 11, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Contributor

@hellohuanlin hellohuanlin left a comment

Choose a reason for hiding this comment

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

can you add some tests?

@sha3rawi33
Copy link
Author

can you add some tests?

I added tests but they will pass even on the old versions with this unwanted behavior too.

As when you seek and playback speed resets, it only resets on the native player, but for the flutter side's video player controller; the playback speed property is still 2.0x(or any speed you set before seeking).

here is an example scenario:

  • video starts playing -> you set playback speed to 2.0x -> you seek (anywhere) -> video playback speed resets to 1.0x but the player still shows you are on 2.0x

and btw, this happens on all video players based on your official video_player that uses video_player_avfoundation on ios

@stuartmorgan
Copy link
Contributor

#7657 was intended to address the same issue, and was essentially ready to land, so I would suggest holding off on working on this for a few days since as long as we get a new PR for that branch the issue should be addressed.

@sha3rawi33
Copy link
Author

#7657 was intended to address the same issue, and was essentially ready to land, so I would suggest holding off on working on this for a few days since as long as we get a new PR for that branch the issue should be addressed.

I was facing this issue on my projects depending on this package, and I tried to depend on the package from @misos1 branch but found the issue still unresolved. that's why I decided to open a PR.

@sha3rawi33
Copy link
Author

I added tests for both the dart and native sides. Tests pass on my branch and fail on the repo's; ensuring defaultRate was not being set on the previous versions which caused this issue to happen.

@misos1
Copy link
Contributor

misos1 commented Dec 12, 2024

I tested this with #7657 on ios 17.5 and I cannot reproduce. Can you provide some more details? Are you sure you tested with change from that PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants