Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[video_player] Pause video when it completes #3727

Merged
merged 10 commits into from
Jul 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/video_player/video_player/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.1.10

* Ensure video pauses correctly when it finishes.

## 2.1.9

* Silenced warnings that may occur during build when using a very
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,51 @@ void main() {
},
);

testWidgets(
'stay paused when seeking after video completed',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original issue (flutter/flutter#77674)

(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',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The additional symptom I observed in flutter/flutter#77674 (comment)

(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<bool> started() async {
Expand Down
12 changes: 10 additions & 2 deletions packages/video_player/video_player/lib/video_player.dart
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,11 @@ class VideoPlayerController extends ValueNotifier<VideoPlayerValue> {
_applyPlayPause();
break;
case VideoEventType.completed:
value = value.copyWith(isPlaying: false, position: value.duration);
_timer?.cancel();
// In this case we need to stop _timer, set isPlaying=false, and
// position=value.duration. Instead of setting the values directly,
// we use pause() and seekTo() to ensure the platform stops playing
// and seeks to the last frame of the video.
pause().then((void pauseResult) => seekTo(value.duration));
break;
case VideoEventType.bufferingUpdate:
value = value.copyWith(buffered: event.buffered);
Expand Down Expand Up @@ -385,10 +388,15 @@ class VideoPlayerController extends ValueNotifier<VideoPlayerValue> {

/// 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.
Future<void> play() async {
if (value.position == value.duration) {
await seekTo(const Duration());
}
value = value.copyWith(isPlaying: true);
await _applyPlayPause();
}
Expand Down
2 changes: 1 addition & 1 deletion packages/video_player/video_player/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.9
version: 2.1.10

environment:
sdk: ">=2.12.0 <3.0.0"
Expand Down
21 changes: 20 additions & 1 deletion packages/video_player/video_player/test/video_player_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,23 @@ void main() {
expect(fakeVideoPlayerPlatform.calls.last, 'setPlaybackSpeed');
});

test('play restarts from beginning if video is at end', () async {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new behavior I added to make Android and iOS consistent with web.

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',
Expand Down Expand Up @@ -459,6 +476,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);
Expand All @@ -470,7 +489,7 @@ void main() {
await tester.pumpAndSettle();

expect(controller.value.isPlaying, isFalse);
expect(controller.value.position, controller.value.duration);
expect(controller.value.position, nonzeroDuration);
Copy link
Contributor Author

@KyleFin KyleFin Mar 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, duration was zero so this assertion would pass even if position never changed.

});

testWidgets('buffering status', (WidgetTester tester) async {
Expand Down