-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[video_player]fix video play() not update VideoPlayerValue when isInitialized is false #4295
[video_player]fix video play() not update VideoPlayerValue when isInitialized is false #4295
Conversation
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.
Thanks for the submission! I've left an initial review comment, but this will also need the missing items in the checklist addressed before moving forward.
|
||
try { | ||
await _videoPlayerPlatform.seekTo(_textureId, position); | ||
} catch (e) { |
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.
Silently discarding all exceptions is not a fix, it's hiding the problem—and a variety of other possible problems as well, which is not okay.
seekTo
isn't doing the same up-front isInitialized
check that other methods are doing, which seems like the actual bug here. It looks like this flow throwing is a regression from #3727 (@cc @KyleFin)
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.
Thanks @stuartmorgan! Yes, this issue was mentioned in #3727 (comment). @jerryzhoujw let me know if there's anything I can do to help. Thanks!
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.
Instead of this change I believe you could just change line 397 to be
if (value.position == value.duration && value.isInitialized) {
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.
And unit tests could be similar to those added in https://github.com/flutter/plugins/pull/3727/files#diff-b67d4b0ee52dfea4c9c71082e12eca5459a174693c8d40577b0191d8ee498e34
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.
Thanks
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.
close this since #4300 much better.
close this since #4300 much better. |
Before
VideoPlayerController.initialize()
finished.Client maybe call
VideoPlayerController.play()
.It's better to notify the
VideoPlayerValue
to client.To make this work:
The exception on my android devices is:
List which issues are fixed by this PR. You must list at least one issue.
flutter/flutter#89259
Pre-launch Checklist
dart format
.)[shared_preferences]
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.