-
Notifications
You must be signed in to change notification settings - Fork 3k
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
base: main
Are you sure you want to change the base?
Conversation
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. |
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. |
...undation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayer.m
Show resolved
Hide resolved
There was a problem hiding this 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?
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:
and btw, this happens on all video players based on your official |
#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. |
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. |
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? |
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