[multiple] Improve video playback in image_picker example (#2819)

ditman · web-flow · commit d3ccc34855b7 · 2020-07-01T15:53:53.000-07:00
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).
diff --git a/packages/image_picker/image_picker/CHANGELOG.md b/packages/image_picker/image_picker/CHANGELOG.md
@@ -1,3 +1,9 @@
+## 0.6.7+3
+
+* Fixes to the example app:
+  * Make videos in web start muted. This allows auto-play across browsers.
+  * Prevent the app from disposing of video controllers too early.
+
 ## 0.6.7+2
 
 * iOS: Fixes unpresentable album/image picker if window's root view controller is already presenting other view controller.
diff --git a/packages/image_picker/image_picker/example/lib/main.dart b/packages/image_picker/image_picker/example/lib/main.dart
@@ -42,6 +42,7 @@ class _MyHomePageState extends State<MyHomePage> {
   dynamic _pickImageError;
   bool isVideo = false;
   VideoPlayerController _controller;
+  VideoPlayerController _toBeDisposed;
   String _retrieveDataError;
 
   final ImagePicker _picker = ImagePicker();
@@ -54,10 +55,16 @@ class _MyHomePageState extends State<MyHomePage> {
       await _disposeVideoController();
       if (kIsWeb) {
         _controller = VideoPlayerController.network(file.path);
+        // In web, most browsers won't honor a programmatic call to .play
+        // if the video has a sound track (and is not muted).
+        // Mute the video so it auto-plays in web!
+        // This is not needed if the call to .play is the result of user
+        // interaction (clicking on a "play" button, for example).
+        await _controller.setVolume(0.0);
       } else {
         _controller = VideoPlayerController.file(File(file.path));
+        await _controller.setVolume(1.0);
       }
-      await _controller.setVolume(1.0);
       await _controller.initialize();
       await _controller.setLooping(true);
       await _controller.play();
@@ -114,10 +121,11 @@ class _MyHomePageState extends State<MyHomePage> {
   }
 
   Future<void> _disposeVideoController() async {
-    if (_controller != null) {
-      await _controller.dispose();
-      _controller = null;
+    if (_toBeDisposed != null) {
+      await _toBeDisposed.dispose();
     }
+    _toBeDisposed = _controller;
+    _controller = null;
   }
 
   Widget _previewVideo() {
diff --git a/packages/image_picker/image_picker/pubspec.yaml b/packages/image_picker/image_picker/pubspec.yaml
@@ -2,7 +2,7 @@ name: image_picker
 description: Flutter plugin for selecting images from the Android and iOS image
   library, and taking new pictures with the camera.
 homepage: https://github.com/flutter/plugins/tree/master/packages/image_picker/image_picker
-version: 0.6.7+2
+version: 0.6.7+3
 
 flutter:
   plugin:
diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.10.11+2
+
+* Fix aspectRatio calculation when size.width or size.height are zero.
+
 ## 0.10.11+1
 
 * Post-v2 Android embedding cleanups.
diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart
@@ -97,7 +97,7 @@ class VideoPlayerValue {
   /// Returns [size.width] / [size.height] when size is non-null, or `1.0.` when
   /// size is null or the aspect ratio would be less than or equal to 0.0.
   double get aspectRatio {
-    if (size == null) {
+    if (size == null || size.width == 0 || size.height == 0) {
       return 1.0;
     }
     final double aspectRatio = size.width / size.height;
diff --git a/packages/video_player/video_player/pubspec.yaml b/packages/video_player/video_player/pubspec.yaml
@@ -4,7 +4,7 @@ description: Flutter plugin for displaying inline video with other Flutter
 # 0.10.y+z is compatible with 1.0.0, if you land a breaking change bump
 # the version to 2.0.0.
 # See more details: https://github.com/flutter/flutter/wiki/Package-migration-to-1.0.0
-version: 0.10.11+1
+version: 0.10.11+2
 homepage: https://github.com/flutter/plugins/tree/master/packages/video_player/video_player
 
 flutter:
diff --git a/packages/video_player/video_player/test/video_player_test.dart b/packages/video_player/video_player/test/video_player_test.dart
@@ -517,6 +517,48 @@ void main() {
 
       expect(exactCopy.toString(), original.toString());
     });
+
+    group('aspectRatio', () {
+      test('640x480 -> 4:3', () {
+        final value = VideoPlayerValue(
+          size: Size(640, 480),
+          duration: Duration(seconds: 1),
+        );
+        expect(value.aspectRatio, 4 / 3);
+      });
+
+      test('null size -> 1.0', () {
+        final value = VideoPlayerValue(
+          size: null,
+          duration: Duration(seconds: 1),
+        );
+        expect(value.aspectRatio, 1.0);
+      });
+
+      test('height = 0 -> 1.0', () {
+        final value = VideoPlayerValue(
+          size: Size(640, 0),
+          duration: Duration(seconds: 1),
+        );
+        expect(value.aspectRatio, 1.0);
+      });
+
+      test('width = 0 -> 1.0', () {
+        final value = VideoPlayerValue(
+          size: Size(0, 480),
+          duration: Duration(seconds: 1),
+        );
+        expect(value.aspectRatio, 1.0);
+      });
+
+      test('negative aspect ratio -> 1.0', () {
+        final value = VideoPlayerValue(
+          size: Size(640, -480),
+          duration: Duration(seconds: 1),
+        );
+        expect(value.aspectRatio, 1.0);
+      });
+    });
   });
 
   test('VideoProgressColors', () {
diff --git a/packages/video_player/video_player_web/CHANGELOG.md b/packages/video_player/video_player_web/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 0.1.3+2
+
+* Allow users to set the 'muted' attribute on video elements by setting their volume to 0.
+* Do not parse URIs on 'network' videos to not break blobs (Safari).
+
 ## 0.1.3+1
 
 * Remove Android folder from `video_player_web`.
diff --git a/packages/video_player/video_player_web/lib/video_player_web.dart b/packages/video_player/video_player_web/lib/video_player_web.dart
@@ -66,10 +66,12 @@ class VideoPlayerPlugin extends VideoPlayerPlatform {
     final int textureId = _textureCounter;
     _textureCounter++;
 
-    Uri uri;
+    String uri;
     switch (dataSource.sourceType) {
       case DataSourceType.network:
-        uri = Uri.parse(dataSource.uri);
+        // Do NOT modify the incoming uri, it can be a Blob, and Safari doesn't
+        // like blobs that have changed.
+        uri = dataSource.uri;
         break;
       case DataSourceType.asset:
         String assetUrl = dataSource.asset;
@@ -79,7 +81,7 @@ class VideoPlayerPlugin extends VideoPlayerPlatform {
         // 'webOnlyAssetManager' is only in the web version of dart:ui
         // ignore: undefined_prefixed_name
         assetUrl = ui.webOnlyAssetManager.getAssetUrl(assetUrl);
-        uri = Uri.parse(assetUrl);
+        uri = assetUrl;
         break;
       case DataSourceType.file:
         return Future.error(UnimplementedError(
@@ -145,18 +147,21 @@ class _VideoPlayer {
   final StreamController<VideoEvent> eventController =
       StreamController<VideoEvent>();
 
-  final Uri uri;
+  final String uri;
   final int textureId;
   VideoElement videoElement;
   bool isInitialized = false;
 
   void initialize() {
     videoElement = VideoElement()
-      ..src = uri.toString()
+      ..src = uri
       ..autoplay = false
       ..controls = false
       ..style.border = 'none';
 
+    // Allows Safari iOS to play the video inline
+    videoElement.setAttribute('playsinline', 'true');
+
     // TODO(hterkelsen): Use initialization parameters once they are available
     // ignore: undefined_prefixed_name
     ui.platformViewRegistry.registerViewFactory(
@@ -218,6 +223,12 @@ class _VideoPlayer {
   }
 
   void setVolume(double value) {
+    // TODO: Do we need to expose a "muted" API? https://github.com/flutter/flutter/issues/60721
+    if (value > 0.0) {
+      videoElement.muted = false;
+    } else {
+      videoElement.muted = true;
+    }
     videoElement.volume = value;
   }
 
diff --git a/packages/video_player/video_player_web/pubspec.yaml b/packages/video_player/video_player_web/pubspec.yaml
@@ -4,7 +4,7 @@ homepage: https://github.com/flutter/plugins/tree/master/packages/video_player/v
 # 0.1.y+z is compatible with 1.0.0, if you land a breaking change bump
 # the version to 2.0.0.
 # See more details: https://github.com/flutter/flutter/wiki/Package-migration-to-1.0.0
-version: 0.1.3+1
+version: 0.1.3+2
 
 flutter:
   plugin:

Original file line number	Diff line number	Diff line change
`@@ -97,7 +97,7 @@ class VideoPlayerValue {`
`97`	`97`	/// Returns [size.width] / [size.height] when size is non-null, or `1.0.` when
`98`	`98`	`/// size is null or the aspect ratio would be less than or equal to 0.0.`
`99`	`99`	`double get aspectRatio {`
`100`		`- if (size == null) {`
	`100`	`+ if (size == null \|\| size.width == 0 \|\| size.height == 0) {`
`101`	`101`	`return 1.0;`
`102`	`102`	`}`
`103`	`103`	`final double aspectRatio = size.width / size.height;`