-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[video_player] Fix pause infinite loop on completed #5920
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
Changes from 5 commits
990decf
cccf9d3
546e37f
05d8479
3d71c14
5842263
d02d031
7de2626
1abfd73
b65c9dc
47de4f9
5b54f4b
0da37df
5ea0689
73077fd
b2b9748
5a82692
c4a5c9d
e9d6fbe
4f7466e
30162ec
af527eb
50ee253
dda2702
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,13 @@ | ||
| ## NEXT | ||
|
|
||
| * Description of the change. | ||
|
|
||
| ## 2.8.3 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The changes to this package should be reverted; we don't artificially constrain allowed versions in the app-facing packages.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has not been addressed. |
||
|
|
||
| * Updates support matrix in README to indicate that iOS 11 is no longer supported. | ||
| * Clients on versions of Flutter that still support iOS 11 can continue to use this | ||
| package with iOS 11, but will not receive any further updates to the iOS implementation. | ||
| * Fix infinite pause loop caused by on completed to call pause even when video is pause. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixes infinite pause loop caused by call to pause even when video is already paused. |
||
|
|
||
| ## 2.8.2 | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -460,7 +460,9 @@ class VideoPlayerController extends ValueNotifier<VideoPlayerValue> { | |
| // position=value.duration. Instead of setting the values directly, | ||
| // we use pause() and seekTo() to ensure the platform stops playing | ||
| // and seeks to the last frame of the video. | ||
| pause().then((void pauseResult) => seekTo(value.duration)); | ||
| if (value.isPlaying) { | ||
| pause().then((void pauseResult) => seekTo(value.duration)); | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This changed logic would definitely not be guaranteed to actually do what this code is supposed to be doing; if the native side reported reaching the end as:
then the seek code would never run.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stuartmorgan have updated the logic to account for this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, I believe this infinite loop issue is only seen on web for some reason |
||
| value = value.copyWith(isCompleted: true); | ||
| case VideoEventType.bufferingUpdate: | ||
| value = value.copyWith(buffered: event.buffered); | ||
|
|
||
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.
This should be removed