[video_player] Miscellaneous fixes to improve video player in web. #2819
Conversation
packages/video_player/video_player_web/lib/video_player_web.dart
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| void setVolume(double value) { | ||
| if (value > 0.0) { |
There was a problem hiding this comment.
might need to introduce a small epsilon when comparing float values
There was a problem hiding this comment.
Or we make the muted property public so user can directly set mute instead of setting a 0.0 value.
There was a problem hiding this comment.
This "hatch" is only there so web users can do: setVolume(0.0) and get a "muted" player.
About making the muted property public: I agree, but I am not sure what the implications are for Android and iOS of that.
There's a huge API surface for the video element that is being hidden by the current platform API, and I'm not sure if it's because we want a very simple API, or because the API can't be extended for other platforms :/
For this PR I was only trying to make the selected video auto-play in the image_picker web example, not do any significant changes to the API of the video_player widget.
There was a problem hiding this comment.
I've created this issue so we can discuss this API change:
If we decide to add a "muted" attribute to the public API, this hack can be removed!
packages/video_player/video_player_web/lib/video_player_web.dart
Outdated
Show resolved
Hide resolved
@yjbanov I'll try what happens with malformed URLs, since this may be user-generated content. |
|
Tested malformed URLs, and the plugin seems to be able to spit out an error that is understandable: |
…me to 0. This enables autoplay without user interaction in most browsers.
ditman
force-pushed
the
image-picker-example-video-fixes
branch
from
e17688a to
74de285
Compare
This change adds small bugfixes to the following packages: * video_player_web: * Prevent parsing Blob URLs so Safari can play from a PickedFile. * Allow users to 'mute' videos by setting their volume to 0.0 (this enables auto-play in most modern browsers) * video_player: * Fix an issue where aspect ratio calculations failed when some of the video sizes were zero (happened in web for incorrect videos) * image_picker (example app): * Start videos in web muted so they can auto-play * Dispose video controllers when they're really not needed. This fixes a fatal crash in Safari (not so fatal in other browsers).
This change adds small bugfixes to the following packages: * video_player_web: * Prevent parsing Blob URLs so Safari can play from a PickedFile. * Allow users to 'mute' videos by setting their volume to 0.0 (this enables auto-play in most modern browsers) * video_player: * Fix an issue where aspect ratio calculations failed when some of the video sizes were zero (happened in web for incorrect videos) * image_picker (example app): * Start videos in web muted so they can auto-play * Dispose video controllers when they're really not needed. This fixes a fatal crash in Safari (not so fatal in other browsers).
This change adds small bugfixes to the following packages: * video_player_web: * Prevent parsing Blob URLs so Safari can play from a PickedFile. * Allow users to 'mute' videos by setting their volume to 0.0 (this enables auto-play in most modern browsers) * video_player: * Fix an issue where aspect ratio calculations failed when some of the video sizes were zero (happened in web for incorrect videos) * image_picker (example app): * Start videos in web muted so they can auto-play * Dispose video controllers when they're really not needed. This fixes a fatal crash in Safari (not so fatal in other browsers).
4 participants
Description
While testing the image_picker_web plugin, I noticed that video playback was a little bit flaky. This PR addresses some of the subtle bugs in the image_picker example app, video_player plugin and video_player_web implementation.
Related Issues
n/a
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?
All changes are minor and independent, so no need to split this PR in several different ones. This all should be mergeable and publishable in parallel.