From e374869161d2d30cac07b2017d2eabd405f073d7 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 30 Sep 2019 08:06:14 -0700 Subject: [PATCH 01/28] video player crash fix --- packages/video_player/example/lib/main.dart | 434 ++---------------- .../ios/Classes/VideoPlayerPlugin.m | 8 +- 2 files changed, 47 insertions(+), 395 deletions(-) diff --git a/packages/video_player/example/lib/main.dart b/packages/video_player/example/lib/main.dart index dea1086593df..bd704f23e2ed 100644 --- a/packages/video_player/example/lib/main.dart +++ b/packages/video_player/example/lib/main.dart @@ -1,432 +1,78 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -/// An example of using the plugin, controlling lifecycle and playback of the -/// video. - -import 'package:flutter/cupertino.dart'; import 'package:flutter/material.dart'; import 'package:video_player/video_player.dart'; -/// Controls play and pause of [controller]. -/// -/// Toggles play/pause on tap (accompanied by a fading status icon). -/// -/// Plays (looping) on initialization, and mutes on deactivation. -class VideoPlayPause extends StatefulWidget { - VideoPlayPause(this.controller); - - final VideoPlayerController controller; - - @override - State createState() { - return _VideoPlayPauseState(); - } -} - -class _VideoPlayPauseState extends State { - _VideoPlayPauseState() { - listener = () { - setState(() {}); - }; - } - - FadeAnimation imageFadeAnim = - FadeAnimation(child: const Icon(Icons.play_arrow, size: 100.0)); - VoidCallback listener; - - VideoPlayerController get controller => widget.controller; - - @override - void initState() { - super.initState(); - controller.addListener(listener); - controller.setVolume(1.0); - controller.play(); - } - - @override - void deactivate() { - controller.setVolume(0.0); - controller.removeListener(listener); - super.deactivate(); - } - +class MainPage extends StatelessWidget { @override Widget build(BuildContext context) { - final List children = [ - GestureDetector( - child: VideoPlayer(controller), - onTap: () { - if (!controller.value.initialized) { - return; - } - if (controller.value.isPlaying) { - imageFadeAnim = - FadeAnimation(child: const Icon(Icons.pause, size: 100.0)); - controller.pause(); - } else { - imageFadeAnim = - FadeAnimation(child: const Icon(Icons.play_arrow, size: 100.0)); - controller.play(); - } + return Scaffold( + body: Center( + child: FlatButton( + child: const Text('push'), + onPressed: () { + Navigator.push( + context, + MaterialPageRoute( + builder: (BuildContext context) => VideoCrashPlayerPage()), + ); }, ), - Align( - alignment: Alignment.bottomCenter, - child: VideoProgressIndicator( - controller, - allowScrubbing: true, - ), - ), - Center(child: imageFadeAnim), - Center( - child: controller.value.isBuffering - ? const CircularProgressIndicator() - : null), - ]; - - return Stack( - fit: StackFit.passthrough, - children: children, - ); + )); } } -class FadeAnimation extends StatefulWidget { - FadeAnimation( - {this.child, this.duration = const Duration(milliseconds: 500)}); - - final Widget child; - final Duration duration; - +class VideoCrashPlayerPage extends StatefulWidget { @override - _FadeAnimationState createState() => _FadeAnimationState(); + _VideoCrashPlayerPageState createState() => _VideoCrashPlayerPageState(); } -class _FadeAnimationState extends State - with SingleTickerProviderStateMixin { - AnimationController animationController; +class _VideoCrashPlayerPageState extends State { + VideoPlayerController _videoPlayerController; @override void initState() { super.initState(); - animationController = - AnimationController(duration: widget.duration, vsync: this); - animationController.addListener(() { - if (mounted) { - setState(() {}); - } - }); - animationController.forward(from: 0.0); - } - - @override - void deactivate() { - animationController.stop(); - super.deactivate(); - } - - @override - void didUpdateWidget(FadeAnimation oldWidget) { - super.didUpdateWidget(oldWidget); - if (oldWidget.child != widget.child) { - animationController.forward(from: 0.0); - } - } - - @override - void dispose() { - animationController.dispose(); - super.dispose(); - } - - @override - Widget build(BuildContext context) { - return animationController.isAnimating - ? Opacity( - opacity: 1.0 - animationController.value, - child: widget.child, - ) - : Container(); - } -} - -typedef Widget VideoWidgetBuilder( - BuildContext context, VideoPlayerController controller); - -abstract class PlayerLifeCycle extends StatefulWidget { - PlayerLifeCycle(this.dataSource, this.childBuilder); - - final VideoWidgetBuilder childBuilder; - final String dataSource; -} - -/// A widget connecting its life cycle to a [VideoPlayerController] using -/// a data source from the network. -class NetworkPlayerLifeCycle extends PlayerLifeCycle { - NetworkPlayerLifeCycle(String dataSource, VideoWidgetBuilder childBuilder) - : super(dataSource, childBuilder); - - @override - _NetworkPlayerLifeCycleState createState() => _NetworkPlayerLifeCycleState(); -} - -/// A widget connecting its life cycle to a [VideoPlayerController] using -/// an asset as data source -class AssetPlayerLifeCycle extends PlayerLifeCycle { - AssetPlayerLifeCycle(String dataSource, VideoWidgetBuilder childBuilder) - : super(dataSource, childBuilder); - @override - _AssetPlayerLifeCycleState createState() => _AssetPlayerLifeCycleState(); -} - -abstract class _PlayerLifeCycleState extends State { - VideoPlayerController controller; - - @override - - /// Subclasses should implement [createVideoPlayerController], which is used - /// by this method. - void initState() { - super.initState(); - controller = createVideoPlayerController(); - controller.addListener(() { - if (controller.value.hasError) { - print(controller.value.errorDescription); + _videoPlayerController = + VideoPlayerController.asset("assets/Butterfly-209.mp4"); + _videoPlayerController.addListener(() { + if (!_videoPlayerController.value.isPlaying) { + Navigator.pop(context); } }); - controller.initialize(); - controller.setLooping(true); - controller.play(); - } - - @override - void deactivate() { - super.deactivate(); + _videoPlayerController.play(); + _videoPlayerController.initialize(); } @override void dispose() { - controller.dispose(); - super.dispose(); - } - - @override - Widget build(BuildContext context) { - return widget.childBuilder(context, controller); - } - - VideoPlayerController createVideoPlayerController(); -} - -class _NetworkPlayerLifeCycleState extends _PlayerLifeCycleState { - @override - VideoPlayerController createVideoPlayerController() { - return VideoPlayerController.network(widget.dataSource); - } -} - -class _AssetPlayerLifeCycleState extends _PlayerLifeCycleState { - @override - VideoPlayerController createVideoPlayerController() { - return VideoPlayerController.asset(widget.dataSource); - } -} - -/// A filler card to show the video in a list of scrolling contents. -Widget buildCard(String title) { - return Card( - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - ListTile( - leading: const Icon(Icons.airline_seat_flat_angled), - title: Text(title), - ), - // TODO(jackson): Remove when deprecation is on stable branch - // ignore: deprecated_member_use - ButtonTheme.bar( - child: ButtonBar( - children: [ - FlatButton( - child: const Text('BUY TICKETS'), - onPressed: () { - /* ... */ - }, - ), - FlatButton( - child: const Text('SELL TICKETS'), - onPressed: () { - /* ... */ - }, - ), - ], - ), - ), - ], - ), - ); -} - -class VideoInListOfCards extends StatelessWidget { - VideoInListOfCards(this.controller); - - final VideoPlayerController controller; - - @override - Widget build(BuildContext context) { - return ListView( - children: [ - buildCard("Item a"), - buildCard("Item b"), - buildCard("Item c"), - buildCard("Item d"), - buildCard("Item e"), - buildCard("Item f"), - buildCard("Item g"), - Card( - child: Column(children: [ - Column( - children: [ - const ListTile( - leading: Icon(Icons.cake), - title: Text("Video video"), - ), - Stack( - alignment: FractionalOffset.bottomRight + - const FractionalOffset(-0.1, -0.1), - children: [ - AspectRatioVideo(controller), - Image.asset('assets/flutter-mark-square-64.png'), - ]), - ], - ), - ])), - buildCard("Item h"), - buildCard("Item i"), - buildCard("Item j"), - buildCard("Item k"), - buildCard("Item l"), - ], - ); - } -} - -class AspectRatioVideo extends StatefulWidget { - AspectRatioVideo(this.controller); + _videoPlayerController.dispose(); - final VideoPlayerController controller; - - @override - AspectRatioVideoState createState() => AspectRatioVideoState(); -} - -class AspectRatioVideoState extends State { - VideoPlayerController get controller => widget.controller; - bool initialized = false; - - VoidCallback listener; - - @override - void initState() { - super.initState(); - listener = () { - if (!mounted) { - return; - } - if (initialized != controller.value.initialized) { - initialized = controller.value.initialized; - setState(() {}); - } - }; - controller.addListener(listener); + super.dispose(); } @override Widget build(BuildContext context) { - if (initialized) { - return Center( - child: AspectRatio( - aspectRatio: controller.value.aspectRatio, - child: VideoPlayPause(controller), - ), - ); - } else { - return Container(); - } + return Material( + elevation: 0, + child: Column( + children: [ + Expanded( + child: FittedBox( + fit: BoxFit.cover, + child: Container( + width: 818.0, + height: 864.0, + child: VideoPlayer(_videoPlayerController)), + )) + ], + )); } } void main() { runApp( MaterialApp( - home: DefaultTabController( - length: 3, - child: Scaffold( - appBar: AppBar( - title: const Text('Video player example'), - bottom: const TabBar( - isScrollable: true, - tabs: [ - Tab( - icon: Icon(Icons.cloud), - text: "Remote", - ), - Tab(icon: Icon(Icons.insert_drive_file), text: "Asset"), - Tab(icon: Icon(Icons.list), text: "List example"), - ], - ), - ), - body: TabBarView( - children: [ - SingleChildScrollView( - child: Column( - children: [ - Container( - padding: const EdgeInsets.only(top: 20.0), - ), - const Text('With remote m3u8'), - Container( - padding: const EdgeInsets.all(20), - child: NetworkPlayerLifeCycle( - 'http://184.72.239.149/vod/smil:BigBuckBunny.smil/playlist.m3u8', - (BuildContext context, - VideoPlayerController controller) => - AspectRatioVideo(controller), - ), - ), - ], - ), - ), - SingleChildScrollView( - child: Column( - children: [ - Container( - padding: const EdgeInsets.only(top: 20.0), - ), - const Text('With assets mp4'), - Container( - padding: const EdgeInsets.all(20), - child: AssetPlayerLifeCycle( - 'assets/Butterfly-209.mp4', - (BuildContext context, - VideoPlayerController controller) => - AspectRatioVideo(controller)), - ), - ], - ), - ), - AssetPlayerLifeCycle( - 'assets/Butterfly-209.mp4', - (BuildContext context, VideoPlayerController controller) => - VideoInListOfCards(controller)), - ], - ), - ), - ), + home: MainPage(), ), ); } diff --git a/packages/video_player/ios/Classes/VideoPlayerPlugin.m b/packages/video_player/ios/Classes/VideoPlayerPlugin.m index bf449ec0e8e2..45e40a9c1c48 100644 --- a/packages/video_player/ios/Classes/VideoPlayerPlugin.m +++ b/packages/video_player/ios/Classes/VideoPlayerPlugin.m @@ -363,6 +363,12 @@ - (CVPixelBufferRef)copyPixelBuffer { } } +- (void)onUnregistered { + dispatch_async(dispatch_get_main_queue(), ^{ + [self dispose]; + }); +} + - (FlutterError* _Nullable)onCancelWithArguments:(id _Nullable)arguments { _eventSink = nil; return nil; @@ -487,7 +493,7 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { if ([@"dispose" isEqualToString:call.method]) { [_registry unregisterTexture:textureId]; [_players removeObjectForKey:@(textureId)]; - [player dispose]; + //[player dispose]; result(nil); } else if ([@"setLooping" isEqualToString:call.method]) { [player setIsLooping:[argsMap[@"looping"] boolValue]]; From b850f20d796415a8dfa62eda016800c580f4d7b5 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 30 Sep 2019 09:54:34 -0700 Subject: [PATCH 02/28] remove commmented out code --- packages/video_player/ios/Classes/VideoPlayerPlugin.m | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/video_player/ios/Classes/VideoPlayerPlugin.m b/packages/video_player/ios/Classes/VideoPlayerPlugin.m index 45e40a9c1c48..46f7dc95eea1 100644 --- a/packages/video_player/ios/Classes/VideoPlayerPlugin.m +++ b/packages/video_player/ios/Classes/VideoPlayerPlugin.m @@ -493,7 +493,6 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { if ([@"dispose" isEqualToString:call.method]) { [_registry unregisterTexture:textureId]; [_players removeObjectForKey:@(textureId)]; - //[player dispose]; result(nil); } else if ([@"setLooping" isEqualToString:call.method]) { [player setIsLooping:[argsMap[@"looping"] boolValue]]; From 185bd0914178c00dda657325d7e606dd59b0f1b3 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 30 Sep 2019 09:55:33 -0700 Subject: [PATCH 03/28] revert changes in example app --- packages/video_player/example/lib/main.dart | 434 ++++++++++++++++++-- 1 file changed, 394 insertions(+), 40 deletions(-) diff --git a/packages/video_player/example/lib/main.dart b/packages/video_player/example/lib/main.dart index bd704f23e2ed..dea1086593df 100644 --- a/packages/video_player/example/lib/main.dart +++ b/packages/video_player/example/lib/main.dart @@ -1,78 +1,432 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/// An example of using the plugin, controlling lifecycle and playback of the +/// video. + +import 'package:flutter/cupertino.dart'; import 'package:flutter/material.dart'; import 'package:video_player/video_player.dart'; -class MainPage extends StatelessWidget { +/// Controls play and pause of [controller]. +/// +/// Toggles play/pause on tap (accompanied by a fading status icon). +/// +/// Plays (looping) on initialization, and mutes on deactivation. +class VideoPlayPause extends StatefulWidget { + VideoPlayPause(this.controller); + + final VideoPlayerController controller; + + @override + State createState() { + return _VideoPlayPauseState(); + } +} + +class _VideoPlayPauseState extends State { + _VideoPlayPauseState() { + listener = () { + setState(() {}); + }; + } + + FadeAnimation imageFadeAnim = + FadeAnimation(child: const Icon(Icons.play_arrow, size: 100.0)); + VoidCallback listener; + + VideoPlayerController get controller => widget.controller; + + @override + void initState() { + super.initState(); + controller.addListener(listener); + controller.setVolume(1.0); + controller.play(); + } + + @override + void deactivate() { + controller.setVolume(0.0); + controller.removeListener(listener); + super.deactivate(); + } + @override Widget build(BuildContext context) { - return Scaffold( - body: Center( - child: FlatButton( - child: const Text('push'), - onPressed: () { - Navigator.push( - context, - MaterialPageRoute( - builder: (BuildContext context) => VideoCrashPlayerPage()), - ); + final List children = [ + GestureDetector( + child: VideoPlayer(controller), + onTap: () { + if (!controller.value.initialized) { + return; + } + if (controller.value.isPlaying) { + imageFadeAnim = + FadeAnimation(child: const Icon(Icons.pause, size: 100.0)); + controller.pause(); + } else { + imageFadeAnim = + FadeAnimation(child: const Icon(Icons.play_arrow, size: 100.0)); + controller.play(); + } }, ), - )); + Align( + alignment: Alignment.bottomCenter, + child: VideoProgressIndicator( + controller, + allowScrubbing: true, + ), + ), + Center(child: imageFadeAnim), + Center( + child: controller.value.isBuffering + ? const CircularProgressIndicator() + : null), + ]; + + return Stack( + fit: StackFit.passthrough, + children: children, + ); } } -class VideoCrashPlayerPage extends StatefulWidget { +class FadeAnimation extends StatefulWidget { + FadeAnimation( + {this.child, this.duration = const Duration(milliseconds: 500)}); + + final Widget child; + final Duration duration; + @override - _VideoCrashPlayerPageState createState() => _VideoCrashPlayerPageState(); + _FadeAnimationState createState() => _FadeAnimationState(); } -class _VideoCrashPlayerPageState extends State { - VideoPlayerController _videoPlayerController; +class _FadeAnimationState extends State + with SingleTickerProviderStateMixin { + AnimationController animationController; @override void initState() { super.initState(); - - _videoPlayerController = - VideoPlayerController.asset("assets/Butterfly-209.mp4"); - _videoPlayerController.addListener(() { - if (!_videoPlayerController.value.isPlaying) { - Navigator.pop(context); + animationController = + AnimationController(duration: widget.duration, vsync: this); + animationController.addListener(() { + if (mounted) { + setState(() {}); } }); - _videoPlayerController.play(); - _videoPlayerController.initialize(); + animationController.forward(from: 0.0); + } + + @override + void deactivate() { + animationController.stop(); + super.deactivate(); + } + + @override + void didUpdateWidget(FadeAnimation oldWidget) { + super.didUpdateWidget(oldWidget); + if (oldWidget.child != widget.child) { + animationController.forward(from: 0.0); + } } @override void dispose() { - _videoPlayerController.dispose(); + animationController.dispose(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + return animationController.isAnimating + ? Opacity( + opacity: 1.0 - animationController.value, + child: widget.child, + ) + : Container(); + } +} + +typedef Widget VideoWidgetBuilder( + BuildContext context, VideoPlayerController controller); + +abstract class PlayerLifeCycle extends StatefulWidget { + PlayerLifeCycle(this.dataSource, this.childBuilder); + + final VideoWidgetBuilder childBuilder; + final String dataSource; +} + +/// A widget connecting its life cycle to a [VideoPlayerController] using +/// a data source from the network. +class NetworkPlayerLifeCycle extends PlayerLifeCycle { + NetworkPlayerLifeCycle(String dataSource, VideoWidgetBuilder childBuilder) + : super(dataSource, childBuilder); + + @override + _NetworkPlayerLifeCycleState createState() => _NetworkPlayerLifeCycleState(); +} + +/// A widget connecting its life cycle to a [VideoPlayerController] using +/// an asset as data source +class AssetPlayerLifeCycle extends PlayerLifeCycle { + AssetPlayerLifeCycle(String dataSource, VideoWidgetBuilder childBuilder) + : super(dataSource, childBuilder); + + @override + _AssetPlayerLifeCycleState createState() => _AssetPlayerLifeCycleState(); +} + +abstract class _PlayerLifeCycleState extends State { + VideoPlayerController controller; + + @override + + /// Subclasses should implement [createVideoPlayerController], which is used + /// by this method. + void initState() { + super.initState(); + controller = createVideoPlayerController(); + controller.addListener(() { + if (controller.value.hasError) { + print(controller.value.errorDescription); + } + }); + controller.initialize(); + controller.setLooping(true); + controller.play(); + } + @override + void deactivate() { + super.deactivate(); + } + + @override + void dispose() { + controller.dispose(); super.dispose(); } @override Widget build(BuildContext context) { - return Material( - elevation: 0, - child: Column( - children: [ - Expanded( - child: FittedBox( - fit: BoxFit.cover, - child: Container( - width: 818.0, - height: 864.0, - child: VideoPlayer(_videoPlayerController)), - )) - ], - )); + return widget.childBuilder(context, controller); + } + + VideoPlayerController createVideoPlayerController(); +} + +class _NetworkPlayerLifeCycleState extends _PlayerLifeCycleState { + @override + VideoPlayerController createVideoPlayerController() { + return VideoPlayerController.network(widget.dataSource); + } +} + +class _AssetPlayerLifeCycleState extends _PlayerLifeCycleState { + @override + VideoPlayerController createVideoPlayerController() { + return VideoPlayerController.asset(widget.dataSource); + } +} + +/// A filler card to show the video in a list of scrolling contents. +Widget buildCard(String title) { + return Card( + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + ListTile( + leading: const Icon(Icons.airline_seat_flat_angled), + title: Text(title), + ), + // TODO(jackson): Remove when deprecation is on stable branch + // ignore: deprecated_member_use + ButtonTheme.bar( + child: ButtonBar( + children: [ + FlatButton( + child: const Text('BUY TICKETS'), + onPressed: () { + /* ... */ + }, + ), + FlatButton( + child: const Text('SELL TICKETS'), + onPressed: () { + /* ... */ + }, + ), + ], + ), + ), + ], + ), + ); +} + +class VideoInListOfCards extends StatelessWidget { + VideoInListOfCards(this.controller); + + final VideoPlayerController controller; + + @override + Widget build(BuildContext context) { + return ListView( + children: [ + buildCard("Item a"), + buildCard("Item b"), + buildCard("Item c"), + buildCard("Item d"), + buildCard("Item e"), + buildCard("Item f"), + buildCard("Item g"), + Card( + child: Column(children: [ + Column( + children: [ + const ListTile( + leading: Icon(Icons.cake), + title: Text("Video video"), + ), + Stack( + alignment: FractionalOffset.bottomRight + + const FractionalOffset(-0.1, -0.1), + children: [ + AspectRatioVideo(controller), + Image.asset('assets/flutter-mark-square-64.png'), + ]), + ], + ), + ])), + buildCard("Item h"), + buildCard("Item i"), + buildCard("Item j"), + buildCard("Item k"), + buildCard("Item l"), + ], + ); + } +} + +class AspectRatioVideo extends StatefulWidget { + AspectRatioVideo(this.controller); + + final VideoPlayerController controller; + + @override + AspectRatioVideoState createState() => AspectRatioVideoState(); +} + +class AspectRatioVideoState extends State { + VideoPlayerController get controller => widget.controller; + bool initialized = false; + + VoidCallback listener; + + @override + void initState() { + super.initState(); + listener = () { + if (!mounted) { + return; + } + if (initialized != controller.value.initialized) { + initialized = controller.value.initialized; + setState(() {}); + } + }; + controller.addListener(listener); + } + + @override + Widget build(BuildContext context) { + if (initialized) { + return Center( + child: AspectRatio( + aspectRatio: controller.value.aspectRatio, + child: VideoPlayPause(controller), + ), + ); + } else { + return Container(); + } } } void main() { runApp( MaterialApp( - home: MainPage(), + home: DefaultTabController( + length: 3, + child: Scaffold( + appBar: AppBar( + title: const Text('Video player example'), + bottom: const TabBar( + isScrollable: true, + tabs: [ + Tab( + icon: Icon(Icons.cloud), + text: "Remote", + ), + Tab(icon: Icon(Icons.insert_drive_file), text: "Asset"), + Tab(icon: Icon(Icons.list), text: "List example"), + ], + ), + ), + body: TabBarView( + children: [ + SingleChildScrollView( + child: Column( + children: [ + Container( + padding: const EdgeInsets.only(top: 20.0), + ), + const Text('With remote m3u8'), + Container( + padding: const EdgeInsets.all(20), + child: NetworkPlayerLifeCycle( + 'http://184.72.239.149/vod/smil:BigBuckBunny.smil/playlist.m3u8', + (BuildContext context, + VideoPlayerController controller) => + AspectRatioVideo(controller), + ), + ), + ], + ), + ), + SingleChildScrollView( + child: Column( + children: [ + Container( + padding: const EdgeInsets.only(top: 20.0), + ), + const Text('With assets mp4'), + Container( + padding: const EdgeInsets.all(20), + child: AssetPlayerLifeCycle( + 'assets/Butterfly-209.mp4', + (BuildContext context, + VideoPlayerController controller) => + AspectRatioVideo(controller)), + ), + ], + ), + ), + AssetPlayerLifeCycle( + 'assets/Butterfly-209.mp4', + (BuildContext context, VideoPlayerController controller) => + VideoInListOfCards(controller)), + ], + ), + ), + ), ), ); } From 6ff4a2ffceb5c20e881df1f58337c2650ac471c0 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 30 Sep 2019 10:33:43 -0700 Subject: [PATCH 04/28] update version and changelog --- packages/video_player/CHANGELOG.md | 4 ++++ packages/video_player/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/video_player/CHANGELOG.md b/packages/video_player/CHANGELOG.md index abf41afcfbb0..e8794e8df35a 100644 --- a/packages/video_player/CHANGELOG.md +++ b/packages/video_player/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.10.2+1 + +* Dispose 'player' after the texture is unregistered on iOS. + ## 0.10.2 * **Android Only** Adds optional VideoFormat used to signal what format the plugin should try. diff --git a/packages/video_player/pubspec.yaml b/packages/video_player/pubspec.yaml index 3c18e734c964..96374a133405 100644 --- a/packages/video_player/pubspec.yaml +++ b/packages/video_player/pubspec.yaml @@ -2,7 +2,7 @@ name: video_player description: Flutter plugin for displaying inline video with other Flutter widgets on Android and iOS. author: Flutter Team -version: 0.10.2 +version: 0.10.2+1 homepage: https://github.com/flutter/plugins/tree/master/packages/video_player flutter: From 0bd3ca01b89fa2cc8dc889e397f4c062b3d097b9 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 16 Oct 2019 11:33:16 -0700 Subject: [PATCH 05/28] add driver tests --- packages/video_player/CHANGELOG.md | 1 + packages/video_player/example/lib/main.dart | 192 ++++++++++++------ packages/video_player/example/pubspec.yaml | 8 +- .../example/test_driver/video_player.dart | 11 + .../example/test_driver/video_player_e2e.dart | 15 -- .../test_driver/video_player_e2e_test.dart | 15 -- .../test_driver/video_player_test.dart | 26 +++ .../video_player_texture_race.dart | 74 ------- .../ios/Classes/VideoPlayerPlugin.m | 18 +- 9 files changed, 192 insertions(+), 168 deletions(-) create mode 100644 packages/video_player/example/test_driver/video_player.dart delete mode 100644 packages/video_player/example/test_driver/video_player_e2e.dart delete mode 100644 packages/video_player/example/test_driver/video_player_e2e_test.dart create mode 100644 packages/video_player/example/test_driver/video_player_test.dart delete mode 100644 packages/video_player/example/test_driver/video_player_texture_race.dart diff --git a/packages/video_player/CHANGELOG.md b/packages/video_player/CHANGELOG.md index b6e15cb08a68..cc9b9f6d0c08 100644 --- a/packages/video_player/CHANGELOG.md +++ b/packages/video_player/CHANGELOG.md @@ -1,5 +1,6 @@ ## 0.10.2+5 +* Dispose `FLTVideoPlayer` in `onTextureUnregistered` callback on iOS. * Dispose 'player' after the texture is unregistered on iOS. ## 0.10.2+4 diff --git a/packages/video_player/example/lib/main.dart b/packages/video_player/example/lib/main.dart index dea1086593df..a28227fb7e94 100644 --- a/packages/video_player/example/lib/main.dart +++ b/packages/video_player/example/lib/main.dart @@ -360,73 +360,145 @@ class AspectRatioVideoState extends State { } } -void main() { - runApp( - MaterialApp( - home: DefaultTabController( - length: 3, - child: Scaffold( - appBar: AppBar( - title: const Text('Video player example'), - bottom: const TabBar( - isScrollable: true, - tabs: [ - Tab( - icon: Icon(Icons.cloud), - text: "Remote", - ), - Tab(icon: Icon(Icons.insert_drive_file), text: "Asset"), - Tab(icon: Icon(Icons.list), text: "List example"), - ], - ), +class App extends StatelessWidget { + @override + Widget build(BuildContext context) { + return DefaultTabController( + length: 3, + child: Scaffold( + appBar: AppBar( + title: const Text('Video player example'), + actions: [ + IconButton( + key: const ValueKey('push_tab'), + icon: const Icon(Icons.navigation), + onPressed: () { + Navigator.push( + context, + MaterialPageRoute( + builder: (BuildContext context) => + PlayerVideoAndPopPage()), + ); + }, + ) + ], + bottom: const TabBar( + isScrollable: true, + tabs: [ + Tab( + icon: Icon(Icons.cloud), + text: "Remote", + ), + Tab(icon: Icon(Icons.insert_drive_file), text: "Asset"), + Tab(icon: Icon(Icons.list), text: "List example"), + ], ), - body: TabBarView( - children: [ - SingleChildScrollView( - child: Column( - children: [ - Container( - padding: const EdgeInsets.only(top: 20.0), + ), + body: TabBarView( + children: [ + SingleChildScrollView( + child: Column( + children: [ + Container( + padding: const EdgeInsets.only(top: 20.0), + ), + const Text('With remote m3u8'), + Container( + padding: const EdgeInsets.all(20), + child: NetworkPlayerLifeCycle( + 'http://184.72.239.149/vod/smil:BigBuckBunny.smil/playlist.m3u8', + (BuildContext context, + VideoPlayerController controller) => + AspectRatioVideo(controller), ), - const Text('With remote m3u8'), - Container( - padding: const EdgeInsets.all(20), - child: NetworkPlayerLifeCycle( - 'http://184.72.239.149/vod/smil:BigBuckBunny.smil/playlist.m3u8', + ), + ], + ), + ), + SingleChildScrollView( + child: Column( + children: [ + Container( + padding: const EdgeInsets.only(top: 20.0), + ), + const Text('With assets mp4'), + Container( + padding: const EdgeInsets.all(20), + child: AssetPlayerLifeCycle( + 'assets/Butterfly-209.mp4', (BuildContext context, VideoPlayerController controller) => - AspectRatioVideo(controller), - ), - ), - ], - ), + AspectRatioVideo(controller)), + ), + ], ), - SingleChildScrollView( - child: Column( - children: [ - Container( - padding: const EdgeInsets.only(top: 20.0), - ), - const Text('With assets mp4'), - Container( - padding: const EdgeInsets.all(20), - child: AssetPlayerLifeCycle( - 'assets/Butterfly-209.mp4', - (BuildContext context, - VideoPlayerController controller) => - AspectRatioVideo(controller)), - ), - ], - ), - ), - AssetPlayerLifeCycle( - 'assets/Butterfly-209.mp4', - (BuildContext context, VideoPlayerController controller) => - VideoInListOfCards(controller)), - ], - ), + ), + AssetPlayerLifeCycle( + 'assets/Butterfly-209.mp4', + (BuildContext context, VideoPlayerController controller) => + VideoInListOfCards(controller)), + ], ), ), + ); + } +} + +void main() { + runApp( + MaterialApp( + home: App(), ), ); } + +class PlayerVideoAndPopPage extends StatefulWidget { + @override + _PlayerVideoAndPopPageState createState() => _PlayerVideoAndPopPageState(); +} + +class _PlayerVideoAndPopPageState extends State { + VideoPlayerController _videoPlayerController; + + @override + void initState() { + super.initState(); + + _videoPlayerController = + VideoPlayerController.asset("assets/Butterfly-209.mp4"); + _videoPlayerController.addListener(() { + if (!_videoPlayerController.value.isPlaying) { + Navigator.pop(context); + } + }); + _videoPlayerController.play(); + _videoPlayerController.initialize(); + } + + @override + void dispose() { + _videoPlayerController.dispose(); + + super.dispose(); + } + + @override + Widget build(BuildContext context) { + return Material( + elevation: 0, + child: AssetPlayerLifeCycle('assets/Butterfly-209.mp4', + (BuildContext context, VideoPlayerController controller) { + controller.addListener(() { + if (!controller.value.isPlaying) { + Navigator.pop(context); + } + }); + return Center( + child: AspectRatio( + aspectRatio: controller.value.aspectRatio, + child: VideoPlayer(controller), + )); + }), + ); + } +} diff --git a/packages/video_player/example/pubspec.yaml b/packages/video_player/example/pubspec.yaml index da48f06f0796..9fdbdcf47395 100644 --- a/packages/video_player/example/pubspec.yaml +++ b/packages/video_player/example/pubspec.yaml @@ -4,13 +4,15 @@ description: Demonstrates how to use the video_player plugin. dependencies: flutter: sdk: flutter + video_player: + path: ../ dev_dependencies: flutter_test: sdk: flutter - - video_player: - path: ../ + flutter_driver: + sdk: flutter + test: any flutter: uses-material-design: true diff --git a/packages/video_player/example/test_driver/video_player.dart b/packages/video_player/example/test_driver/video_player.dart new file mode 100644 index 000000000000..cc498f41fccb --- /dev/null +++ b/packages/video_player/example/test_driver/video_player.dart @@ -0,0 +1,11 @@ +// Copyright 2019, the Chromium project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:flutter_driver/driver_extension.dart'; +import 'package:video_player_example/main.dart' as app; + +void main() { + enableFlutterDriverExtension(); + app.main(); +} diff --git a/packages/video_player/example/test_driver/video_player_e2e.dart b/packages/video_player/example/test_driver/video_player_e2e.dart deleted file mode 100644 index 6660777b88ae..000000000000 --- a/packages/video_player/example/test_driver/video_player_e2e.dart +++ /dev/null @@ -1,15 +0,0 @@ -// // Copyright 2019, the Chromium project authors. Please see the AUTHORS file -// // for details. All rights reserved. Use of this source code is governed by a -// // BSD-style license that can be found in the LICENSE file. - -// import 'package:flutter_test/flutter_test.dart'; -// import 'video_player_texture_race.dart'; -// import 'package:e2e/e2e.dart'; - -// void main() { -// E2EWidgetsFlutterBinding.ensureInitialized(); - -// testWidgets('Can launch share', (WidgetTester tester) async { -// expect(Share.share('message', subject: 'title'), completes); -// }); -// } \ No newline at end of file diff --git a/packages/video_player/example/test_driver/video_player_e2e_test.dart b/packages/video_player/example/test_driver/video_player_e2e_test.dart deleted file mode 100644 index 642dc8bd6ab2..000000000000 --- a/packages/video_player/example/test_driver/video_player_e2e_test.dart +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright 2019, the Chromium project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -import 'dart:async'; -import 'dart:io'; -import 'package:flutter_driver/flutter_driver.dart'; - -Future main() async { - final FlutterDriver driver = await FlutterDriver.connect(); - final String result = - await driver.requestData(null, timeout: const Duration(minutes: 1)); - driver.close(); - exit(result == 'pass' ? 0 : 1); -} \ No newline at end of file diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart new file mode 100644 index 000000000000..d1d370a3e4fa --- /dev/null +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -0,0 +1,26 @@ +// Copyright 2019, the Chromium project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:async'; +import 'package:flutter_driver/flutter_driver.dart'; +import 'package:test/test.dart'; + +Future main() async { + final FlutterDriver driver = await FlutterDriver.connect(); + tearDownAll(() async { + driver.close(); + }); + + //TODO(cyanglaz): Use TabBar tabs to navigate between pages after https://github.com/flutter/flutter/issues/16991 is fixed. + test('Push a page contains video and pop back, do not crash.', () async { + final SerializableFinder pushTab = find.byValueKey('push_tab'); + await driver.waitFor(pushTab, timeout: const Duration(seconds: 10)); + await driver.tap(pushTab, timeout: const Duration(seconds: 10)); + await driver.waitForAbsent(pushTab, timeout: const Duration(seconds: 10)); + await driver.waitFor(pushTab, timeout: const Duration(seconds: 30)); + final Health health = + await driver.checkHealth(timeout: const Duration(seconds: 10)); + expect(health.status, HealthStatus.ok); + }); +} diff --git a/packages/video_player/example/test_driver/video_player_texture_race.dart b/packages/video_player/example/test_driver/video_player_texture_race.dart deleted file mode 100644 index 065fd8e341d8..000000000000 --- a/packages/video_player/example/test_driver/video_player_texture_race.dart +++ /dev/null @@ -1,74 +0,0 @@ -import 'package:flutter/material.dart'; -import 'package:video_player/video_player.dart'; - -class MainPage extends StatelessWidget { - @override - Widget build(BuildContext context) { - return Scaffold( - body: Center( - child: FlatButton( - child: const Text('push'), - onPressed: () { - Navigator.push( - context, - MaterialPageRoute(builder: (context) => VideoCrashPlayerPage()), - ); - }, - ), - ), - ); - } -} - -class VideoCrashPlayerPage extends StatefulWidget { - @override - _VideoCrashPlayerPageState createState() => _VideoCrashPlayerPageState(); -} - -class _VideoCrashPlayerPageState extends State { - VideoPlayerController _videoPlayerController; - - @override - void initState() { - super.initState(); - - _videoPlayerController = - VideoPlayerController.asset("assets/Butterfly-209.mp4"); - _videoPlayerController.addListener(() { - if (!_videoPlayerController.value.isPlaying) { - Navigator.pop(context); - } - }); - _videoPlayerController.play(); - _videoPlayerController.initialize(); - } - - @override - void dispose() { - _videoPlayerController.dispose(); - - super.dispose(); - } - - @override - Widget build(BuildContext context) { - return Material( - elevation: 0, - child: Column( - children: [ - Expanded( - child: FittedBox( - fit: BoxFit.cover, - child: Container( - width: 818.0, - height: 864.0, - child: VideoPlayer(_videoPlayerController)), - )) - ], - )); - } -} - -void main() { - runApp(MaterialApp(home: MainPage(),)); -} \ No newline at end of file diff --git a/packages/video_player/ios/Classes/VideoPlayerPlugin.m b/packages/video_player/ios/Classes/VideoPlayerPlugin.m index 46f7dc95eea1..4208f0cd9b75 100644 --- a/packages/video_player/ios/Classes/VideoPlayerPlugin.m +++ b/packages/video_player/ios/Classes/VideoPlayerPlugin.m @@ -363,7 +363,7 @@ - (CVPixelBufferRef)copyPixelBuffer { } } -- (void)onUnregistered { +- (void)onTextureUnregistered { dispatch_async(dispatch_get_main_queue(), ^{ [self dispose]; }); @@ -493,6 +493,22 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { if ([@"dispose" isEqualToString:call.method]) { [_registry unregisterTexture:textureId]; [_players removeObjectForKey:@(textureId)]; + // If the Flutter contains https://github.com/flutter/engine/pull/12695, + // the `player` is disposed via `onTextureUnregistered` at the right time. + // Without https://github.com/flutter/engine/pull/12695, there is no gurantee that the texture + // has completed unregistration. It may leads a crash if we dispose the `player` before the + // texture is unregistered. We add a dispatch_after hack to make sure the texture is + // unregistered before we dispose the `player`. + // + // TODO(cyanglaz): Remove this dispatch block when + // https://github.com/flutter/flutter/commit/8159a9906095efc9af8b223f5e232cb63542ad0b is in + // stable And update the min flutter version of the plugin to the stable version. + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)), + dispatch_get_main_queue(), ^{ + if (!player.disposed) { + [player dispose]; + } + }); result(nil); } else if ([@"setLooping" isEqualToString:call.method]) { [player setIsLooping:[argsMap[@"looping"] boolValue]]; From 41c6b35b9ec746b1f5a8f313defaf0ca1c3903da Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 16 Oct 2019 11:36:39 -0700 Subject: [PATCH 06/28] update changelog --- packages/video_player/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/video_player/CHANGELOG.md b/packages/video_player/CHANGELOG.md index cc9b9f6d0c08..861b7c72a4cb 100644 --- a/packages/video_player/CHANGELOG.md +++ b/packages/video_player/CHANGELOG.md @@ -1,7 +1,8 @@ ## 0.10.2+5 * Dispose `FLTVideoPlayer` in `onTextureUnregistered` callback on iOS. -* Dispose 'player' after the texture is unregistered on iOS. +* Add a temporary fix to dispose the `FLTVideoPlayer` with a delay to avoid race condition. +* Updated the example app to include a new page that pop back after video is done playing. ## 0.10.2+4 From 7603c140dfdf87a205c44d9a23db6b15d4890ada Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 16 Oct 2019 11:58:20 -0700 Subject: [PATCH 07/28] formatting --- .../example/test_driver/video_player_test.dart | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart index d1d370a3e4fa..1a1423d10ffd 100644 --- a/packages/video_player/example/test_driver/video_player_test.dart +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -14,6 +14,16 @@ Future main() async { //TODO(cyanglaz): Use TabBar tabs to navigate between pages after https://github.com/flutter/flutter/issues/16991 is fixed. test('Push a page contains video and pop back, do not crash.', () async { + if (Platform.isIOS) { + final SerializableFinder pushTab = find.byValueKey('push_tab'); + await driver.waitFor(pushTab, timeout: const Duration(seconds: 10)); + await driver.tap(pushTab, timeout: const Duration(seconds: 10)); + await driver.waitForAbsent(pushTab, timeout: const Duration(seconds: 10)); + await driver.waitFor(pushTab, timeout: const Duration(seconds: 30)); + final Health health = + await driver.checkHealth(timeout: const Duration(seconds: 10)); + expect(health.status, HealthStatus.ok); + } final SerializableFinder pushTab = find.byValueKey('push_tab'); await driver.waitFor(pushTab, timeout: const Duration(seconds: 10)); await driver.tap(pushTab, timeout: const Duration(seconds: 10)); From ab9f4795c21fcd3972dca5daa587808fe4c0ab3e Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 16 Oct 2019 12:08:30 -0700 Subject: [PATCH 08/28] remove platform check --- .../example/test_driver/video_player_test.dart | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart index 1a1423d10ffd..d1d370a3e4fa 100644 --- a/packages/video_player/example/test_driver/video_player_test.dart +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -14,16 +14,6 @@ Future main() async { //TODO(cyanglaz): Use TabBar tabs to navigate between pages after https://github.com/flutter/flutter/issues/16991 is fixed. test('Push a page contains video and pop back, do not crash.', () async { - if (Platform.isIOS) { - final SerializableFinder pushTab = find.byValueKey('push_tab'); - await driver.waitFor(pushTab, timeout: const Duration(seconds: 10)); - await driver.tap(pushTab, timeout: const Duration(seconds: 10)); - await driver.waitForAbsent(pushTab, timeout: const Duration(seconds: 10)); - await driver.waitFor(pushTab, timeout: const Duration(seconds: 30)); - final Health health = - await driver.checkHealth(timeout: const Duration(seconds: 10)); - expect(health.status, HealthStatus.ok); - } final SerializableFinder pushTab = find.byValueKey('push_tab'); await driver.waitFor(pushTab, timeout: const Duration(seconds: 10)); await driver.tap(pushTab, timeout: const Duration(seconds: 10)); From 7b9031980e2ca1c760f74ef19beac0cab31a4a3a Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 16 Oct 2019 12:52:31 -0700 Subject: [PATCH 09/28] fix main app not able to find recourses --- packages/video_player/example/lib/main.dart | 28 ++++++--------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/packages/video_player/example/lib/main.dart b/packages/video_player/example/lib/main.dart index a28227fb7e94..40c8fdb991ee 100644 --- a/packages/video_player/example/lib/main.dart +++ b/packages/video_player/example/lib/main.dart @@ -465,7 +465,7 @@ class _PlayerVideoAndPopPageState extends State { super.initState(); _videoPlayerController = - VideoPlayerController.asset("assets/Butterfly-209.mp4"); + VideoPlayerController.asset('assets/Butterfly-209.mp4'); _videoPlayerController.addListener(() { if (!_videoPlayerController.value.isPlaying) { Navigator.pop(context); @@ -475,30 +475,16 @@ class _PlayerVideoAndPopPageState extends State { _videoPlayerController.initialize(); } - @override - void dispose() { - _videoPlayerController.dispose(); - - super.dispose(); - } - @override Widget build(BuildContext context) { return Material( elevation: 0, - child: AssetPlayerLifeCycle('assets/Butterfly-209.mp4', - (BuildContext context, VideoPlayerController controller) { - controller.addListener(() { - if (!controller.value.isPlaying) { - Navigator.pop(context); - } - }); - return Center( - child: AspectRatio( - aspectRatio: controller.value.aspectRatio, - child: VideoPlayer(controller), - )); - }), + child: Center( + child: AspectRatio( + aspectRatio: _videoPlayerController.value.aspectRatio, + child: VideoPlayer(_videoPlayerController), + ), + ), ); } } From a0447ebbd7815b3962e1e5d257f1b955ad86ce82 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 16 Oct 2019 15:58:44 -0700 Subject: [PATCH 10/28] fix example app --- packages/video_player/example/lib/main.dart | 26 ++++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/video_player/example/lib/main.dart b/packages/video_player/example/lib/main.dart index 40c8fdb991ee..b2e10c1e32c4 100644 --- a/packages/video_player/example/lib/main.dart +++ b/packages/video_player/example/lib/main.dart @@ -459,6 +459,7 @@ class PlayerVideoAndPopPage extends StatefulWidget { class _PlayerVideoAndPopPageState extends State { VideoPlayerController _videoPlayerController; + bool startedPlaying = false; @override void initState() { @@ -467,12 +468,17 @@ class _PlayerVideoAndPopPageState extends State { _videoPlayerController = VideoPlayerController.asset('assets/Butterfly-209.mp4'); _videoPlayerController.addListener(() { - if (!_videoPlayerController.value.isPlaying) { + if (startedPlaying && !_videoPlayerController.value.isPlaying) { Navigator.pop(context); } }); - _videoPlayerController.play(); - _videoPlayerController.initialize(); + } + + Future started() async { + await _videoPlayerController.initialize(); + await _videoPlayerController.play(); + startedPlaying = true; + return true; } @override @@ -480,9 +486,17 @@ class _PlayerVideoAndPopPageState extends State { return Material( elevation: 0, child: Center( - child: AspectRatio( - aspectRatio: _videoPlayerController.value.aspectRatio, - child: VideoPlayer(_videoPlayerController), + child: FutureBuilder( + future: started(), + builder: (BuildContext context, AsyncSnapshot snapshot) { + if (snapshot.data == true) { + return AspectRatio( + aspectRatio: _videoPlayerController.value.aspectRatio, + child: VideoPlayer(_videoPlayerController)); + } else { + return const Text('waiting for video to load'); + } + }, ), ), ); From 7c6c8fde1af26474a8d8a80ee315f9f41b37cad5 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 16 Oct 2019 16:44:23 -0700 Subject: [PATCH 11/28] draft --- packages/video_player/example/lib/main.dart | 70 +++++++++++++++++++ .../test_driver/video_player_test.dart | 3 +- 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/packages/video_player/example/lib/main.dart b/packages/video_player/example/lib/main.dart index b2e10c1e32c4..3004251c4951 100644 --- a/packages/video_player/example/lib/main.dart +++ b/packages/video_player/example/lib/main.dart @@ -366,6 +366,7 @@ class App extends StatelessWidget { return DefaultTabController( length: 3, child: Scaffold( + key: const ValueKey('main_page'), appBar: AppBar( title: const Text('Video player example'), actions: [ @@ -502,3 +503,72 @@ class _PlayerVideoAndPopPageState extends State { ); } } + +// class VideoCrashPlayerPage extends StatefulWidget { +// @override +// _VideoCrashPlayerPageState createState() => _VideoCrashPlayerPageState(); +// } + +// class _VideoCrashPlayerPageState extends State { +// VideoPlayerController _videoPlayerController; +// bool startedPlaying = false; + +// @override +// void initState() { +// super.initState(); + +// _videoPlayerController = +// VideoPlayerController.asset("assets/Butterfly-209.mp4"); +// _videoPlayerController.addListener(() { +// if (startedPlaying && !_videoPlayerController.value.isPlaying) { +// Navigator.pop(context); +// } +// }); + +// // _videoPlayerController.play(); +// // _videoPlayerController.initialize(); +// } + +// Future started() async { +// await _videoPlayerController.initialize(); +// await _videoPlayerController.play(); +// startedPlaying = true; +// return true; +// } + +// @override +// void dispose() { +// _videoPlayerController.dispose(); + +// super.dispose(); +// } + +// @override +// Widget build(BuildContext context) { +// return Material( +// elevation: 0, +// child: Column( +// children: [ +// Expanded( +// child: FittedBox( +// fit: BoxFit.cover, +// child: Container( +// width: 818.0, +// height: 864.0, +// child: FutureBuilder( +// future: started(), +// builder: +// (BuildContext context, AsyncSnapshot snapshot) { +// if (snapshot.data == true) { +// return VideoPlayer(_videoPlayerController); +// } else { +// return const Text('waiting for video to load'); +// } +// }, +// ), +// ), +// )) +// ], +// )); +// } +// } diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart index d1d370a3e4fa..96873246fa05 100644 --- a/packages/video_player/example/test_driver/video_player_test.dart +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -18,7 +18,8 @@ Future main() async { await driver.waitFor(pushTab, timeout: const Duration(seconds: 10)); await driver.tap(pushTab, timeout: const Duration(seconds: 10)); await driver.waitForAbsent(pushTab, timeout: const Duration(seconds: 10)); - await driver.waitFor(pushTab, timeout: const Duration(seconds: 30)); + await driver.waitFor(find.byValueKey('main_page')); + await driver.waitFor(pushTab); final Health health = await driver.checkHealth(timeout: const Duration(seconds: 10)); expect(health.status, HealthStatus.ok); From 93af8cd9dd4b512494db08c8b17d02c934444fd3 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 16 Oct 2019 16:55:53 -0700 Subject: [PATCH 12/28] fix example app --- packages/video_player/example/lib/main.dart | 8 +++++++- .../example/test_driver/video_player_test.dart | 11 +++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/video_player/example/lib/main.dart b/packages/video_player/example/lib/main.dart index 3004251c4951..8ebcf47bea32 100644 --- a/packages/video_player/example/lib/main.dart +++ b/packages/video_player/example/lib/main.dart @@ -366,7 +366,7 @@ class App extends StatelessWidget { return DefaultTabController( length: 3, child: Scaffold( - key: const ValueKey('main_page'), + key: const ValueKey('home_page'), appBar: AppBar( title: const Text('Video player example'), actions: [ @@ -475,6 +475,12 @@ class _PlayerVideoAndPopPageState extends State { }); } + @override + void dispose() { + _videoPlayerController.dispose(); + super.dispose(); + } + Future started() async { await _videoPlayerController.initialize(); await _videoPlayerController.play(); diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart index 96873246fa05..f5cbd90f1645 100644 --- a/packages/video_player/example/test_driver/video_player_test.dart +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -15,13 +15,12 @@ Future main() async { //TODO(cyanglaz): Use TabBar tabs to navigate between pages after https://github.com/flutter/flutter/issues/16991 is fixed. test('Push a page contains video and pop back, do not crash.', () async { final SerializableFinder pushTab = find.byValueKey('push_tab'); - await driver.waitFor(pushTab, timeout: const Duration(seconds: 10)); - await driver.tap(pushTab, timeout: const Duration(seconds: 10)); - await driver.waitForAbsent(pushTab, timeout: const Duration(seconds: 10)); - await driver.waitFor(find.byValueKey('main_page')); await driver.waitFor(pushTab); - final Health health = - await driver.checkHealth(timeout: const Duration(seconds: 10)); + await driver.tap(pushTab); + await driver.waitForAbsent(pushTab); + await driver.waitFor(find.byValueKey('home_page')); + await driver.waitUntilNoTransientCallbacks(); + final Health health = await driver.checkHealth(); expect(health.status, HealthStatus.ok); }); } From 5b97567dad98b08ebb3c234e35adde783d8d467e Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 16 Oct 2019 17:12:49 -0700 Subject: [PATCH 13/28] adding a bigger timeout --- .../video_player/example/test_driver/video_player_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart index f5cbd90f1645..7ac27c8b07fa 100644 --- a/packages/video_player/example/test_driver/video_player_test.dart +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -22,5 +22,5 @@ Future main() async { await driver.waitUntilNoTransientCallbacks(); final Health health = await driver.checkHealth(); expect(health.status, HealthStatus.ok); - }); + }, timeout: const Timeout(Duration(minutes: 10))); } From 82cc0295ef5bd625af814e54bcc56c75ec9517ab Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Thu, 17 Oct 2019 21:35:48 -0700 Subject: [PATCH 14/28] Update pubspec.yaml --- packages/video_player/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/video_player/pubspec.yaml b/packages/video_player/pubspec.yaml index f86383d0737b..1b0805be1764 100644 --- a/packages/video_player/pubspec.yaml +++ b/packages/video_player/pubspec.yaml @@ -2,7 +2,7 @@ name: video_player description: Flutter plugin for displaying inline video with other Flutter widgets on Android and iOS. author: Flutter Team -version: 0.10.2+5 +version: 0.10.2+6 homepage: https://github.com/flutter/plugins/tree/master/packages/video_player flutter: From 3ef701214136e47b5c1462724e5b5362a11fd3e6 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 18 Oct 2019 09:09:29 -0700 Subject: [PATCH 15/28] skip the failing test --- packages/video_player/example/lib/main.dart | 69 ------------------- .../test_driver/video_player_test.dart | 4 +- 2 files changed, 3 insertions(+), 70 deletions(-) diff --git a/packages/video_player/example/lib/main.dart b/packages/video_player/example/lib/main.dart index 8ebcf47bea32..e63b155d98b1 100644 --- a/packages/video_player/example/lib/main.dart +++ b/packages/video_player/example/lib/main.dart @@ -509,72 +509,3 @@ class _PlayerVideoAndPopPageState extends State { ); } } - -// class VideoCrashPlayerPage extends StatefulWidget { -// @override -// _VideoCrashPlayerPageState createState() => _VideoCrashPlayerPageState(); -// } - -// class _VideoCrashPlayerPageState extends State { -// VideoPlayerController _videoPlayerController; -// bool startedPlaying = false; - -// @override -// void initState() { -// super.initState(); - -// _videoPlayerController = -// VideoPlayerController.asset("assets/Butterfly-209.mp4"); -// _videoPlayerController.addListener(() { -// if (startedPlaying && !_videoPlayerController.value.isPlaying) { -// Navigator.pop(context); -// } -// }); - -// // _videoPlayerController.play(); -// // _videoPlayerController.initialize(); -// } - -// Future started() async { -// await _videoPlayerController.initialize(); -// await _videoPlayerController.play(); -// startedPlaying = true; -// return true; -// } - -// @override -// void dispose() { -// _videoPlayerController.dispose(); - -// super.dispose(); -// } - -// @override -// Widget build(BuildContext context) { -// return Material( -// elevation: 0, -// child: Column( -// children: [ -// Expanded( -// child: FittedBox( -// fit: BoxFit.cover, -// child: Container( -// width: 818.0, -// height: 864.0, -// child: FutureBuilder( -// future: started(), -// builder: -// (BuildContext context, AsyncSnapshot snapshot) { -// if (snapshot.data == true) { -// return VideoPlayer(_videoPlayerController); -// } else { -// return const Text('waiting for video to load'); -// } -// }, -// ), -// ), -// )) -// ], -// )); -// } -// } diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart index 7ac27c8b07fa..fce6d22319df 100644 --- a/packages/video_player/example/test_driver/video_player_test.dart +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -22,5 +22,7 @@ Future main() async { await driver.waitUntilNoTransientCallbacks(); final Health health = await driver.checkHealth(); expect(health.status, HealthStatus.ok); - }, timeout: const Timeout(Duration(minutes: 10))); + }, + skip: + 'This test works locally but fails on CI because of timing out on waitFor.'); } From ca7cc692f62d5040cf547f2653dc2072192e79fa Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 18 Oct 2019 09:43:19 -0700 Subject: [PATCH 16/28] add todo --- packages/video_player/example/test_driver/video_player_test.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart index fce6d22319df..ce204a76bd6f 100644 --- a/packages/video_player/example/test_driver/video_player_test.dart +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -13,6 +13,7 @@ Future main() async { }); //TODO(cyanglaz): Use TabBar tabs to navigate between pages after https://github.com/flutter/flutter/issues/16991 is fixed. + //TODO(cyanglaz): Un-skip the test after https://github.com/flutter/flutter/issues/43012 is fixed test('Push a page contains video and pop back, do not crash.', () async { final SerializableFinder pushTab = find.byValueKey('push_tab'); await driver.waitFor(pushTab); From 08dee5ad53934682aaeafc1b81f49e3266aa63e1 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 30 Oct 2019 12:31:26 -0700 Subject: [PATCH 17/28] edit skip message --- .../video_player/example/test_driver/video_player_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart index ce204a76bd6f..e265620b41a3 100644 --- a/packages/video_player/example/test_driver/video_player_test.dart +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -25,5 +25,5 @@ Future main() async { expect(health.status, HealthStatus.ok); }, skip: - 'This test works locally but fails on CI because of timing out on waitFor.'); + 'This test works locally but fails on CI because of timing out on waitFor. Un-skip this after iOS test is moved to firebase device lab on real devices'); } From 83e78be393bc3ef1ba5a4dc7f27e5d9f5ea8ea4c Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 30 Oct 2019 12:34:09 -0700 Subject: [PATCH 18/28] edit skip message --- .../video_player/example/test_driver/video_player_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart index e265620b41a3..1bad033c7866 100644 --- a/packages/video_player/example/test_driver/video_player_test.dart +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -25,5 +25,6 @@ Future main() async { expect(health.status, HealthStatus.ok); }, skip: - 'This test works locally but fails on CI because of timing out on waitFor. Un-skip this after iOS test is moved to firebase device lab on real devices'); + 'This test works locally but fails on CI because the simulator is not able to access the local video assets.' + 'Un-skip this after iOS test is moved to firebase device lab on real devices'); } From 2c1a5803960a8199c0935f61bd1caca0029c9acf Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 30 Oct 2019 13:35:48 -0700 Subject: [PATCH 19/28] formatting --- .../video_player/example/test_driver/video_player_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart index 1bad033c7866..9280997a7ca2 100644 --- a/packages/video_player/example/test_driver/video_player_test.dart +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -25,6 +25,6 @@ Future main() async { expect(health.status, HealthStatus.ok); }, skip: - 'This test works locally but fails on CI because the simulator is not able to access the local video assets.' - 'Un-skip this after iOS test is moved to firebase device lab on real devices'); + 'This test would fail on CI because the simulator is not able to access the local video resources.' + 'Un-skip this test after iOS integration test is moved to firebase device lab on real devices'); } From 9e89c12b883e9e2c09f0919cccecd7b4543ace06 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 12 Nov 2019 17:47:48 -0800 Subject: [PATCH 20/28] unskip --- .../video_player/example/test_driver/video_player_test.dart | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart index 9280997a7ca2..c520301e13b4 100644 --- a/packages/video_player/example/test_driver/video_player_test.dart +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -23,8 +23,5 @@ Future main() async { await driver.waitUntilNoTransientCallbacks(); final Health health = await driver.checkHealth(); expect(health.status, HealthStatus.ok); - }, - skip: - 'This test would fail on CI because the simulator is not able to access the local video resources.' - 'Un-skip this test after iOS integration test is moved to firebase device lab on real devices'); + },); } From c9db5762e947df844a2abcb4a3a7d1de3f985959 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 13 Nov 2019 16:02:19 -0800 Subject: [PATCH 21/28] merge master --- .../example/test_driver/video_player.dart | 11 +++++++ .../test_driver/video_player_e2e_test.dart | 12 ------- .../test_driver/video_player_test.dart | 31 +++++++++++++++++++ 3 files changed, 42 insertions(+), 12 deletions(-) create mode 100644 packages/video_player/example/test_driver/video_player.dart create mode 100644 packages/video_player/example/test_driver/video_player_test.dart diff --git a/packages/video_player/example/test_driver/video_player.dart b/packages/video_player/example/test_driver/video_player.dart new file mode 100644 index 000000000000..3bc3b67c352e --- /dev/null +++ b/packages/video_player/example/test_driver/video_player.dart @@ -0,0 +1,11 @@ +// Copyright 2019, the Chromium project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:flutter_driver/driver_extension.dart'; +import 'package:video_player_example/main.dart' as app; + +void main() { + enableFlutterDriverExtension(); + app.main(); +} \ No newline at end of file diff --git a/packages/video_player/example/test_driver/video_player_e2e_test.dart b/packages/video_player/example/test_driver/video_player_e2e_test.dart index 98c8800f1599..80d7093b791a 100644 --- a/packages/video_player/example/test_driver/video_player_e2e_test.dart +++ b/packages/video_player/example/test_driver/video_player_e2e_test.dart @@ -6,7 +6,6 @@ import 'dart:async'; import 'dart:io'; import 'package:flutter_driver/flutter_driver.dart'; -import 'package:test/test.dart'; import 'package:video_player_example/main.dart' as app; Future main() async { @@ -16,15 +15,4 @@ Future main() async { app.main(); await driver.close(); exit(result == 'pass' ? 0 : 1); - - test('Push a page contains video and pop back, do not crash.', () async { - final SerializableFinder pushTab = find.byValueKey('push_tab'); - await driver.waitFor(pushTab); - await driver.tap(pushTab); - await driver.waitForAbsent(pushTab); - await driver.waitFor(find.byValueKey('home_page')); - await driver.waitUntilNoTransientCallbacks(); - final Health health = await driver.checkHealth(); - expect(health.status, HealthStatus.ok); - },); } diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart new file mode 100644 index 000000000000..69241b1ef8d2 --- /dev/null +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -0,0 +1,31 @@ +// Copyright 2019, the Chromium project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:async'; +import 'package:flutter_driver/flutter_driver.dart'; +import 'package:test/test.dart'; + +Future main() async { + final FlutterDriver driver = await FlutterDriver.connect(); + tearDownAll(() async { + driver.close(); + }); + + //TODO(cyanglaz): Use TabBar tabs to navigate between pages after https://github.com/flutter/flutter/issues/16991 is fixed. + //TODO(cyanglaz): Un-skip the test after https://github.com/flutter/flutter/issues/43012 is fixed + test('Push a page contains video and pop back, do not crash.', () async { + final SerializableFinder pushTab = find.byValueKey('push_tab'); + await driver.waitFor(pushTab); + await driver.tap(pushTab); + await driver.waitForAbsent(pushTab); + await driver.waitFor(find.byValueKey('home_page')); + await driver.waitUntilNoTransientCallbacks(); + final Health health = await driver.checkHealth(); + expect(health.status, HealthStatus.ok); + },); + + test('failure', () async{ + expect(true, false); + }); +} \ No newline at end of file From eb0ee98315bcc2495c74322412ec5ed714b3678e Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 13 Nov 2019 16:18:05 -0800 Subject: [PATCH 22/28] formatting --- packages/video_player/example/lib/main.dart | 10 +++++----- .../video_player/example/test_driver/video_player.dart | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/video_player/example/lib/main.dart b/packages/video_player/example/lib/main.dart index 0eed5ebc6bc2..6e05ac67bc8a 100644 --- a/packages/video_player/example/lib/main.dart +++ b/packages/video_player/example/lib/main.dart @@ -403,11 +403,11 @@ class App extends StatelessWidget { Container( padding: const EdgeInsets.only(top: 20.0), ), - const Text('With remote mp4'), - Container( - padding: const EdgeInsets.all(20), - child: NetworkPlayerLifeCycle( - 'https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4', + const Text('With remote mp4'), + Container( + padding: const EdgeInsets.all(20), + child: NetworkPlayerLifeCycle( + 'https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4', (BuildContext context, VideoPlayerController controller) => AspectRatioVideo(controller), diff --git a/packages/video_player/example/test_driver/video_player.dart b/packages/video_player/example/test_driver/video_player.dart index 3bc3b67c352e..cc498f41fccb 100644 --- a/packages/video_player/example/test_driver/video_player.dart +++ b/packages/video_player/example/test_driver/video_player.dart @@ -8,4 +8,4 @@ import 'package:video_player_example/main.dart' as app; void main() { enableFlutterDriverExtension(); app.main(); -} \ No newline at end of file +} From c7dfb905ca736e4819c9dab6bdf2ce355b8fd163 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 20 Nov 2019 09:17:30 -0800 Subject: [PATCH 23/28] updates --- packages/video_player/example/lib/main.dart | 8 ++++++-- .../example/test_driver/video_player_test.dart | 2 +- .../video_player/ios/Classes/VideoPlayerPlugin.m | 16 ---------------- packages/video_player/pubspec.yaml | 2 +- 4 files changed, 8 insertions(+), 20 deletions(-) diff --git a/packages/video_player/example/lib/main.dart b/packages/video_player/example/lib/main.dart index 6e05ac67bc8a..1c104d3414fa 100644 --- a/packages/video_player/example/lib/main.dart +++ b/packages/video_player/example/lib/main.dart @@ -8,6 +8,7 @@ import 'package:flutter/cupertino.dart'; import 'package:flutter/material.dart'; import 'package:video_player/video_player.dart'; +import 'package:flutter/scheduler.dart'; /// Controls play and pause of [controller]. /// @@ -28,7 +29,7 @@ class VideoPlayPause extends StatefulWidget { class _VideoPlayPauseState extends State { _VideoPlayPauseState() { listener = () { - setState(() {}); + SchedulerBinding.instance.addPostFrameCallback((_) => setState(() {})); }; } @@ -48,8 +49,11 @@ class _VideoPlayPauseState extends State { @override void deactivate() { - controller.setVolume(0.0); + SchedulerBinding.instance.addPostFrameCallback((_) { + controller.setVolume(0.0); controller.removeListener(listener); + }); + super.deactivate(); } diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart index 7f786f9b2758..57da878fbe27 100644 --- a/packages/video_player/example/test_driver/video_player_test.dart +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -24,5 +24,5 @@ Future main() async { await driver.waitUntilNoTransientCallbacks(); final Health health = await driver.checkHealth(); expect(health.status, HealthStatus.ok); - }, skip: Platform.isIOS); + }, skip: 'Cirrus CI currently hangs while playing videos'); } diff --git a/packages/video_player/ios/Classes/VideoPlayerPlugin.m b/packages/video_player/ios/Classes/VideoPlayerPlugin.m index 4208f0cd9b75..094ce75f67bb 100644 --- a/packages/video_player/ios/Classes/VideoPlayerPlugin.m +++ b/packages/video_player/ios/Classes/VideoPlayerPlugin.m @@ -493,22 +493,6 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { if ([@"dispose" isEqualToString:call.method]) { [_registry unregisterTexture:textureId]; [_players removeObjectForKey:@(textureId)]; - // If the Flutter contains https://github.com/flutter/engine/pull/12695, - // the `player` is disposed via `onTextureUnregistered` at the right time. - // Without https://github.com/flutter/engine/pull/12695, there is no gurantee that the texture - // has completed unregistration. It may leads a crash if we dispose the `player` before the - // texture is unregistered. We add a dispatch_after hack to make sure the texture is - // unregistered before we dispose the `player`. - // - // TODO(cyanglaz): Remove this dispatch block when - // https://github.com/flutter/flutter/commit/8159a9906095efc9af8b223f5e232cb63542ad0b is in - // stable And update the min flutter version of the plugin to the stable version. - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)), - dispatch_get_main_queue(), ^{ - if (!player.disposed) { - [player dispose]; - } - }); result(nil); } else if ([@"setLooping" isEqualToString:call.method]) { [player setIsLooping:[argsMap[@"looping"] boolValue]]; diff --git a/packages/video_player/pubspec.yaml b/packages/video_player/pubspec.yaml index 516b6f778c84..f85c5f80793a 100644 --- a/packages/video_player/pubspec.yaml +++ b/packages/video_player/pubspec.yaml @@ -22,4 +22,4 @@ dev_dependencies: environment: sdk: ">=2.0.0-dev.28.0 <3.0.0" - flutter: ">=1.9.1+hotfix.5 <2.0.0" + flutter: ">=1.10.15 <2.0.0" From 2577b60fb0353f7f1cefc3b31d78a306c336798c Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 20 Nov 2019 09:17:55 -0800 Subject: [PATCH 24/28] remove unnecessary imports --- packages/video_player/example/test_driver/video_player_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/video_player/example/test_driver/video_player_test.dart b/packages/video_player/example/test_driver/video_player_test.dart index 57da878fbe27..a72c6944cfbe 100644 --- a/packages/video_player/example/test_driver/video_player_test.dart +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -3,7 +3,6 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; -import 'dart:io'; import 'package:flutter_driver/flutter_driver.dart'; import 'package:test/test.dart'; From a0fbd5f7d34d704565dabbbecfd318bdfec41282 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 20 Nov 2019 09:21:00 -0800 Subject: [PATCH 25/28] remove extra app.main --- .../example/test_driver/video_player_e2e_test.dart | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/video_player/example/test_driver/video_player_e2e_test.dart b/packages/video_player/example/test_driver/video_player_e2e_test.dart index 80d7093b791a..f3aa9e218d82 100644 --- a/packages/video_player/example/test_driver/video_player_e2e_test.dart +++ b/packages/video_player/example/test_driver/video_player_e2e_test.dart @@ -4,15 +4,12 @@ import 'dart:async'; import 'dart:io'; - import 'package:flutter_driver/flutter_driver.dart'; -import 'package:video_player_example/main.dart' as app; Future main() async { final FlutterDriver driver = await FlutterDriver.connect(); final String result = await driver.requestData(null, timeout: const Duration(minutes: 1)); - app.main(); await driver.close(); exit(result == 'pass' ? 0 : 1); } From 8d9783848cd77d8e53b56b5d33761f64eb077f05 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 20 Nov 2019 10:14:27 -0800 Subject: [PATCH 26/28] revert flutter lower bound --- packages/video_player/example/lib/main.dart | 4 ++-- .../video_player/ios/Classes/VideoPlayerPlugin.m | 16 ++++++++++++++++ packages/video_player/pubspec.yaml | 2 +- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/video_player/example/lib/main.dart b/packages/video_player/example/lib/main.dart index 1c104d3414fa..43d9355c4345 100644 --- a/packages/video_player/example/lib/main.dart +++ b/packages/video_player/example/lib/main.dart @@ -50,8 +50,8 @@ class _VideoPlayPauseState extends State { @override void deactivate() { SchedulerBinding.instance.addPostFrameCallback((_) { - controller.setVolume(0.0); - controller.removeListener(listener); + controller.setVolume(0.0); + controller.removeListener(listener); }); super.deactivate(); diff --git a/packages/video_player/ios/Classes/VideoPlayerPlugin.m b/packages/video_player/ios/Classes/VideoPlayerPlugin.m index 094ce75f67bb..4208f0cd9b75 100644 --- a/packages/video_player/ios/Classes/VideoPlayerPlugin.m +++ b/packages/video_player/ios/Classes/VideoPlayerPlugin.m @@ -493,6 +493,22 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { if ([@"dispose" isEqualToString:call.method]) { [_registry unregisterTexture:textureId]; [_players removeObjectForKey:@(textureId)]; + // If the Flutter contains https://github.com/flutter/engine/pull/12695, + // the `player` is disposed via `onTextureUnregistered` at the right time. + // Without https://github.com/flutter/engine/pull/12695, there is no gurantee that the texture + // has completed unregistration. It may leads a crash if we dispose the `player` before the + // texture is unregistered. We add a dispatch_after hack to make sure the texture is + // unregistered before we dispose the `player`. + // + // TODO(cyanglaz): Remove this dispatch block when + // https://github.com/flutter/flutter/commit/8159a9906095efc9af8b223f5e232cb63542ad0b is in + // stable And update the min flutter version of the plugin to the stable version. + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)), + dispatch_get_main_queue(), ^{ + if (!player.disposed) { + [player dispose]; + } + }); result(nil); } else if ([@"setLooping" isEqualToString:call.method]) { [player setIsLooping:[argsMap[@"looping"] boolValue]]; diff --git a/packages/video_player/pubspec.yaml b/packages/video_player/pubspec.yaml index f85c5f80793a..516b6f778c84 100644 --- a/packages/video_player/pubspec.yaml +++ b/packages/video_player/pubspec.yaml @@ -22,4 +22,4 @@ dev_dependencies: environment: sdk: ">=2.0.0-dev.28.0 <3.0.0" - flutter: ">=1.10.15 <2.0.0" + flutter: ">=1.9.1+hotfix.5 <2.0.0" From 2e427aa0dd3b100414338219921cb18a8a904dd4 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 20 Nov 2019 12:50:45 -0800 Subject: [PATCH 27/28] typo fixes --- packages/video_player/ios/Classes/VideoPlayerPlugin.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/video_player/ios/Classes/VideoPlayerPlugin.m b/packages/video_player/ios/Classes/VideoPlayerPlugin.m index 4208f0cd9b75..8675db6ef27c 100644 --- a/packages/video_player/ios/Classes/VideoPlayerPlugin.m +++ b/packages/video_player/ios/Classes/VideoPlayerPlugin.m @@ -495,8 +495,8 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { [_players removeObjectForKey:@(textureId)]; // If the Flutter contains https://github.com/flutter/engine/pull/12695, // the `player` is disposed via `onTextureUnregistered` at the right time. - // Without https://github.com/flutter/engine/pull/12695, there is no gurantee that the texture - // has completed unregistration. It may leads a crash if we dispose the `player` before the + // Without https://github.com/flutter/engine/pull/12695, there is no guarantee that the texture + // has completed the un-reregistration. It may leads a crash if we dispose the `player` before the // texture is unregistered. We add a dispatch_after hack to make sure the texture is // unregistered before we dispose the `player`. // From a36c6f63136ca9116464b18553b9b48633b9b6dc Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 20 Nov 2019 12:57:45 -0800 Subject: [PATCH 28/28] formatting --- packages/video_player/ios/Classes/VideoPlayerPlugin.m | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/video_player/ios/Classes/VideoPlayerPlugin.m b/packages/video_player/ios/Classes/VideoPlayerPlugin.m index 8675db6ef27c..9bf61d8fed14 100644 --- a/packages/video_player/ios/Classes/VideoPlayerPlugin.m +++ b/packages/video_player/ios/Classes/VideoPlayerPlugin.m @@ -495,10 +495,10 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { [_players removeObjectForKey:@(textureId)]; // If the Flutter contains https://github.com/flutter/engine/pull/12695, // the `player` is disposed via `onTextureUnregistered` at the right time. - // Without https://github.com/flutter/engine/pull/12695, there is no guarantee that the texture - // has completed the un-reregistration. It may leads a crash if we dispose the `player` before the - // texture is unregistered. We add a dispatch_after hack to make sure the texture is - // unregistered before we dispose the `player`. + // Without https://github.com/flutter/engine/pull/12695, there is no guarantee that the + // texture has completed the un-reregistration. It may leads a crash if we dispose the + // `player` before the texture is unregistered. We add a dispatch_after hack to make sure the + // texture is unregistered before we dispose the `player`. // // TODO(cyanglaz): Remove this dispatch block when // https://github.com/flutter/flutter/commit/8159a9906095efc9af8b223f5e232cb63542ad0b is in