diff --git a/packages/video_player/CHANGELOG.md b/packages/video_player/CHANGELOG.md index 759c955e39df..fa84c90f5a61 100644 --- a/packages/video_player/CHANGELOG.md +++ b/packages/video_player/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.10.3+1 + +* Dispose `FLTVideoPlayer` in `onTextureUnregistered` callback 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.3 * Add support for the v2 Android embedding. This shouldn't impact existing diff --git a/packages/video_player/example/lib/main.dart b/packages/video_player/example/lib/main.dart index 196b90846e16..43d9355c4345 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); - controller.removeListener(listener); + SchedulerBinding.instance.addPostFrameCallback((_) { + controller.setVolume(0.0); + controller.removeListener(listener); + }); + super.deactivate(); } @@ -360,73 +364,152 @@ 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( + key: const ValueKey('home_page'), + 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 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), ), - 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', + ), + ], + ), + ), + 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; + 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); + } + }); + } + + @override + void dispose() { + _videoPlayerController.dispose(); + super.dispose(); + } + + Future started() async { + await _videoPlayerController.initialize(); + await _videoPlayerController.play(); + startedPlaying = true; + return true; + } + + @override + Widget build(BuildContext context) { + return Material( + elevation: 0, + child: Center( + 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'); + } + }, + ), + ), + ); + } +} diff --git a/packages/video_player/example/pubspec.yaml b/packages/video_player/example/pubspec.yaml index bcb559b35bb5..b83e8d177646 100644 --- a/packages/video_player/example/pubspec.yaml +++ b/packages/video_player/example/pubspec.yaml @@ -4,6 +4,8 @@ description: Demonstrates how to use the video_player plugin. dependencies: flutter: sdk: flutter + video_player: + path: ../ dev_dependencies: flutter_test: @@ -11,8 +13,7 @@ dev_dependencies: flutter_driver: sdk: flutter e2e: "^0.2.0" - video_player: - path: ../ + 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 index e726ab2f410d..bf35cf50b728 100644 --- a/packages/video_player/example/test_driver/video_player_e2e.dart +++ b/packages/video_player/example/test_driver/video_player_e2e.dart @@ -12,7 +12,6 @@ const Duration _playDuration = Duration(seconds: 1); void main() { E2EWidgetsFlutterBinding.ensureInitialized(); VideoPlayerController _controller; - tearDown(() async => _controller.dispose()); group('asset videos', () { 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 ccd716607d60..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,7 +4,6 @@ import 'dart:async'; import 'dart:io'; - import 'package:flutter_driver/flutter_driver.dart'; Future main() async { 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..a72c6944cfbe --- /dev/null +++ b/packages/video_player/example/test_driver/video_player_test.dart @@ -0,0 +1,27 @@ +// 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); + }, 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 bf449ec0e8e2..9bf61d8fed14 100644 --- a/packages/video_player/ios/Classes/VideoPlayerPlugin.m +++ b/packages/video_player/ios/Classes/VideoPlayerPlugin.m @@ -363,6 +363,12 @@ - (CVPixelBufferRef)copyPixelBuffer { } } +- (void)onTextureUnregistered { + dispatch_async(dispatch_get_main_queue(), ^{ + [self dispose]; + }); +} + - (FlutterError* _Nullable)onCancelWithArguments:(id _Nullable)arguments { _eventSink = nil; return nil; @@ -487,7 +493,22 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { if ([@"dispose" isEqualToString:call.method]) { [_registry unregisterTexture:textureId]; [_players removeObjectForKey:@(textureId)]; - [player dispose]; + // 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`. + // + // 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 656a81fa8643..516b6f778c84 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.3 +version: 0.10.3+1 homepage: https://github.com/flutter/plugins/tree/master/packages/video_player flutter: