From fd4b230b5b29202193f5e0b2ca2efddc8b16e598 Mon Sep 17 00:00:00 2001 From: KyleFin Date: Wed, 17 Mar 2021 22:31:03 -0600 Subject: [PATCH 01/13] Fix https://github.com/flutter/flutter/issues/77674 --- packages/video_player/video_player/lib/video_player.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index 96dd73d3adea..03d726e2d291 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -314,8 +314,7 @@ class VideoPlayerController extends ValueNotifier { _applyPlayPause(); break; case VideoEventType.completed: - value = value.copyWith(isPlaying: false, position: value.duration); - _timer?.cancel(); + pause().then((void pauseResult) => seekTo(value.duration)); break; case VideoEventType.bufferingUpdate: value = value.copyWith(buffered: event.buffered); @@ -375,6 +374,9 @@ class VideoPlayerController extends ValueNotifier { /// has been sent to the platform, not when playback itself is totally /// finished. Future play() async { + if (value.position == value.duration) { + await seekTo(const Duration()); + } value = value.copyWith(isPlaying: true); await _applyPlayPause(); } From 2c88d48a2bf0a73f8d837b9debd514b42eb8c5df Mon Sep 17 00:00:00 2001 From: KyleFin <5882840+KyleFin@users.noreply.github.com> Date: Fri, 26 Mar 2021 10:27:57 -0600 Subject: [PATCH 02/13] Add unit tests. --- .../video_player/test/video_player_test.dart | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/packages/video_player/video_player/test/video_player_test.dart b/packages/video_player/video_player/test/video_player_test.dart index 582012097b71..b6a61ea4dd43 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -115,7 +115,7 @@ void main() { await tester.pumpWidget(VideoPlayer(controller1)); expect( find.byWidgetPredicate( - (Widget widget) => widget is Texture && widget.textureId == 101, + (Widget widget) => widget is Texture && widget.textureId == 101, ), findsOneWidget); @@ -124,7 +124,7 @@ void main() { await tester.pumpWidget(VideoPlayer(controller2)); expect( find.byWidgetPredicate( - (Widget widget) => widget is Texture && widget.textureId == 102, + (Widget widget) => widget is Texture && widget.textureId == 102, ), findsOneWidget); }); @@ -160,18 +160,18 @@ void main() { }); testWidgets('Passes text contrast ratio guidelines', - (WidgetTester tester) async { - final String text = 'foo'; - await tester.pumpWidget(MaterialApp( - home: Scaffold( - backgroundColor: Colors.white, - body: ClosedCaption(text: text), - ), - )); - expect(find.text(text), findsOneWidget); - - await expectLater(tester, meetsGuideline(textContrastGuideline)); - }, skip: isBrowser); + (WidgetTester tester) async { + final String text = 'foo'; + await tester.pumpWidget(MaterialApp( + home: Scaffold( + backgroundColor: Colors.white, + body: ClosedCaption(text: text), + ), + )); + expect(find.text(text), findsOneWidget); + + await expectLater(tester, meetsGuideline(textContrastGuideline)); + }, skip: isBrowser); }); group('VideoPlayerController', () { @@ -235,7 +235,7 @@ void main() { test('file', () async { final VideoPlayerController controller = - VideoPlayerController.file(File('a.avi')); + VideoPlayerController.file(File('a.avi')); await controller.initialize(); expect(fakeVideoPlayerPlatform.dataSourceDescriptions[0].uri, @@ -440,7 +440,7 @@ void main() { expect(controller.value.isBuffering, false); expect(controller.value.buffered, isEmpty); final FakeVideoEventStream fakeVideoEventStream = - fakeVideoPlayerPlatform.streams[controller.textureId]!; + fakeVideoPlayerPlatform.streams[controller.textureId]!; fakeVideoEventStream.eventsChannel .sendEvent({'event': 'bufferingStart'}); @@ -566,17 +566,17 @@ void main() { expect( value.toString(), 'VideoPlayerValue(duration: 0:00:05.000000, ' - 'size: Size(400.0, 300.0), ' - 'position: 0:00:01.000000, ' - 'caption: Caption(number: 0, start: 0:00:00.000000, end: 0:00:00.000000, text: foo), ' - 'buffered: [DurationRange(start: 0:00:00.000000, end: 0:00:04.000000)], ' - 'isInitialized: true, ' - 'isPlaying: true, ' - 'isLooping: true, ' - 'isBuffering: true, ' - 'volume: 0.5, ' - 'playbackSpeed: 1.5, ' - 'errorDescription: null)'); + 'size: Size(400.0, 300.0), ' + 'position: 0:00:01.000000, ' + 'caption: Caption(number: 0, start: 0:00:00.000000, end: 0:00:00.000000, text: foo), ' + 'buffered: [DurationRange(start: 0:00:00.000000, end: 0:00:04.000000)], ' + 'isInitialized: true, ' + 'isPlaying: true, ' + 'isLooping: true, ' + 'isBuffering: true, ' + 'volume: 0.5, ' + 'playbackSpeed: 1.5, ' + 'errorDescription: null)'); }); test('copyWith()', () { From be0bd21be764ef156feccb8becdec17e1fe05957 Mon Sep 17 00:00:00 2001 From: KyleFin <5882840+KyleFin@users.noreply.github.com> Date: Fri, 26 Mar 2021 10:46:12 -0600 Subject: [PATCH 03/13] Add unit tests. --- .../video_player/test/video_player_test.dart | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/video_player/video_player/test/video_player_test.dart b/packages/video_player/video_player/test/video_player_test.dart index 582012097b71..1e087b77a0d2 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -418,6 +418,8 @@ void main() { 'https://127.0.0.1', ); await controller.initialize(); + const Duration nonzeroDuration = Duration(milliseconds: 100); + controller.value = controller.value.copyWith(duration: nonzeroDuration); expect(controller.value.isPlaying, isFalse); await controller.play(); expect(controller.value.isPlaying, isTrue); @@ -429,7 +431,37 @@ void main() { await tester.pumpAndSettle(); expect(controller.value.isPlaying, isFalse); - expect(controller.value.position, controller.value.duration); + expect(controller.value.position, nonzeroDuration); + expect( + fakeVideoPlayerPlatform + .calls[fakeVideoPlayerPlatform.calls.length - 2], + 'pause'); + expect(fakeVideoPlayerPlatform.calls.last, 'seekTo'); + }); + + testWidgets('play after playing completed restarts from beginning', + (WidgetTester tester) async { + final VideoPlayerController controller = VideoPlayerController.network( + 'https://127.0.0.1', + ); + await controller.initialize(); + const Duration nonzeroDuration = Duration(milliseconds: 100); + controller.value = controller.value.copyWith(duration: nonzeroDuration); + await controller.play(); + expect(controller.value.isPlaying, isTrue); + final FakeVideoEventStream fakeVideoEventStream = + fakeVideoPlayerPlatform.streams[controller.textureId]!; + fakeVideoEventStream.eventsChannel + .sendEvent({'event': 'completed'}); + await tester.pumpAndSettle(); + expect(controller.value.isPlaying, isFalse); + expect(controller.value.position, nonzeroDuration); + + await controller.play(); + + expect(controller.value.isPlaying, isTrue); + expect(controller.value.position, Duration.zero); + await controller.pause(); }); testWidgets('buffering status', (WidgetTester tester) async { From deaae24d22f56063f4efd97376631e03a7e6bc07 Mon Sep 17 00:00:00 2001 From: KyleFin <5882840+KyleFin@users.noreply.github.com> Date: Fri, 26 Mar 2021 10:49:13 -0600 Subject: [PATCH 04/13] dartfmt --- .../video_player/test/video_player_test.dart | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/packages/video_player/video_player/test/video_player_test.dart b/packages/video_player/video_player/test/video_player_test.dart index c638608b1dd2..1e087b77a0d2 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -115,7 +115,7 @@ void main() { await tester.pumpWidget(VideoPlayer(controller1)); expect( find.byWidgetPredicate( - (Widget widget) => widget is Texture && widget.textureId == 101, + (Widget widget) => widget is Texture && widget.textureId == 101, ), findsOneWidget); @@ -124,7 +124,7 @@ void main() { await tester.pumpWidget(VideoPlayer(controller2)); expect( find.byWidgetPredicate( - (Widget widget) => widget is Texture && widget.textureId == 102, + (Widget widget) => widget is Texture && widget.textureId == 102, ), findsOneWidget); }); @@ -160,18 +160,18 @@ void main() { }); testWidgets('Passes text contrast ratio guidelines', - (WidgetTester tester) async { - final String text = 'foo'; - await tester.pumpWidget(MaterialApp( - home: Scaffold( - backgroundColor: Colors.white, - body: ClosedCaption(text: text), - ), - )); - expect(find.text(text), findsOneWidget); - - await expectLater(tester, meetsGuideline(textContrastGuideline)); - }, skip: isBrowser); + (WidgetTester tester) async { + final String text = 'foo'; + await tester.pumpWidget(MaterialApp( + home: Scaffold( + backgroundColor: Colors.white, + body: ClosedCaption(text: text), + ), + )); + expect(find.text(text), findsOneWidget); + + await expectLater(tester, meetsGuideline(textContrastGuideline)); + }, skip: isBrowser); }); group('VideoPlayerController', () { @@ -235,7 +235,7 @@ void main() { test('file', () async { final VideoPlayerController controller = - VideoPlayerController.file(File('a.avi')); + VideoPlayerController.file(File('a.avi')); await controller.initialize(); expect(fakeVideoPlayerPlatform.dataSourceDescriptions[0].uri, @@ -472,7 +472,7 @@ void main() { expect(controller.value.isBuffering, false); expect(controller.value.buffered, isEmpty); final FakeVideoEventStream fakeVideoEventStream = - fakeVideoPlayerPlatform.streams[controller.textureId]!; + fakeVideoPlayerPlatform.streams[controller.textureId]!; fakeVideoEventStream.eventsChannel .sendEvent({'event': 'bufferingStart'}); @@ -598,17 +598,17 @@ void main() { expect( value.toString(), 'VideoPlayerValue(duration: 0:00:05.000000, ' - 'size: Size(400.0, 300.0), ' - 'position: 0:00:01.000000, ' - 'caption: Caption(number: 0, start: 0:00:00.000000, end: 0:00:00.000000, text: foo), ' - 'buffered: [DurationRange(start: 0:00:00.000000, end: 0:00:04.000000)], ' - 'isInitialized: true, ' - 'isPlaying: true, ' - 'isLooping: true, ' - 'isBuffering: true, ' - 'volume: 0.5, ' - 'playbackSpeed: 1.5, ' - 'errorDescription: null)'); + 'size: Size(400.0, 300.0), ' + 'position: 0:00:01.000000, ' + 'caption: Caption(number: 0, start: 0:00:00.000000, end: 0:00:00.000000, text: foo), ' + 'buffered: [DurationRange(start: 0:00:00.000000, end: 0:00:04.000000)], ' + 'isInitialized: true, ' + 'isPlaying: true, ' + 'isLooping: true, ' + 'isBuffering: true, ' + 'volume: 0.5, ' + 'playbackSpeed: 1.5, ' + 'errorDescription: null)'); }); test('copyWith()', () { From 60a65957a6ae10491fb1590360cba0e2688bf55d Mon Sep 17 00:00:00 2001 From: KyleFin <5882840+KyleFin@users.noreply.github.com> Date: Fri, 26 Mar 2021 11:27:54 -0600 Subject: [PATCH 05/13] Update play() documentation. --- packages/video_player/video_player/lib/video_player.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index 03d726e2d291..e761e89a8dd7 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -370,6 +370,8 @@ class VideoPlayerController extends ValueNotifier { /// Starts playing the video. /// + /// If the video is at the end, this method starts playing from the beginning. + /// /// This method returns a future that completes as soon as the "play" command /// has been sent to the platform, not when playback itself is totally /// finished. From 46af8d4e8540f990ce3aaebcc56e955c31eca47c Mon Sep 17 00:00:00 2001 From: KyleFin <5882840+kylefin@users.noreply.github.com> Date: Wed, 31 Mar 2021 20:46:35 -0600 Subject: [PATCH 06/13] Update test cases to use integration_tests. --- .../integration_test/video_player_test.dart | 45 ++++++++++++++++++ .../video_player/test/video_player_test.dart | 47 +++++++------------ 2 files changed, 62 insertions(+), 30 deletions(-) diff --git a/packages/video_player/video_player/example/integration_test/video_player_test.dart b/packages/video_player/video_player/example/integration_test/video_player_test.dart index b39bb087a21c..124cde07d639 100644 --- a/packages/video_player/video_player/example/integration_test/video_player_test.dart +++ b/packages/video_player/video_player/example/integration_test/video_player_test.dart @@ -129,6 +129,51 @@ void main() { }, ); + testWidgets( + 'stay paused when seeking after video completed', + (WidgetTester tester) async { + await _controller.initialize(); + // Mute to allow playing without DOM interaction on Web. + // See https://developers.google.com/web/updates/2017/09/autoplay-policy-changes + await _controller.setVolume(0); + Duration tenMillisBeforeEnd = + _controller.value.duration - const Duration(milliseconds: 10); + await _controller.seekTo(tenMillisBeforeEnd); + await _controller.play(); + await tester.pumpAndSettle(_playDuration); + expect(_controller.value.isPlaying, false); + expect(_controller.value.position, _controller.value.duration); + + await _controller.seekTo(tenMillisBeforeEnd); + await tester.pumpAndSettle(_playDuration); + + expect(_controller.value.isPlaying, false); + expect(_controller.value.position, tenMillisBeforeEnd); + }, + ); + + testWidgets( + 'do not exceed duration on play after video completed', + (WidgetTester tester) async { + await _controller.initialize(); + // Mute to allow playing without DOM interaction on Web. + // See https://developers.google.com/web/updates/2017/09/autoplay-policy-changes + await _controller.setVolume(0); + await _controller.seekTo( + _controller.value.duration - const Duration(milliseconds: 10)); + await _controller.play(); + await tester.pumpAndSettle(_playDuration); + expect(_controller.value.isPlaying, false); + expect(_controller.value.position, _controller.value.duration); + + await _controller.play(); + await tester.pumpAndSettle(_playDuration); + + expect(_controller.value.position, + lessThanOrEqualTo(_controller.value.duration)); + }, + ); + testWidgets('test video player view with local asset', (WidgetTester tester) async { Future started() async { diff --git a/packages/video_player/video_player/test/video_player_test.dart b/packages/video_player/video_player/test/video_player_test.dart index 1e087b77a0d2..3fa89ec3776c 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -277,6 +277,23 @@ void main() { expect(fakeVideoPlayerPlatform.calls.last, 'setPlaybackSpeed'); }); + test('play restarts from beginning if video is at end', () async { + final VideoPlayerController controller = VideoPlayerController.network( + 'https://127.0.0.1', + ); + await controller.initialize(); + const Duration nonzeroDuration = Duration(milliseconds: 100); + controller.value = controller.value.copyWith(duration: nonzeroDuration); + await controller.seekTo(nonzeroDuration); + expect(controller.value.isPlaying, isFalse); + expect(controller.value.position, nonzeroDuration); + + await controller.play(); + + expect(controller.value.isPlaying, isTrue); + expect(controller.value.position, Duration.zero); + }); + test('setLooping', () async { final VideoPlayerController controller = VideoPlayerController.network( 'https://127.0.0.1', @@ -432,36 +449,6 @@ void main() { expect(controller.value.isPlaying, isFalse); expect(controller.value.position, nonzeroDuration); - expect( - fakeVideoPlayerPlatform - .calls[fakeVideoPlayerPlatform.calls.length - 2], - 'pause'); - expect(fakeVideoPlayerPlatform.calls.last, 'seekTo'); - }); - - testWidgets('play after playing completed restarts from beginning', - (WidgetTester tester) async { - final VideoPlayerController controller = VideoPlayerController.network( - 'https://127.0.0.1', - ); - await controller.initialize(); - const Duration nonzeroDuration = Duration(milliseconds: 100); - controller.value = controller.value.copyWith(duration: nonzeroDuration); - await controller.play(); - expect(controller.value.isPlaying, isTrue); - final FakeVideoEventStream fakeVideoEventStream = - fakeVideoPlayerPlatform.streams[controller.textureId]!; - fakeVideoEventStream.eventsChannel - .sendEvent({'event': 'completed'}); - await tester.pumpAndSettle(); - expect(controller.value.isPlaying, isFalse); - expect(controller.value.position, nonzeroDuration); - - await controller.play(); - - expect(controller.value.isPlaying, isTrue); - expect(controller.value.position, Duration.zero); - await controller.pause(); }); testWidgets('buffering status', (WidgetTester tester) async { From 3c8d958ed653fdad2c6b709b42869ebab6ec15ac Mon Sep 17 00:00:00 2001 From: KyleFin <5882840+KyleFin@users.noreply.github.com> Date: Wed, 21 Apr 2021 23:21:31 -0600 Subject: [PATCH 07/13] Restoring `main` fork branch to fetch from upstream (will create a new video_player_pause_on_complete remote branch in my fork). --- .../integration_test/video_player_test.dart | 45 ------------------- .../video_player/test/video_player_test.dart | 21 +-------- 2 files changed, 1 insertion(+), 65 deletions(-) diff --git a/packages/video_player/video_player/example/integration_test/video_player_test.dart b/packages/video_player/video_player/example/integration_test/video_player_test.dart index 124cde07d639..b39bb087a21c 100644 --- a/packages/video_player/video_player/example/integration_test/video_player_test.dart +++ b/packages/video_player/video_player/example/integration_test/video_player_test.dart @@ -129,51 +129,6 @@ void main() { }, ); - testWidgets( - 'stay paused when seeking after video completed', - (WidgetTester tester) async { - await _controller.initialize(); - // Mute to allow playing without DOM interaction on Web. - // See https://developers.google.com/web/updates/2017/09/autoplay-policy-changes - await _controller.setVolume(0); - Duration tenMillisBeforeEnd = - _controller.value.duration - const Duration(milliseconds: 10); - await _controller.seekTo(tenMillisBeforeEnd); - await _controller.play(); - await tester.pumpAndSettle(_playDuration); - expect(_controller.value.isPlaying, false); - expect(_controller.value.position, _controller.value.duration); - - await _controller.seekTo(tenMillisBeforeEnd); - await tester.pumpAndSettle(_playDuration); - - expect(_controller.value.isPlaying, false); - expect(_controller.value.position, tenMillisBeforeEnd); - }, - ); - - testWidgets( - 'do not exceed duration on play after video completed', - (WidgetTester tester) async { - await _controller.initialize(); - // Mute to allow playing without DOM interaction on Web. - // See https://developers.google.com/web/updates/2017/09/autoplay-policy-changes - await _controller.setVolume(0); - await _controller.seekTo( - _controller.value.duration - const Duration(milliseconds: 10)); - await _controller.play(); - await tester.pumpAndSettle(_playDuration); - expect(_controller.value.isPlaying, false); - expect(_controller.value.position, _controller.value.duration); - - await _controller.play(); - await tester.pumpAndSettle(_playDuration); - - expect(_controller.value.position, - lessThanOrEqualTo(_controller.value.duration)); - }, - ); - testWidgets('test video player view with local asset', (WidgetTester tester) async { Future started() async { diff --git a/packages/video_player/video_player/test/video_player_test.dart b/packages/video_player/video_player/test/video_player_test.dart index 3fa89ec3776c..582012097b71 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -277,23 +277,6 @@ void main() { expect(fakeVideoPlayerPlatform.calls.last, 'setPlaybackSpeed'); }); - test('play restarts from beginning if video is at end', () async { - final VideoPlayerController controller = VideoPlayerController.network( - 'https://127.0.0.1', - ); - await controller.initialize(); - const Duration nonzeroDuration = Duration(milliseconds: 100); - controller.value = controller.value.copyWith(duration: nonzeroDuration); - await controller.seekTo(nonzeroDuration); - expect(controller.value.isPlaying, isFalse); - expect(controller.value.position, nonzeroDuration); - - await controller.play(); - - expect(controller.value.isPlaying, isTrue); - expect(controller.value.position, Duration.zero); - }); - test('setLooping', () async { final VideoPlayerController controller = VideoPlayerController.network( 'https://127.0.0.1', @@ -435,8 +418,6 @@ void main() { 'https://127.0.0.1', ); await controller.initialize(); - const Duration nonzeroDuration = Duration(milliseconds: 100); - controller.value = controller.value.copyWith(duration: nonzeroDuration); expect(controller.value.isPlaying, isFalse); await controller.play(); expect(controller.value.isPlaying, isTrue); @@ -448,7 +429,7 @@ void main() { await tester.pumpAndSettle(); expect(controller.value.isPlaying, isFalse); - expect(controller.value.position, nonzeroDuration); + expect(controller.value.position, controller.value.duration); }); testWidgets('buffering status', (WidgetTester tester) async { From 5a6656729b1b02893d07cfb94858c8724cb52c3c Mon Sep 17 00:00:00 2001 From: KyleFin <5882840+KyleFin@users.noreply.github.com> Date: Wed, 21 Apr 2021 23:21:38 -0600 Subject: [PATCH 08/13] Restoring `main` fork branch to fetch from upstream (will create a new video_player_pause_on_complete remote branch in my fork). --- packages/video_player/video_player/lib/video_player.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index e761e89a8dd7..03d726e2d291 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -370,8 +370,6 @@ class VideoPlayerController extends ValueNotifier { /// Starts playing the video. /// - /// If the video is at the end, this method starts playing from the beginning. - /// /// This method returns a future that completes as soon as the "play" command /// has been sent to the platform, not when playback itself is totally /// finished. From f6338a6a10a2ea55553feb207f1391f94de7945e Mon Sep 17 00:00:00 2001 From: KyleFin <5882840+KyleFin@users.noreply.github.com> Date: Wed, 21 Apr 2021 23:21:46 -0600 Subject: [PATCH 09/13] Restoring `main` fork branch to fetch from upstream (will create a new video_player_pause_on_complete remote branch in my fork). --- packages/video_player/video_player/lib/video_player.dart | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index 03d726e2d291..96dd73d3adea 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -314,7 +314,8 @@ class VideoPlayerController extends ValueNotifier { _applyPlayPause(); break; case VideoEventType.completed: - pause().then((void pauseResult) => seekTo(value.duration)); + value = value.copyWith(isPlaying: false, position: value.duration); + _timer?.cancel(); break; case VideoEventType.bufferingUpdate: value = value.copyWith(buffered: event.buffered); @@ -374,9 +375,6 @@ class VideoPlayerController extends ValueNotifier { /// has been sent to the platform, not when playback itself is totally /// finished. Future play() async { - if (value.position == value.duration) { - await seekTo(const Duration()); - } value = value.copyWith(isPlaying: true); await _applyPlayPause(); } From ace08de3eabbfb0548089c86a44eff333fedf780 Mon Sep 17 00:00:00 2001 From: KyleFin <5882840+KyleFin@users.noreply.github.com> Date: Wed, 1 Sep 2021 20:30:53 -0600 Subject: [PATCH 10/13] Ensure seekTo isn't called before video player is initialized. Fixes [#89259](https://github.com/flutter/flutter/issues/89259). --- .../video_player/video_player/CHANGELOG.md | 3 ++- .../video_player/lib/video_player.dart | 16 ++++++++----- .../video_player/video_player/pubspec.yaml | 2 +- .../video_player/test/video_player_test.dart | 24 ++++++++++++++++++- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md index f07bb5f66f8c..9cb642a4db56 100644 --- a/packages/video_player/video_player/CHANGELOG.md +++ b/packages/video_player/video_player/CHANGELOG.md @@ -1,5 +1,6 @@ -## NEXT +## 2.1.15 +* Ensured seekTo isn't called before video player is initialized. Fixes [#89259](https://github.com/flutter/flutter/issues/89259). * Updated Android lint settings. ## 2.1.14 diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index 772409258ac4..3fc84505e169 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -415,14 +415,14 @@ class VideoPlayerController extends ValueNotifier { } Future _applyLooping() async { - if (!value.isInitialized || _isDisposed) { + if (_isDisposedOrNotInitialized()) { return; } await _videoPlayerPlatform.setLooping(_textureId, value.isLooping); } Future _applyPlayPause() async { - if (!value.isInitialized || _isDisposed) { + if (_isDisposedOrNotInitialized()) { return; } if (value.isPlaying) { @@ -455,14 +455,14 @@ class VideoPlayerController extends ValueNotifier { } Future _applyVolume() async { - if (!value.isInitialized || _isDisposed) { + if (_isDisposedOrNotInitialized()) { return; } await _videoPlayerPlatform.setVolume(_textureId, value.volume); } Future _applyPlaybackSpeed() async { - if (!value.isInitialized || _isDisposed) { + if (_isDisposedOrNotInitialized()) { return; } @@ -491,7 +491,7 @@ class VideoPlayerController extends ValueNotifier { /// If [moment] is outside of the video's full range it will be automatically /// and silently clamped. Future seekTo(Duration position) async { - if (_isDisposed) { + if (_isDisposedOrNotInitialized()) { return; } if (position > value.duration) { @@ -572,6 +572,10 @@ class VideoPlayerController extends ValueNotifier { value = value.copyWith(position: position); value = value.copyWith(caption: _getCaptionAt(position)); } + + bool _isDisposedOrNotInitialized() { + return _isDisposed || !value.isInitialized; + } } class _VideoAppLifeCycleObserver extends Object with WidgetsBindingObserver { @@ -963,4 +967,4 @@ class ClosedCaption extends StatelessWidget { /// We use this so that APIs that have become non-nullable can still be used /// with `!` and `?` on the stable branch. // TODO(ianh): Remove this once we roll stable in late 2021. -T? _ambiguate(T? value) => value; +T? _ambiguate(T? value) => value; \ No newline at end of file diff --git a/packages/video_player/video_player/pubspec.yaml b/packages/video_player/video_player/pubspec.yaml index 0d0cdb1cb436..7f6f608687cc 100644 --- a/packages/video_player/video_player/pubspec.yaml +++ b/packages/video_player/video_player/pubspec.yaml @@ -3,7 +3,7 @@ description: Flutter plugin for displaying inline video with other Flutter widgets on Android, iOS, and web. repository: https://github.com/flutter/plugins/tree/master/packages/video_player/video_player issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22 -version: 2.1.14 +version: 2.1.15 environment: sdk: ">=2.12.0 <3.0.0" diff --git a/packages/video_player/video_player/test/video_player_test.dart b/packages/video_player/video_player/test/video_player_test.dart index b5bfad605620..6ae8e24229f9 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -318,6 +318,17 @@ void main() { expect(fakeVideoPlayerPlatform.calls.last, 'setPlaybackSpeed'); }); + test('play before initialized does not call platform', () async { + final VideoPlayerController controller = VideoPlayerController.network( + 'https://127.0.0.1', + ); + expect(controller.value.isInitialized, isFalse); + + await controller.play(); + + expect(fakeVideoPlayerPlatform.calls, isEmpty); + }); + test('play restarts from beginning if video is at end', () async { final VideoPlayerController controller = VideoPlayerController.network( 'https://127.0.0.1', @@ -373,6 +384,17 @@ void main() { expect(await controller.position, const Duration(milliseconds: 500)); }); + test('before initialized does not call platform', () async { + final VideoPlayerController controller = VideoPlayerController.network( + 'https://127.0.0.1', + ); + expect(controller.value.isInitialized, isFalse); + + await controller.seekTo(const Duration(milliseconds: 500)); + + expect(fakeVideoPlayerPlatform.calls, isEmpty); + }); + test('clamps values that are too high or low', () async { final VideoPlayerController controller = VideoPlayerController.network( 'https://127.0.0.1', @@ -868,4 +890,4 @@ class FakeEventsChannel { /// We use this so that APIs that have become non-nullable can still be used /// with `!` and `?` on the stable branch. // TODO(ianh): Remove this once we roll stable in late 2021. -T? _ambiguate(T? value) => value; +T? _ambiguate(T? value) => value; \ No newline at end of file From bfbea434f2f2f05f4c2545872ca40dd6531425cb Mon Sep 17 00:00:00 2001 From: KyleFin <5882840+KyleFin@users.noreply.github.com> Date: Wed, 1 Sep 2021 20:36:44 -0600 Subject: [PATCH 11/13] Restore newlines at end of file. --- packages/video_player/video_player/lib/video_player.dart | 2 +- packages/video_player/video_player/test/video_player_test.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index 3fc84505e169..cac3e3471406 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -967,4 +967,4 @@ class ClosedCaption extends StatelessWidget { /// We use this so that APIs that have become non-nullable can still be used /// with `!` and `?` on the stable branch. // TODO(ianh): Remove this once we roll stable in late 2021. -T? _ambiguate(T? value) => value; \ No newline at end of file +T? _ambiguate(T? value) => value; diff --git a/packages/video_player/video_player/test/video_player_test.dart b/packages/video_player/video_player/test/video_player_test.dart index 6ae8e24229f9..ad536f840c1d 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -890,4 +890,4 @@ class FakeEventsChannel { /// We use this so that APIs that have become non-nullable can still be used /// with `!` and `?` on the stable branch. // TODO(ianh): Remove this once we roll stable in late 2021. -T? _ambiguate(T? value) => value; \ No newline at end of file +T? _ambiguate(T? value) => value; From 82f951b1f26e28b347b04cbc60dcf5a3ab7c8bf6 Mon Sep 17 00:00:00 2001 From: KyleFin <5882840+KyleFin@users.noreply.github.com> Date: Thu, 2 Sep 2021 09:42:51 -0600 Subject: [PATCH 12/13] Use getter for _isDisposedOrNotInitialized. --- .../video_player/lib/video_player.dart | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index cac3e3471406..b4c4b2b2a311 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -415,14 +415,14 @@ class VideoPlayerController extends ValueNotifier { } Future _applyLooping() async { - if (_isDisposedOrNotInitialized()) { + if (_isDisposedOrNotInitialized) { return; } await _videoPlayerPlatform.setLooping(_textureId, value.isLooping); } Future _applyPlayPause() async { - if (_isDisposedOrNotInitialized()) { + if (_isDisposedOrNotInitialized) { return; } if (value.isPlaying) { @@ -455,14 +455,14 @@ class VideoPlayerController extends ValueNotifier { } Future _applyVolume() async { - if (_isDisposedOrNotInitialized()) { + if (_isDisposedOrNotInitialized) { return; } await _videoPlayerPlatform.setVolume(_textureId, value.volume); } Future _applyPlaybackSpeed() async { - if (_isDisposedOrNotInitialized()) { + if (_isDisposedOrNotInitialized) { return; } @@ -491,7 +491,7 @@ class VideoPlayerController extends ValueNotifier { /// If [moment] is outside of the video's full range it will be automatically /// and silently clamped. Future seekTo(Duration position) async { - if (_isDisposedOrNotInitialized()) { + if (_isDisposedOrNotInitialized) { return; } if (position > value.duration) { @@ -573,9 +573,7 @@ class VideoPlayerController extends ValueNotifier { value = value.copyWith(caption: _getCaptionAt(position)); } - bool _isDisposedOrNotInitialized() { - return _isDisposed || !value.isInitialized; - } + bool get _isDisposedOrNotInitialized => _isDisposed || !value.isInitialized; } class _VideoAppLifeCycleObserver extends Object with WidgetsBindingObserver { From a85601f18ce5db0eb02b1c5a234592d61c7607d3 Mon Sep 17 00:00:00 2001 From: KyleFin <5882840+KyleFin@users.noreply.github.com> Date: Mon, 20 Dec 2021 12:47:04 -0700 Subject: [PATCH 13/13] Platform interfaces changes to fix Android rotation https://github.com/flutter/flutter/issues/60327 --- .../CHANGELOG.md | 10 ++++++- .../lib/method_channel_video_player.dart | 4 +++ .../lib/video_player_platform_interface.dart | 15 +++++++++-- .../pubspec.yaml | 2 +- .../method_channel_video_player_test.dart | 27 +++++++++++++++++-- 5 files changed, 52 insertions(+), 6 deletions(-) diff --git a/packages/video_player/video_player_platform_interface/CHANGELOG.md b/packages/video_player/video_player_platform_interface/CHANGELOG.md index 24631513f800..c42036511d96 100644 --- a/packages/video_player/video_player_platform_interface/CHANGELOG.md +++ b/packages/video_player/video_player_platform_interface/CHANGELOG.md @@ -1,3 +1,11 @@ +## 4.2.1 + +* Added rotation on Android for videos recorded in landscapeRight Fixes [#60327](https://github.com/flutter/flutter/issues/60327). + +## 4.2.0 + +* Add `contentUri` to `DataSourceType`. + ## 4.1.0 * Add `httpHeaders` to `DataSource` @@ -29,7 +37,7 @@ ## 2.1.0 -* Add VideoPlayerOptions with audo mix mode +* Add VideoPlayerOptions with audio mix mode ## 2.0.2 diff --git a/packages/video_player/video_player_platform_interface/lib/method_channel_video_player.dart b/packages/video_player/video_player_platform_interface/lib/method_channel_video_player.dart index e92e87013d68..f87ed92b3aa5 100644 --- a/packages/video_player/video_player_platform_interface/lib/method_channel_video_player.dart +++ b/packages/video_player/video_player_platform_interface/lib/method_channel_video_player.dart @@ -42,6 +42,9 @@ class MethodChannelVideoPlayer extends VideoPlayerPlatform { case DataSourceType.file: message.uri = dataSource.uri; break; + case DataSourceType.contentUri: + message.uri = dataSource.uri; + break; } TextureMessage response = await _api.create(message); @@ -108,6 +111,7 @@ class MethodChannelVideoPlayer extends VideoPlayerPlatform { duration: Duration(milliseconds: map['duration']), size: Size(map['width']?.toDouble() ?? 0.0, map['height']?.toDouble() ?? 0.0), + rotationCorrection: map['rotationCorrection']?.toDouble() ?? 0.0, ); case 'completed': return VideoEvent( diff --git a/packages/video_player/video_player_platform_interface/lib/video_player_platform_interface.dart b/packages/video_player/video_player_platform_interface/lib/video_player_platform_interface.dart index b2bff990401e..ae3c0709f7bf 100644 --- a/packages/video_player/video_player_platform_interface/lib/video_player_platform_interface.dart +++ b/packages/video_player/video_player_platform_interface/lib/video_player_platform_interface.dart @@ -196,6 +196,9 @@ enum DataSourceType { /// The video was loaded off of the local filesystem. file, + + /// The video is available via contentUri. Android only. + contentUri, } /// The file format of the given video. @@ -219,12 +222,13 @@ class VideoEvent { /// /// The [eventType] argument is required. /// - /// Depending on the [eventType], the [duration], [size] and [buffered] - /// arguments can be null. + /// Depending on the [eventType], the [duration], [size], + /// [rotationCorrection], and [buffered] arguments can be null. VideoEvent({ required this.eventType, this.duration, this.size, + this.rotationCorrection, this.buffered, }); @@ -241,6 +245,11 @@ class VideoEvent { /// Only used if [eventType] is [VideoEventType.initialized]. final Size? size; + /// Radians to rotate the video so it is displayed correctly. + /// + /// Only used if [eventType] is [VideoEventType.initialized]. + final double? rotationCorrection; + /// Buffered parts of the video. /// /// Only used if [eventType] is [VideoEventType.bufferingUpdate]. @@ -254,6 +263,7 @@ class VideoEvent { eventType == other.eventType && duration == other.duration && size == other.size && + rotationCorrection == other.rotationCorrection && listEquals(buffered, other.buffered); } @@ -262,6 +272,7 @@ class VideoEvent { eventType.hashCode ^ duration.hashCode ^ size.hashCode ^ + rotationCorrection.hashCode ^ buffered.hashCode; } diff --git a/packages/video_player/video_player_platform_interface/pubspec.yaml b/packages/video_player/video_player_platform_interface/pubspec.yaml index 2a0ef10a9d2b..79167bf7b233 100644 --- a/packages/video_player/video_player_platform_interface/pubspec.yaml +++ b/packages/video_player/video_player_platform_interface/pubspec.yaml @@ -4,7 +4,7 @@ repository: https://github.com/flutter/plugins/tree/master/packages/video_player issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22 # NOTE: We strongly prefer non-breaking changes, even at the expense of a # less-clean API. See https://flutter.dev/go/platform-interface-breaking-changes -version: 4.1.0 +version: 4.2.1 environment: sdk: ">=2.12.0 <3.0.0" diff --git a/packages/video_player/video_player_platform_interface/test/method_channel_video_player_test.dart b/packages/video_player/video_player_platform_interface/test/method_channel_video_player_test.dart index 9da71617e66a..04a67591c534 100644 --- a/packages/video_player/video_player_platform_interface/test/method_channel_video_player_test.dart +++ b/packages/video_player/video_player_platform_interface/test/method_channel_video_player_test.dart @@ -92,10 +92,12 @@ class _ApiLogger implements TestHostVideoPlayerApi { void main() { TestWidgetsFlutterBinding.ensureInitialized(); + // Store the initial instance before any tests change it. + final VideoPlayerPlatform initialInstance = VideoPlayerPlatform.instance; + group('$VideoPlayerPlatform', () { test('$MethodChannelVideoPlayer() is the default instance', () { - expect(VideoPlayerPlatform.instance, - isInstanceOf()); + expect(initialInstance, isInstanceOf()); }); }); @@ -253,6 +255,20 @@ void main() { }), (ByteData? data) {}); + await _ambiguate(ServicesBinding.instance) + ?.defaultBinaryMessenger + .handlePlatformMessage( + "flutter.io/videoPlayer/videoEvents123", + const StandardMethodCodec() + .encodeSuccessEnvelope({ + 'event': 'initialized', + 'duration': 98765, + 'width': 1920, + 'height': 1080, + 'rotationCorrection': 3.14, + }), + (ByteData? data) {}); + await _ambiguate(ServicesBinding.instance) ?.defaultBinaryMessenger .handlePlatformMessage( @@ -312,6 +328,13 @@ void main() { eventType: VideoEventType.initialized, duration: const Duration(milliseconds: 98765), size: const Size(1920, 1080), + rotationCorrection: 0.0, + ), + VideoEvent( + eventType: VideoEventType.initialized, + duration: const Duration(milliseconds: 98765), + size: const Size(1920, 1080), + rotationCorrection: 3.14, ), VideoEvent(eventType: VideoEventType.completed), VideoEvent(