From 0c3ff8a4efdcdfe21c67a89501021e73e016de1a Mon Sep 17 00:00:00 2001 From: Harry Cheng Date: Thu, 27 Feb 2020 15:31:41 +0800 Subject: [PATCH 01/17] Fix buffering state on Android --- .../java/io/flutter/plugins/videoplayer/VideoPlayer.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java b/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java index 43123ef09238..ea98c2bbf8cd 100644 --- a/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java +++ b/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java @@ -174,8 +174,15 @@ public void onCancel(Object o) { public void onPlayerStateChanged(final boolean playWhenReady, final int playbackState) { if (playbackState == Player.STATE_BUFFERING) { sendBufferingUpdate(); + Map event = new HashMap<>(); + event.put("event", "bufferingStart"); + eventSink.success(event); } else if (playbackState == Player.STATE_READY) { - if (!isInitialized) { + if (isInitialized) { + Map event = new HashMap<>(); + event.put("event", "bufferingEnd"); + eventSink.success(event); + } else { isInitialized = true; sendInitialized(); } From 8174d495468776701b4284d2c7b20b0484cb12c0 Mon Sep 17 00:00:00 2001 From: Harry Cheng Date: Thu, 27 Feb 2020 15:45:27 +0800 Subject: [PATCH 02/17] Update version of video_player --- packages/video_player/video_player/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/video_player/video_player/pubspec.yaml b/packages/video_player/video_player/pubspec.yaml index 178789d35fae..30d1338023dd 100644 --- a/packages/video_player/video_player/pubspec.yaml +++ b/packages/video_player/video_player/pubspec.yaml @@ -1,7 +1,7 @@ name: video_player description: Flutter plugin for displaying inline video with other Flutter widgets on Android and iOS. -version: 0.10.8+1 +version: 0.10.8+2 homepage: https://github.com/flutter/plugins/tree/master/packages/video_player/video_player flutter: From 5f297a5018da116f28035c63986f0bc59678864b Mon Sep 17 00:00:00 2001 From: Harry Cheng Date: Thu, 27 Feb 2020 15:56:00 +0800 Subject: [PATCH 03/17] Update CHANGELOG.md --- packages/video_player/video_player/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md index 45bdf86088ab..02bda5ab65b8 100644 --- a/packages/video_player/video_player/CHANGELOG.md +++ b/packages/video_player/video_player/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.10.8+2 + +* Fix issue where `isBuffering` is not updating on Android. + ## 0.10.8+1 * Make the pedantic dev_dependency explicit. From c19d5e3de28b55fbfc70fbfeb30e51be1f0cc772 Mon Sep 17 00:00:00 2001 From: Salakar Date: Mon, 7 Dec 2020 11:25:39 +0000 Subject: [PATCH 04/17] refactor: update changelog & pubspec version to latest version --- packages/video_player/video_player/CHANGELOG.md | 5 ++++- packages/video_player/video_player/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md index 8f7f7516ae3a..87f3db54392f 100644 --- a/packages/video_player/video_player/CHANGELOG.md +++ b/packages/video_player/video_player/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.0.2 + +* Fix issue where `isBuffering` is not updating on Android. + ## 1.0.1 * Android: Dispose video players when app is closed. @@ -66,7 +70,6 @@ ## 0.10.11+2 -* Fix issue where `isBuffering` is not updating on Android. * Fix aspectRatio calculation when size.width or size.height are zero. ## 0.10.11+1 diff --git a/packages/video_player/video_player/pubspec.yaml b/packages/video_player/video_player/pubspec.yaml index 69be8b24100b..528a23bd49e2 100644 --- a/packages/video_player/video_player/pubspec.yaml +++ b/packages/video_player/video_player/pubspec.yaml @@ -1,7 +1,7 @@ name: video_player description: Flutter plugin for displaying inline video with other Flutter widgets on Android, iOS, and web. -version: 1.0.1 +version: 1.0.2 homepage: https://github.com/flutter/plugins/tree/master/packages/video_player/video_player flutter: From ef78567a895d209fd81e5df5878ba1e1bb90c34a Mon Sep 17 00:00:00 2001 From: Salakar Date: Mon, 7 Dec 2020 15:49:09 +0000 Subject: [PATCH 05/17] fix: android not correctly sending `bufferingEnd` event --- .../plugins/videoplayer/VideoPlayer.java | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java b/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java index ab0c43d78ffa..9fbdf45516c9 100644 --- a/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java +++ b/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java @@ -54,6 +54,8 @@ final class VideoPlayer { private boolean isInitialized = false; + private boolean isBuffering = false; + private final VideoPlayerOptions options; VideoPlayer( @@ -173,16 +175,15 @@ public void onCancel(Object o) { @Override public void onPlaybackStateChanged(final int playbackState) { if (playbackState == Player.STATE_BUFFERING) { - sendBufferingUpdate(); - Map event = new HashMap<>(); - event.put("event", "bufferingStart"); - eventSink.success(event); - } else if (playbackState == Player.STATE_READY) { - if (isInitialized) { + if (!isBuffering) { + isBuffering = true; Map event = new HashMap<>(); - event.put("event", "bufferingEnd"); + event.put("event", "bufferingStart"); eventSink.success(event); - } else { + } + sendBufferingUpdate(); + } else if (playbackState == Player.STATE_READY) { + if (!isInitialized) { isInitialized = true; sendInitialized(); } @@ -191,10 +192,18 @@ public void onPlaybackStateChanged(final int playbackState) { event.put("event", "completed"); eventSink.success(event); } + + if (isBuffering && playbackState != Player.STATE_BUFFERING) { + isBuffering = false; + Map event = new HashMap<>(); + event.put("event", "bufferingEnd"); + eventSink.success(event); + } } @Override public void onPlayerError(final ExoPlaybackException error) { + isBuffering = false; if (eventSink != null) { eventSink.error("VideoError", "Video player had error " + error, null); } From b3d734b9f1dc1695ee4f268f0f6731ce4daf30f6 Mon Sep 17 00:00:00 2001 From: Salakar Date: Mon, 7 Dec 2020 15:50:27 +0000 Subject: [PATCH 06/17] refactor: android sending unnecessary `bufferingUpdate` event. This is already handled by the listener event `Player.STATE_BUFFERING` --- .../java/io/flutter/plugins/videoplayer/VideoPlayerPlugin.java | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayerPlugin.java b/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayerPlugin.java index 0c5854f4bc83..665bbc445646 100644 --- a/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayerPlugin.java +++ b/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayerPlugin.java @@ -186,7 +186,6 @@ public PositionMessage position(TextureMessage arg) { VideoPlayer player = videoPlayers.get(arg.getTextureId()); PositionMessage result = new PositionMessage(); result.setPosition(player.getPosition()); - player.sendBufferingUpdate(); return result; } From 7efdab7fbbacc3246e7da12f8c2d8b7b848244d4 Mon Sep 17 00:00:00 2001 From: Salakar Date: Mon, 7 Dec 2020 15:51:16 +0000 Subject: [PATCH 07/17] refactor: make `videoPlayerPlatform` visible for testing --- .../video_player/lib/video_player.dart | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index ac1645085e36..9d50638f6c2e 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -17,7 +17,10 @@ export 'package:video_player_platform_interface/video_player_platform_interface. import 'src/closed_caption_file.dart'; export 'src/closed_caption_file.dart'; -final VideoPlayerPlatform _videoPlayerPlatform = VideoPlayerPlatform.instance +/// This is just exposed for testing. It shouldn't be used by anyone depending +/// on the plugin. +@visibleForTesting +final VideoPlayerPlatform videoPlayerPlatform = VideoPlayerPlatform.instance // This will clear all open videos on the platform when a full restart is // performed. ..init(); @@ -275,11 +278,11 @@ class VideoPlayerController extends ValueNotifier { } if (videoPlayerOptions?.mixWithOthers != null) { - await _videoPlayerPlatform + await videoPlayerPlatform .setMixWithOthers(videoPlayerOptions.mixWithOthers); } - _textureId = await _videoPlayerPlatform.create(dataSourceDescription); + _textureId = await videoPlayerPlatform.create(dataSourceDescription); _creatingCompleter.complete(null); final Completer initializingCompleter = Completer(); @@ -333,7 +336,7 @@ class VideoPlayerController extends ValueNotifier { } } - _eventSubscription = _videoPlayerPlatform + _eventSubscription = videoPlayerPlatform .videoEventsFor(_textureId) .listen(eventListener, onError: errorListener); return initializingCompleter.future; @@ -347,7 +350,7 @@ class VideoPlayerController extends ValueNotifier { _isDisposed = true; _timer?.cancel(); await _eventSubscription?.cancel(); - await _videoPlayerPlatform.dispose(_textureId); + await videoPlayerPlatform.dispose(_textureId); } _lifeCycleObserver.dispose(); } @@ -382,7 +385,7 @@ class VideoPlayerController extends ValueNotifier { if (!value.initialized || _isDisposed) { return; } - await _videoPlayerPlatform.setLooping(_textureId, value.isLooping); + await videoPlayerPlatform.setLooping(_textureId, value.isLooping); } Future _applyPlayPause() async { @@ -390,7 +393,7 @@ class VideoPlayerController extends ValueNotifier { return; } if (value.isPlaying) { - await _videoPlayerPlatform.play(_textureId); + await videoPlayerPlatform.play(_textureId); // Cancel previous timer. _timer?.cancel(); @@ -414,7 +417,7 @@ class VideoPlayerController extends ValueNotifier { await _applyPlaybackSpeed(); } else { _timer?.cancel(); - await _videoPlayerPlatform.pause(_textureId); + await videoPlayerPlatform.pause(_textureId); } } @@ -422,7 +425,7 @@ class VideoPlayerController extends ValueNotifier { if (!value.initialized || _isDisposed) { return; } - await _videoPlayerPlatform.setVolume(_textureId, value.volume); + await videoPlayerPlatform.setVolume(_textureId, value.volume); } Future _applyPlaybackSpeed() async { @@ -435,7 +438,7 @@ class VideoPlayerController extends ValueNotifier { // the video is manually played from Flutter. if (!value.isPlaying) return; - await _videoPlayerPlatform.setPlaybackSpeed( + await videoPlayerPlatform.setPlaybackSpeed( _textureId, value.playbackSpeed, ); @@ -446,7 +449,7 @@ class VideoPlayerController extends ValueNotifier { if (_isDisposed) { return null; } - return await _videoPlayerPlatform.getPosition(_textureId); + return await videoPlayerPlatform.getPosition(_textureId); } /// Sets the video's current timestamp to be at [moment]. The next @@ -463,7 +466,7 @@ class VideoPlayerController extends ValueNotifier { } else if (position < const Duration()) { position = const Duration(); } - await _videoPlayerPlatform.seekTo(_textureId, position); + await videoPlayerPlatform.seekTo(_textureId, position); _updatePosition(position); } @@ -624,7 +627,7 @@ class _VideoPlayerState extends State { Widget build(BuildContext context) { return _textureId == null ? Container() - : _videoPlayerPlatform.buildView(_textureId); + : videoPlayerPlatform.buildView(_textureId); } } From ff2eb559f42c8fb921b520b3171997ee644d5f79 Mon Sep 17 00:00:00 2001 From: Salakar Date: Mon, 7 Dec 2020 16:06:27 +0000 Subject: [PATCH 08/17] tests: add integration test for video buffering events --- .../integration_test/video_player_test.dart | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) 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 639cca9b8631..235c73e2c463 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 @@ -2,10 +2,15 @@ // 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/foundation.dart'; import 'package:flutter/material.dart'; import 'package:integration_test/integration_test.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:video_player/video_player.dart'; +import 'package:video_player_platform_interface/video_player_platform_interface.dart' + show VideoEventType; const Duration _playDuration = Duration(seconds: 1); @@ -29,10 +34,52 @@ void main() { const Duration(seconds: 7, milliseconds: 540)); }); + testWidgets( + 'reports buffering status', + (WidgetTester tester) async { + VideoPlayerController networkController = VideoPlayerController.network( + 'https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4', + ); + await networkController.initialize(); + // Mute to allow playing without DOM interaction on Web. + // See https://developers.google.com/web/updates/2017/09/autoplay-policy-changes + await networkController.setVolume(0); + List recordedEventTypes = []; + videoPlayerPlatform + // ignore: invalid_use_of_visible_for_testing_member + .videoEventsFor(networkController.textureId) + .listen((event) => recordedEventTypes.add(event.eventType)); + + await networkController.play(); + await networkController.seekTo(const Duration(seconds: 5)); + await tester.pumpAndSettle(_playDuration); + await networkController.pause(); + + expect(networkController.value.isPlaying, false); + expect(networkController.value.position, + (Duration position) => position > const Duration(seconds: 0)); + + expect( + recordedEventTypes, + orderedEquals( + [ + VideoEventType.bufferingStart, + VideoEventType.bufferingUpdate, + VideoEventType.bufferingEnd, + VideoEventType.completed, + ], + ), + ); + }, + ); + testWidgets( 'can be played', (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.play(); await tester.pumpAndSettle(_playDuration); @@ -58,6 +105,9 @@ void main() { 'can be paused', (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); // Play for a second, then pause, and then wait a second. await _controller.play(); @@ -104,6 +154,6 @@ void main() { await tester.pumpAndSettle(); expect(_controller.value.isPlaying, true); - }); + }, skip: kIsWeb); // Web does not support local assets. }); } From 16abc3e0c5d9fd8c06517f2dc8522c3065d37c3d Mon Sep 17 00:00:00 2001 From: Salakar Date: Mon, 7 Dec 2020 16:07:47 +0000 Subject: [PATCH 09/17] fix: implement bufferingStart/End for Web --- .../lib/video_player_web.dart | 36 +++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/packages/video_player/video_player_web/lib/video_player_web.dart b/packages/video_player/video_player_web/lib/video_player_web.dart index 6715d5aca53b..2b0c6c3b1dfc 100644 --- a/packages/video_player/video_player_web/lib/video_player_web.dart +++ b/packages/video_player/video_player_web/lib/video_player_web.dart @@ -131,7 +131,6 @@ class VideoPlayerPlugin extends VideoPlayerPlatform { @override Future getPosition(int textureId) async { - _videoPlayers[textureId].sendBufferingUpdate(); return _videoPlayers[textureId].getPosition(); } @@ -150,12 +149,13 @@ class _VideoPlayer { _VideoPlayer({this.uri, this.textureId}); final StreamController eventController = - StreamController(); + StreamController.broadcast(); final String uri; final int textureId; VideoElement videoElement; bool isInitialized = false; + bool isBuffering = false; void initialize() { videoElement = VideoElement() @@ -176,10 +176,38 @@ class _VideoPlayer { isInitialized = true; sendInitialized(); } + if (isBuffering) { + isBuffering = false; + eventController.add(VideoEvent(eventType: VideoEventType.bufferingEnd)); + } + }); + + videoElement.onCanPlayThrough.listen((dynamic _) { + if (isBuffering) { + isBuffering = false; + eventController.add(VideoEvent(eventType: VideoEventType.bufferingEnd)); + } + }); + + videoElement.onPlaying.listen((dynamic _) { + if (isBuffering) { + isBuffering = false; + eventController.add(VideoEvent(eventType: VideoEventType.bufferingEnd)); + } + }); + + videoElement.onWaiting.listen((dynamic _) { + if (!isBuffering) { + isBuffering = true; + eventController + .add(VideoEvent(eventType: VideoEventType.bufferingStart)); + sendBufferingUpdate(); + } }); // The error event fires when some form of error occurs while attempting to load or perform the media. videoElement.onError.listen((Event _) { + isBuffering = false; // The Event itself (_) doesn't contain info about the actual error. // We need to look at the HTMLMediaElement.error. // See: https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement/error @@ -192,6 +220,10 @@ class _VideoPlayer { }); videoElement.onEnded.listen((dynamic _) { + if (isBuffering) { + isBuffering = false; + eventController.add(VideoEvent(eventType: VideoEventType.bufferingEnd)); + } eventController.add(VideoEvent(eventType: VideoEventType.completed)); }); } From a8134343b56592cf84b2e8aa5648a0de87063a7a Mon Sep 17 00:00:00 2001 From: Salakar Date: Mon, 7 Dec 2020 16:10:01 +0000 Subject: [PATCH 10/17] chore: bump pubspec version & add changelog --- packages/video_player/video_player_web/CHANGELOG.md | 4 ++++ packages/video_player/video_player_web/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/video_player/video_player_web/CHANGELOG.md b/packages/video_player/video_player_web/CHANGELOG.md index d8bebd3ae4b6..dc356b6bd04e 100644 --- a/packages/video_player/video_player_web/CHANGELOG.md +++ b/packages/video_player/video_player_web/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.1.4+2 + +* Fix issue where `isBuffering` is not updating on Web. + ## 0.1.4+1 * Substitute `undefined_prefixed_name: ignore` analyzer setting by a `dart:ui` shim with conditional exports. [Issue](https://github.com/flutter/flutter/issues/69309). diff --git a/packages/video_player/video_player_web/pubspec.yaml b/packages/video_player/video_player_web/pubspec.yaml index ae05bfbcf910..1cf9329f9c14 100644 --- a/packages/video_player/video_player_web/pubspec.yaml +++ b/packages/video_player/video_player_web/pubspec.yaml @@ -4,7 +4,7 @@ homepage: https://github.com/flutter/plugins/tree/master/packages/video_player/v # 0.1.y+z is compatible with 1.0.0, if you land a breaking change bump # the version to 2.0.0. # See more details: https://github.com/flutter/flutter/wiki/Package-migration-to-1.0.0 -version: 0.1.4+1 +version: 0.1.4+2 flutter: plugin: From ca34285651a2ebecf25b0a33302be65983193dc9 Mon Sep 17 00:00:00 2001 From: Salakar Date: Mon, 7 Dec 2020 16:29:53 +0000 Subject: [PATCH 11/17] tests: buffering integration test should run on Android or Web only --- .../video_player/example/integration_test/video_player_test.dart | 1 + 1 file changed, 1 insertion(+) 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 235c73e2c463..455330a9508a 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 @@ -71,6 +71,7 @@ void main() { ), ); }, + skip: !(kIsWeb || defaultTargetPlatform == TargetPlatform.android), ); testWidgets( From 78faecf31a75897b2e4926e7328cf98dd3e57b22 Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 17 Dec 2020 13:23:52 +0000 Subject: [PATCH 12/17] docs: changelog grammar fix --- packages/video_player/video_player/CHANGELOG.md | 2 +- packages/video_player/video_player_web/CHANGELOG.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md index 87f3db54392f..40ff098eaf05 100644 --- a/packages/video_player/video_player/CHANGELOG.md +++ b/packages/video_player/video_player/CHANGELOG.md @@ -1,6 +1,6 @@ ## 1.0.2 -* Fix issue where `isBuffering` is not updating on Android. +* Fix an issue where `isBuffering` is not updating on Android. ## 1.0.1 diff --git a/packages/video_player/video_player_web/CHANGELOG.md b/packages/video_player/video_player_web/CHANGELOG.md index dc356b6bd04e..347a6c039b27 100644 --- a/packages/video_player/video_player_web/CHANGELOG.md +++ b/packages/video_player/video_player_web/CHANGELOG.md @@ -1,6 +1,6 @@ ## 0.1.4+2 -* Fix issue where `isBuffering` is not updating on Web. +* Fix an issue where `isBuffering` is not updating on Web. ## 0.1.4+1 From e4f7c6e515a0f0f836dabe7b47434e1813511959 Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 17 Dec 2020 13:26:28 +0000 Subject: [PATCH 13/17] refactor: move `isBuffering` state to event listener --- .../main/java/io/flutter/plugins/videoplayer/VideoPlayer.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java b/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java index 9fbdf45516c9..4c325c00b171 100644 --- a/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java +++ b/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java @@ -54,8 +54,6 @@ final class VideoPlayer { private boolean isInitialized = false; - private boolean isBuffering = false; - private final VideoPlayerOptions options; VideoPlayer( @@ -171,6 +169,7 @@ public void onCancel(Object o) { exoPlayer.addListener( new EventListener() { + private boolean isBuffering = false; @Override public void onPlaybackStateChanged(final int playbackState) { From c58c1f78e60cf5c04380e8a1dc9adf4b1a49ff02 Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 17 Dec 2020 13:42:08 +0000 Subject: [PATCH 14/17] refactor(android): define a `setBuffering` method to be used instead of code duplication. Additionally buffering end events are now sent on errors if applicable. --- .../plugins/videoplayer/VideoPlayer.java | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java b/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java index 4c325c00b171..65657509b49f 100644 --- a/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java +++ b/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java @@ -171,15 +171,19 @@ public void onCancel(Object o) { new EventListener() { private boolean isBuffering = false; + public void setBuffering(boolean buffering) { + if (isBuffering != buffering) { + isBuffering = buffering; + Map event = new HashMap<>(); + event.put("event", isBuffering ? "bufferingStart" : "bufferingEnd"); + eventSink.success(event); + } + } + @Override public void onPlaybackStateChanged(final int playbackState) { if (playbackState == Player.STATE_BUFFERING) { - if (!isBuffering) { - isBuffering = true; - Map event = new HashMap<>(); - event.put("event", "bufferingStart"); - eventSink.success(event); - } + setBuffering(true); sendBufferingUpdate(); } else if (playbackState == Player.STATE_READY) { if (!isInitialized) { @@ -192,17 +196,14 @@ public void onPlaybackStateChanged(final int playbackState) { eventSink.success(event); } - if (isBuffering && playbackState != Player.STATE_BUFFERING) { - isBuffering = false; - Map event = new HashMap<>(); - event.put("event", "bufferingEnd"); - eventSink.success(event); + if (playbackState != Player.STATE_BUFFERING) { + setBuffering(false); } } @Override public void onPlayerError(final ExoPlaybackException error) { - isBuffering = false; + setBuffering(false); if (eventSink != null) { eventSink.error("VideoError", "Video player had error " + error, null); } From 524001419ecca5a7eb3bab1df548d6d0defe0a5b Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 17 Dec 2020 13:43:21 +0000 Subject: [PATCH 15/17] refactor(web): define a `setBuffering` method to be used instead of code duplication. Additionally buffering end events are now sent on errors if applicable. --- .../lib/video_player_web.dart | 36 +++++++++---------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/packages/video_player/video_player_web/lib/video_player_web.dart b/packages/video_player/video_player_web/lib/video_player_web.dart index 2b0c6c3b1dfc..6eae1507daa8 100644 --- a/packages/video_player/video_player_web/lib/video_player_web.dart +++ b/packages/video_player/video_player_web/lib/video_player_web.dart @@ -157,6 +157,16 @@ class _VideoPlayer { bool isInitialized = false; bool isBuffering = false; + void setBuffering(bool buffering) { + if (isBuffering != buffering) { + isBuffering = buffering; + eventController.add(VideoEvent( + eventType: isBuffering + ? VideoEventType.bufferingStart + : VideoEventType.bufferingEnd)); + } + } + void initialize() { videoElement = VideoElement() ..src = uri @@ -176,38 +186,27 @@ class _VideoPlayer { isInitialized = true; sendInitialized(); } - if (isBuffering) { - isBuffering = false; - eventController.add(VideoEvent(eventType: VideoEventType.bufferingEnd)); - } + setBuffering(false); }); videoElement.onCanPlayThrough.listen((dynamic _) { - if (isBuffering) { - isBuffering = false; - eventController.add(VideoEvent(eventType: VideoEventType.bufferingEnd)); - } + setBuffering(false); }); videoElement.onPlaying.listen((dynamic _) { - if (isBuffering) { - isBuffering = false; - eventController.add(VideoEvent(eventType: VideoEventType.bufferingEnd)); - } + setBuffering(false); }); videoElement.onWaiting.listen((dynamic _) { if (!isBuffering) { - isBuffering = true; - eventController - .add(VideoEvent(eventType: VideoEventType.bufferingStart)); + setBuffering(true); sendBufferingUpdate(); } }); // The error event fires when some form of error occurs while attempting to load or perform the media. videoElement.onError.listen((Event _) { - isBuffering = false; + setBuffering(false); // The Event itself (_) doesn't contain info about the actual error. // We need to look at the HTMLMediaElement.error. // See: https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement/error @@ -220,10 +219,7 @@ class _VideoPlayer { }); videoElement.onEnded.listen((dynamic _) { - if (isBuffering) { - isBuffering = false; - eventController.add(VideoEvent(eventType: VideoEventType.bufferingEnd)); - } + setBuffering(false); eventController.add(VideoEvent(eventType: VideoEventType.completed)); }); } From 081ce00435cd939793ac9ac1b749f7bf8a01a69d Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 17 Dec 2020 14:23:46 +0000 Subject: [PATCH 16/17] refactor: revert `visibleForTesting` addition and test using `networkController.addListener` --- .../integration_test/video_player_test.dart | 39 ++++++++++--------- .../video_player/lib/video_player.dart | 29 +++++++------- .../lib/video_player_web.dart | 8 ++-- 3 files changed, 37 insertions(+), 39 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 455330a9508a..f434df8dd843 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 @@ -9,8 +9,6 @@ import 'package:flutter/material.dart'; import 'package:integration_test/integration_test.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:video_player/video_player.dart'; -import 'package:video_player_platform_interface/video_player_platform_interface.dart' - show VideoEventType; const Duration _playDuration = Duration(seconds: 1); @@ -44,11 +42,22 @@ void main() { // Mute to allow playing without DOM interaction on Web. // See https://developers.google.com/web/updates/2017/09/autoplay-policy-changes await networkController.setVolume(0); - List recordedEventTypes = []; - videoPlayerPlatform - // ignore: invalid_use_of_visible_for_testing_member - .videoEventsFor(networkController.textureId) - .listen((event) => recordedEventTypes.add(event.eventType)); + final Completer started = Completer(); + final Completer ended = Completer(); + bool startedBuffering = false; + bool endedBuffering = false; + networkController.addListener(() { + if (networkController.value.isBuffering && !startedBuffering) { + startedBuffering = true; + started.complete(); + } + if (startedBuffering && + !networkController.value.isBuffering && + !endedBuffering) { + endedBuffering = true; + ended.complete(); + } + }); await networkController.play(); await networkController.seekTo(const Duration(seconds: 5)); @@ -59,17 +68,11 @@ void main() { expect(networkController.value.position, (Duration position) => position > const Duration(seconds: 0)); - expect( - recordedEventTypes, - orderedEquals( - [ - VideoEventType.bufferingStart, - VideoEventType.bufferingUpdate, - VideoEventType.bufferingEnd, - VideoEventType.completed, - ], - ), - ); + await started; + expect(startedBuffering, true); + + await ended; + expect(endedBuffering, true); }, skip: !(kIsWeb || defaultTargetPlatform == TargetPlatform.android), ); diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index 9d50638f6c2e..ac1645085e36 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -17,10 +17,7 @@ export 'package:video_player_platform_interface/video_player_platform_interface. import 'src/closed_caption_file.dart'; export 'src/closed_caption_file.dart'; -/// This is just exposed for testing. It shouldn't be used by anyone depending -/// on the plugin. -@visibleForTesting -final VideoPlayerPlatform videoPlayerPlatform = VideoPlayerPlatform.instance +final VideoPlayerPlatform _videoPlayerPlatform = VideoPlayerPlatform.instance // This will clear all open videos on the platform when a full restart is // performed. ..init(); @@ -278,11 +275,11 @@ class VideoPlayerController extends ValueNotifier { } if (videoPlayerOptions?.mixWithOthers != null) { - await videoPlayerPlatform + await _videoPlayerPlatform .setMixWithOthers(videoPlayerOptions.mixWithOthers); } - _textureId = await videoPlayerPlatform.create(dataSourceDescription); + _textureId = await _videoPlayerPlatform.create(dataSourceDescription); _creatingCompleter.complete(null); final Completer initializingCompleter = Completer(); @@ -336,7 +333,7 @@ class VideoPlayerController extends ValueNotifier { } } - _eventSubscription = videoPlayerPlatform + _eventSubscription = _videoPlayerPlatform .videoEventsFor(_textureId) .listen(eventListener, onError: errorListener); return initializingCompleter.future; @@ -350,7 +347,7 @@ class VideoPlayerController extends ValueNotifier { _isDisposed = true; _timer?.cancel(); await _eventSubscription?.cancel(); - await videoPlayerPlatform.dispose(_textureId); + await _videoPlayerPlatform.dispose(_textureId); } _lifeCycleObserver.dispose(); } @@ -385,7 +382,7 @@ class VideoPlayerController extends ValueNotifier { if (!value.initialized || _isDisposed) { return; } - await videoPlayerPlatform.setLooping(_textureId, value.isLooping); + await _videoPlayerPlatform.setLooping(_textureId, value.isLooping); } Future _applyPlayPause() async { @@ -393,7 +390,7 @@ class VideoPlayerController extends ValueNotifier { return; } if (value.isPlaying) { - await videoPlayerPlatform.play(_textureId); + await _videoPlayerPlatform.play(_textureId); // Cancel previous timer. _timer?.cancel(); @@ -417,7 +414,7 @@ class VideoPlayerController extends ValueNotifier { await _applyPlaybackSpeed(); } else { _timer?.cancel(); - await videoPlayerPlatform.pause(_textureId); + await _videoPlayerPlatform.pause(_textureId); } } @@ -425,7 +422,7 @@ class VideoPlayerController extends ValueNotifier { if (!value.initialized || _isDisposed) { return; } - await videoPlayerPlatform.setVolume(_textureId, value.volume); + await _videoPlayerPlatform.setVolume(_textureId, value.volume); } Future _applyPlaybackSpeed() async { @@ -438,7 +435,7 @@ class VideoPlayerController extends ValueNotifier { // the video is manually played from Flutter. if (!value.isPlaying) return; - await videoPlayerPlatform.setPlaybackSpeed( + await _videoPlayerPlatform.setPlaybackSpeed( _textureId, value.playbackSpeed, ); @@ -449,7 +446,7 @@ class VideoPlayerController extends ValueNotifier { if (_isDisposed) { return null; } - return await videoPlayerPlatform.getPosition(_textureId); + return await _videoPlayerPlatform.getPosition(_textureId); } /// Sets the video's current timestamp to be at [moment]. The next @@ -466,7 +463,7 @@ class VideoPlayerController extends ValueNotifier { } else if (position < const Duration()) { position = const Duration(); } - await videoPlayerPlatform.seekTo(_textureId, position); + await _videoPlayerPlatform.seekTo(_textureId, position); _updatePosition(position); } @@ -627,7 +624,7 @@ class _VideoPlayerState extends State { Widget build(BuildContext context) { return _textureId == null ? Container() - : videoPlayerPlatform.buildView(_textureId); + : _videoPlayerPlatform.buildView(_textureId); } } diff --git a/packages/video_player/video_player_web/lib/video_player_web.dart b/packages/video_player/video_player_web/lib/video_player_web.dart index 6eae1507daa8..c6ec965ed714 100644 --- a/packages/video_player/video_player_web/lib/video_player_web.dart +++ b/packages/video_player/video_player_web/lib/video_player_web.dart @@ -149,7 +149,7 @@ class _VideoPlayer { _VideoPlayer({this.uri, this.textureId}); final StreamController eventController = - StreamController.broadcast(); + StreamController(); final String uri; final int textureId; @@ -198,10 +198,8 @@ class _VideoPlayer { }); videoElement.onWaiting.listen((dynamic _) { - if (!isBuffering) { - setBuffering(true); - sendBufferingUpdate(); - } + setBuffering(true); + sendBufferingUpdate(); }); // The error event fires when some form of error occurs while attempting to load or perform the media. From 7810db5fb9fedacb0092a53c79480cac7389db79 Mon Sep 17 00:00:00 2001 From: Salakar Date: Thu, 17 Dec 2020 14:27:03 +0000 Subject: [PATCH 17/17] refactor: revert removal of manual `sendBufferingUpdate` call --- .../java/io/flutter/plugins/videoplayer/VideoPlayerPlugin.java | 1 + packages/video_player/video_player_web/lib/video_player_web.dart | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayerPlugin.java b/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayerPlugin.java index 665bbc445646..0c5854f4bc83 100644 --- a/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayerPlugin.java +++ b/packages/video_player/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayerPlugin.java @@ -186,6 +186,7 @@ public PositionMessage position(TextureMessage arg) { VideoPlayer player = videoPlayers.get(arg.getTextureId()); PositionMessage result = new PositionMessage(); result.setPosition(player.getPosition()); + player.sendBufferingUpdate(); return result; } diff --git a/packages/video_player/video_player_web/lib/video_player_web.dart b/packages/video_player/video_player_web/lib/video_player_web.dart index c6ec965ed714..55f40f00796a 100644 --- a/packages/video_player/video_player_web/lib/video_player_web.dart +++ b/packages/video_player/video_player_web/lib/video_player_web.dart @@ -131,6 +131,7 @@ class VideoPlayerPlugin extends VideoPlayerPlatform { @override Future getPosition(int textureId) async { + _videoPlayers[textureId].sendBufferingUpdate(); return _videoPlayers[textureId].getPosition(); }