diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md index 22a4bc0ed80a..b3d68431533e 100644 --- a/packages/video_player/video_player/CHANGELOG.md +++ b/packages/video_player/video_player/CHANGELOG.md @@ -1,6 +1,7 @@ -## NEXT +## 2.9.3 * Updates minimum supported SDK version to Flutter 3.22/Dart 3.4. +* Use Binary search for finding the correct caption ## 2.9.2 diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index b7ba8340fa66..eeef6aed358b 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'dart:io'; import 'dart:math' as math; +import 'package:collection/collection.dart' as collection; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; @@ -378,6 +379,8 @@ class VideoPlayerController extends ValueNotifier { Future? _closedCaptionFileFuture; ClosedCaptionFile? _closedCaptionFile; + List? _sortedCaptions; + Timer? _timer; bool _isDisposed = false; Completer? _creatingCompleter; @@ -728,20 +731,46 @@ class VideoPlayerController extends ValueNotifier { /// /// If no [closedCaptionFile] was specified, this will always return an empty /// [Caption]. + Caption _getCaptionAt(Duration position) { - if (_closedCaptionFile == null) { + if (_closedCaptionFile == null || _sortedCaptions == null) { return Caption.none; } final Duration delayedPosition = position + value.captionOffset; - // TODO(johnsonmh): This would be more efficient as a binary search. - for (final Caption caption in _closedCaptionFile!.captions) { - if (caption.start <= delayedPosition && caption.end >= delayedPosition) { - return caption; - } + + final int captionIndex = collection.binarySearch( + _sortedCaptions!, + Caption( + number: -1, + start: delayedPosition, + end: delayedPosition, + text: '', + ), + compare: (Caption candidate, Caption search) { + if (search.start < candidate.start) { + return 1; + } else if (search.start > candidate.end) { + return -1; + } else { + // delayedPosition is within [candidate.start, candidate.end] + return 0; + } + }, + ); + + // -1 means not found by the binary search. + if (captionIndex == -1) { + return Caption.none; + } + + final Caption caption = _sortedCaptions![captionIndex]; + // check if it really fits within that caption's [start, end]. + if (caption.start <= delayedPosition && caption.end >= delayedPosition) { + return caption; } - return Caption.none; + return Caption.none; // No matching caption found } /// Returns the file containing closed captions for the video, if any. @@ -763,6 +792,14 @@ class VideoPlayerController extends ValueNotifier { Future? closedCaptionFile, ) async { _closedCaptionFile = await closedCaptionFile; + + _sortedCaptions = _closedCaptionFile?.captions; + + /// Sort the captions by start time so that we can do a binary search. + _sortedCaptions?.sort((Caption a, Caption b) { + return a.start.compareTo(b.start); + }); + value = value.copyWith(caption: _getCaptionAt(value.position)); } diff --git a/packages/video_player/video_player/pubspec.yaml b/packages/video_player/video_player/pubspec.yaml index c19f6c6f77ca..0ed822f585f0 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/packages/tree/main/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.9.2 +version: 2.9.3 environment: sdk: ^3.4.0 @@ -25,11 +25,13 @@ dependencies: flutter: sdk: flutter html: ^0.15.0 + collection: ^1.19.0 video_player_android: ^2.3.5 video_player_avfoundation: ^2.5.6 video_player_platform_interface: ^6.2.0 video_player_web: ^2.1.0 + dev_dependencies: flutter_test: sdk: flutter 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 f6eef2448119..d2d5f5ae544c 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -98,12 +98,34 @@ class _FakeClosedCaptionFile extends ClosedCaptionFile { start: Duration(milliseconds: 100), end: Duration(milliseconds: 200), ), + const Caption( text: 'two', number: 1, start: Duration(milliseconds: 300), end: Duration(milliseconds: 400), ), + + /// out of order subs to test sorting + const Caption( + text: 'three', + number: 1, + start: Duration(milliseconds: 500), + end: Duration(milliseconds: 600), + ), + + const Caption( + text: 'five', + number: 0, + start: Duration(milliseconds: 700), + end: Duration(milliseconds: 800), + ), + const Caption( + text: 'four', + number: 0, + start: Duration(milliseconds: 600), + end: Duration(milliseconds: 700), + ), ]; } } @@ -727,7 +749,32 @@ void main() { }); group('caption', () { - test('works when seeking', () async { + test('makes sure the input captions are unsorted', () async { + final VideoPlayerController controller = + VideoPlayerController.networkUrl( + _localhostUri, + closedCaptionFile: _loadClosedCaption(), + ); + + await controller.initialize(); + final List captions = + (await controller.closedCaptionFile)!.captions.toList(); + + // Check that captions are not in sorted order + bool isSorted = true; + for (int i = 0; i < captions.length - 1; i++) { + if (captions[i].start.compareTo(captions[i + 1].start) > 0) { + isSorted = false; + break; + } + } + + expect(isSorted, false, reason: 'Expected captions to be unsorted'); + expect(captions.map((Caption c) => c.text).toList(), + ['one', 'two', 'three', 'five', 'four'], + reason: 'Captions should be in original unsorted order'); + }); + test('works when seeking, includes all captions', () async { final VideoPlayerController controller = VideoPlayerController.networkUrl( _localhostUri, @@ -750,17 +797,34 @@ void main() { await controller.seekTo(const Duration(milliseconds: 301)); expect(controller.value.caption.text, 'two'); + await controller.seekTo(const Duration(milliseconds: 400)); + expect(controller.value.caption.text, 'two'); + + await controller.seekTo(const Duration(milliseconds: 401)); + expect(controller.value.caption.text, ''); + await controller.seekTo(const Duration(milliseconds: 500)); + expect(controller.value.caption.text, 'three'); + + await controller.seekTo(const Duration(milliseconds: 601)); + expect(controller.value.caption.text, 'four'); + + await controller.seekTo(const Duration(milliseconds: 701)); + expect(controller.value.caption.text, 'five'); + + await controller.seekTo(const Duration(milliseconds: 800)); + expect(controller.value.caption.text, 'five'); + await controller.seekTo(const Duration(milliseconds: 801)); expect(controller.value.caption.text, ''); + // Test going back await controller.seekTo(const Duration(milliseconds: 300)); expect(controller.value.caption.text, 'two'); - - await controller.seekTo(const Duration(milliseconds: 301)); - expect(controller.value.caption.text, 'two'); }); - test('works when seeking with captionOffset positive', () async { + test( + 'works when seeking with captionOffset positive, includes all captions', + () async { final VideoPlayerController controller = VideoPlayerController.networkUrl( _localhostUri, @@ -772,32 +836,43 @@ void main() { expect(controller.value.position, Duration.zero); expect(controller.value.caption.text, ''); + await controller.seekTo(const Duration(milliseconds: 99)); + expect(controller.value.caption.text, 'one'); + await controller.seekTo(const Duration(milliseconds: 100)); expect(controller.value.caption.text, 'one'); await controller.seekTo(const Duration(milliseconds: 101)); expect(controller.value.caption.text, ''); - await controller.seekTo(const Duration(milliseconds: 250)); + await controller.seekTo(const Duration(milliseconds: 150)); + expect(controller.value.caption.text, ''); + + await controller.seekTo(const Duration(milliseconds: 200)); expect(controller.value.caption.text, 'two'); - await controller.seekTo(const Duration(milliseconds: 300)); + await controller.seekTo(const Duration(milliseconds: 201)); expect(controller.value.caption.text, 'two'); - await controller.seekTo(const Duration(milliseconds: 301)); - expect(controller.value.caption.text, ''); + await controller.seekTo(const Duration(milliseconds: 400)); + expect(controller.value.caption.text, 'three'); await controller.seekTo(const Duration(milliseconds: 500)); - expect(controller.value.caption.text, ''); + expect(controller.value.caption.text, 'three'); - await controller.seekTo(const Duration(milliseconds: 300)); - expect(controller.value.caption.text, 'two'); + await controller.seekTo(const Duration(milliseconds: 600)); + expect(controller.value.caption.text, 'five'); - await controller.seekTo(const Duration(milliseconds: 301)); + await controller.seekTo(const Duration(milliseconds: 700)); + expect(controller.value.caption.text, 'five'); + + await controller.seekTo(const Duration(milliseconds: 800)); expect(controller.value.caption.text, ''); }); - test('works when seeking with captionOffset negative', () async { + test( + 'works when seeking with captionOffset negative, includes all captions', + () async { final VideoPlayerController controller = VideoPlayerController.networkUrl( _localhostUri, @@ -831,10 +906,10 @@ void main() { expect(controller.value.caption.text, 'two'); await controller.seekTo(const Duration(milliseconds: 600)); - expect(controller.value.caption.text, ''); + expect(controller.value.caption.text, 'three'); - await controller.seekTo(const Duration(milliseconds: 300)); - expect(controller.value.caption.text, 'one'); + await controller.seekTo(const Duration(milliseconds: 700)); + expect(controller.value.caption.text, 'three'); }); test('setClosedCaptionFile loads caption file', () async {