From a67e5ef147308e0fdc5cb0e11406a4b5bc2c982c Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Thu, 26 Dec 2024 00:18:03 -0400 Subject: [PATCH 01/15] Optimize caption retrieval with binary search in VideoPlayerController --- .../video_player/lib/video_player.dart | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index b7ba8340fa66..d037e3b340db 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -728,20 +728,36 @@ class VideoPlayerController extends ValueNotifier { /// /// If no [closedCaptionFile] was specified, this will always return an empty /// [Caption]. + Caption _getCaptionAt(Duration position) { if (_closedCaptionFile == 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 List captions = _closedCaptionFile!.captions; + + int left = 0; + int right = captions.length - 1; + + while (left <= right) { + final int mid = left + ((right - left) ~/ 2); + final Caption midCaption = captions[mid]; + + if (midCaption.start <= delayedPosition && + midCaption.end >= delayedPosition) { + return midCaption; // Found the matching caption + } else if (midCaption.end < delayedPosition) { + // Move to the right half + left = mid + 1; + } else { + // Move to the left half + right = mid - 1; } } - return Caption.none; + return Caption.none; // No matching caption found } /// Returns the file containing closed captions for the video, if any. From 106f8384ccd485cd3d65361dcb9c850eab8a8e86 Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Thu, 26 Dec 2024 00:19:48 -0400 Subject: [PATCH 02/15] Update Change Log --- packages/video_player/video_player/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md index 22a4bc0ed80a..38722c2e9964 100644 --- a/packages/video_player/video_player/CHANGELOG.md +++ b/packages/video_player/video_player/CHANGELOG.md @@ -1,6 +1,7 @@ ## NEXT * Updates minimum supported SDK version to Flutter 3.22/Dart 3.4. +* Use Binary search for finding the correct caption ## 2.9.2 From 44aefb8402ce1b37796c96a9fd06eb0a8038bd0b Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Thu, 26 Dec 2024 15:54:51 -0400 Subject: [PATCH 03/15] Refactor: Optimize caption lookup using binary search with `package:collection`, sorting captions and caching start times for improved performance. --- .../video_player/lib/video_player.dart | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index d037e3b340db..695560d0ed1f 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -378,6 +378,10 @@ class VideoPlayerController extends ValueNotifier { Future? _closedCaptionFileFuture; ClosedCaptionFile? _closedCaptionFile; + + /// The starting durations of each closed caption. Used for binary search. + /// to avoid mapping the list of captions each time. + List _closedCaptionStartingDurations = []; Timer? _timer; bool _isDisposed = false; Completer? _creatingCompleter; @@ -736,25 +740,18 @@ class VideoPlayerController extends ValueNotifier { final Duration delayedPosition = position + value.captionOffset; - final List captions = _closedCaptionFile!.captions; - - int left = 0; - int right = captions.length - 1; + final int captionIndex = binarySearch( + _closedCaptionStartingDurations, delayedPosition); - while (left <= right) { - final int mid = left + ((right - left) ~/ 2); - final Caption midCaption = captions[mid]; + /// if the captionIndex is -1, then the position is before the first caption. + if (captionIndex == -1) { + return Caption.none; + } + final Caption caption = _closedCaptionFile!.captions[captionIndex]; - if (midCaption.start <= delayedPosition && - midCaption.end >= delayedPosition) { - return midCaption; // Found the matching caption - } else if (midCaption.end < delayedPosition) { - // Move to the right half - left = mid + 1; - } else { - // Move to the left half - right = mid - 1; - } + /// Check if the current position is within the caption's start and end time. + if (caption.start <= delayedPosition && caption.end >= delayedPosition) { + return caption; } return Caption.none; // No matching caption found @@ -779,6 +776,16 @@ class VideoPlayerController extends ValueNotifier { Future? closedCaptionFile, ) async { _closedCaptionFile = await closedCaptionFile; + if (_closedCaptionFile != null) { + /// Sort the captions by start time so that we can do a binary search. + _closedCaptionFile!.captions.sort((Caption a, Caption b) { + return a.start.compareTo(b.start); + }); + + /// Store the starting durations of each caption to avoid mapping the list on each check + _closedCaptionStartingDurations = + _closedCaptionFile!.captions.map((Caption e) => e.start).toList(); + } value = value.copyWith(caption: _getCaptionAt(value.position)); } From 08f21d94746c60adc51ec07d683283ae3fea4a36 Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Thu, 26 Dec 2024 17:07:43 -0400 Subject: [PATCH 04/15] Refactor: Enhance closed caption retrieval using binary search from `package:collection` and streamline caption sorting --- .../video_player/lib/video_player.dart | 33 ++++++++++--------- .../video_player/video_player/pubspec.yaml | 2 ++ 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index 695560d0ed1f..06478f0d536c 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'; @@ -379,9 +380,6 @@ class VideoPlayerController extends ValueNotifier { Future? _closedCaptionFileFuture; ClosedCaptionFile? _closedCaptionFile; - /// The starting durations of each closed caption. Used for binary search. - /// to avoid mapping the list of captions each time. - List _closedCaptionStartingDurations = []; Timer? _timer; bool _isDisposed = false; Completer? _creatingCompleter; @@ -740,8 +738,17 @@ class VideoPlayerController extends ValueNotifier { final Duration delayedPosition = position + value.captionOffset; - final int captionIndex = binarySearch( - _closedCaptionStartingDurations, delayedPosition); + final int captionIndex = collection.binarySearch( + _closedCaptionFile!.captions, + Caption(number: -1, start: delayedPosition, end: Duration.zero, text: ''), + compare: (Caption p0, Caption p1) { + if (p0.start <= delayedPosition && p0.end >= delayedPosition) { + return 0; + } else { + return p0.start.compareTo(p1.start); + } + }, + ); /// if the captionIndex is -1, then the position is before the first caption. if (captionIndex == -1) { @@ -776,16 +783,12 @@ class VideoPlayerController extends ValueNotifier { Future? closedCaptionFile, ) async { _closedCaptionFile = await closedCaptionFile; - if (_closedCaptionFile != null) { - /// Sort the captions by start time so that we can do a binary search. - _closedCaptionFile!.captions.sort((Caption a, Caption b) { - return a.start.compareTo(b.start); - }); - - /// Store the starting durations of each caption to avoid mapping the list on each check - _closedCaptionStartingDurations = - _closedCaptionFile!.captions.map((Caption e) => e.start).toList(); - } + + /// Sort the captions by start time so that we can do a binary search. + _closedCaptionFile?.captions.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..ed69f80174d8 100644 --- a/packages/video_player/video_player/pubspec.yaml +++ b/packages/video_player/video_player/pubspec.yaml @@ -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 From 246ee3fff3b4356028851d721b18e8864496cc7b Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Fri, 27 Dec 2024 12:49:19 -0400 Subject: [PATCH 05/15] Bump version to 2.9.3 --- packages/video_player/video_player/CHANGELOG.md | 2 +- packages/video_player/video_player/pubspec.yaml | 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 38722c2e9964..b3d68431533e 100644 --- a/packages/video_player/video_player/CHANGELOG.md +++ b/packages/video_player/video_player/CHANGELOG.md @@ -1,4 +1,4 @@ -## NEXT +## 2.9.3 * Updates minimum supported SDK version to Flutter 3.22/Dart 3.4. * Use Binary search for finding the correct caption diff --git a/packages/video_player/video_player/pubspec.yaml b/packages/video_player/video_player/pubspec.yaml index ed69f80174d8..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 From 21426597d7d2f312c12f747417cbf3a7d5ec8a53 Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Fri, 27 Dec 2024 13:24:29 -0400 Subject: [PATCH 06/15] test: Enhance closed caption tests for sorting and seeking behavior --- .../video_player/test/video_player_test.dart | 99 +++++++++++++------ 1 file changed, 69 insertions(+), 30 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 f6eef2448119..613482a2ce0a 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,7 @@ void main() { }); group('caption', () { - test('works when seeking', () async { + test('works when seeking, includes all captions', () async { final VideoPlayerController controller = VideoPlayerController.networkUrl( _localhostUri, @@ -747,20 +769,44 @@ void main() { 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'); + await controller.seekTo(const Duration(milliseconds: 400)); + expect(controller.value.caption.text, ''); await controller.seekTo(const Duration(milliseconds: 500)); + expect(controller.value.caption.text, 'three'); + + await controller.seekTo(const Duration(milliseconds: 600)); + expect(controller.value.caption.text, 'four'); + + 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 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('captions are sorted correctly on initialization', () async { + final VideoPlayerController controller = + VideoPlayerController.networkUrl( + _localhostUri, + closedCaptionFile: _loadClosedCaption(), + ); + + await controller.initialize(); + final sortedCaptions = (await controller.closedCaptionFile!).captions; + for (int i = 1; i < sortedCaptions.length; i++) { + expect( + sortedCaptions[i - 1].start <= sortedCaptions[i].start, isTrue); + } }); - 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, @@ -773,31 +819,30 @@ void main() { expect(controller.value.caption.text, ''); 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)); - expect(controller.value.caption.text, 'two'); + await controller.seekTo(const Duration(milliseconds: 200)); + expect(controller.value.caption.text, 'one'); - await controller.seekTo(const Duration(milliseconds: 300)); + await controller.seekTo(const Duration(milliseconds: 400)); expect(controller.value.caption.text, 'two'); - await controller.seekTo(const Duration(milliseconds: 301)); - expect(controller.value.caption.text, ''); - 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, 'four'); - 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, @@ -815,26 +860,20 @@ void main() { await controller.seekTo(const Duration(milliseconds: 200)); expect(controller.value.caption.text, 'one'); - await controller.seekTo(const Duration(milliseconds: 250)); - expect(controller.value.caption.text, 'one'); - await controller.seekTo(const Duration(milliseconds: 300)); expect(controller.value.caption.text, 'one'); - await controller.seekTo(const Duration(milliseconds: 301)); - expect(controller.value.caption.text, ''); - await controller.seekTo(const Duration(milliseconds: 400)); expect(controller.value.caption.text, 'two'); await controller.seekTo(const Duration(milliseconds: 500)); - expect(controller.value.caption.text, 'two'); + expect(controller.value.caption.text, 'three'); await controller.seekTo(const Duration(milliseconds: 600)); - expect(controller.value.caption.text, ''); + expect(controller.value.caption.text, 'four'); - 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, 'five'); }); test('setClosedCaptionFile loads caption file', () async { From 5e9ace9dc77cea9b8b6df1d3d97bdd03830a3c55 Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Fri, 27 Dec 2024 13:31:33 -0400 Subject: [PATCH 07/15] feat: Add sortedCaptions getter to VideoPlayerController --- .../video_player/video_player/lib/video_player.dart | 11 ++++++++--- .../video_player/test/video_player_test.dart | 6 +++++- 2 files changed, 13 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 06478f0d536c..84d5da4c1093 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -379,7 +379,10 @@ class VideoPlayerController extends ValueNotifier { Future? _closedCaptionFileFuture; ClosedCaptionFile? _closedCaptionFile; + List? _sortedCaptions; + /// The sorted list of captions from the closed caption file. + List? get sortedCaptions => _sortedCaptions; Timer? _timer; bool _isDisposed = false; Completer? _creatingCompleter; @@ -732,14 +735,14 @@ class VideoPlayerController extends ValueNotifier { /// [Caption]. Caption _getCaptionAt(Duration position) { - if (_closedCaptionFile == null) { + if (_closedCaptionFile == null || _sortedCaptions == null) { return Caption.none; } final Duration delayedPosition = position + value.captionOffset; final int captionIndex = collection.binarySearch( - _closedCaptionFile!.captions, + _sortedCaptions!, Caption(number: -1, start: delayedPosition, end: Duration.zero, text: ''), compare: (Caption p0, Caption p1) { if (p0.start <= delayedPosition && p0.end >= delayedPosition) { @@ -784,8 +787,10 @@ class VideoPlayerController extends ValueNotifier { ) async { _closedCaptionFile = await closedCaptionFile; + _sortedCaptions = _closedCaptionFile?.captions; + /// Sort the captions by start time so that we can do a binary search. - _closedCaptionFile?.captions.sort((Caption a, Caption b) { + _sortedCaptions?.sort((Caption a, Caption b) { return a.start.compareTo(b.start); }); 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 613482a2ce0a..5adc0eefbf3e 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -83,6 +83,9 @@ class FakeController extends ValueNotifier Future setClosedCaptionFile( Future? closedCaptionFile, ) async {} + + @override + List? get sortedCaptions => null; } Future _loadClosedCaption() async => @@ -797,7 +800,8 @@ void main() { ); await controller.initialize(); - final sortedCaptions = (await controller.closedCaptionFile!).captions; + + final List sortedCaptions = controller.sortedCaptions!; for (int i = 1; i < sortedCaptions.length; i++) { expect( sortedCaptions[i - 1].start <= sortedCaptions[i].start, isTrue); From b74b3523010d19ce3577c6eedf25ff77659207e8 Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Fri, 27 Dec 2024 14:17:28 -0400 Subject: [PATCH 08/15] fix: Correct binary search logic for caption retrieval and update tests for accurate caption display --- .../video_player/lib/video_player.dart | 24 +++++++++----- .../video_player/test/video_player_test.dart | 32 +++++++++++++------ 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index 84d5da4c1093..b54606ccf372 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -743,23 +743,31 @@ class VideoPlayerController extends ValueNotifier { final int captionIndex = collection.binarySearch( _sortedCaptions!, - Caption(number: -1, start: delayedPosition, end: Duration.zero, text: ''), - compare: (Caption p0, Caption p1) { - if (p0.start <= delayedPosition && p0.end >= delayedPosition) { - return 0; + 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 { - return p0.start.compareTo(p1.start); + // delayedPosition is within [candidate.start, candidate.end] + return 0; } }, ); - /// if the captionIndex is -1, then the position is before the first caption. + // -1 means not found by the binary search. if (captionIndex == -1) { return Caption.none; } - final Caption caption = _closedCaptionFile!.captions[captionIndex]; - /// Check if the current position is within the caption's start and end time. + 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; } 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 5adc0eefbf3e..a7f2922a36da 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -83,7 +83,7 @@ class FakeController extends ValueNotifier Future setClosedCaptionFile( Future? closedCaptionFile, ) async {} - + @override List? get sortedCaptions => null; } @@ -773,18 +773,23 @@ void main() { 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: 600)); + await controller.seekTo(const Duration(milliseconds: 601)); expect(controller.value.caption.text, 'four'); - await controller.seekTo(const Duration(milliseconds: 700)); + 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 @@ -822,20 +827,29 @@ 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: 150)); expect(controller.value.caption.text, ''); await controller.seekTo(const Duration(milliseconds: 200)); - expect(controller.value.caption.text, 'one'); + expect(controller.value.caption.text, 'two'); - await controller.seekTo(const Duration(milliseconds: 400)); + await controller.seekTo(const Duration(milliseconds: 201)); expect(controller.value.caption.text, 'two'); + 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, 'three'); await controller.seekTo(const Duration(milliseconds: 600)); - expect(controller.value.caption.text, 'four'); + expect(controller.value.caption.text, 'five'); await controller.seekTo(const Duration(milliseconds: 700)); expect(controller.value.caption.text, 'five'); @@ -871,13 +885,13 @@ void main() { expect(controller.value.caption.text, 'two'); await controller.seekTo(const Duration(milliseconds: 500)); - expect(controller.value.caption.text, 'three'); + expect(controller.value.caption.text, 'two'); await controller.seekTo(const Duration(milliseconds: 600)); - expect(controller.value.caption.text, 'four'); + expect(controller.value.caption.text, 'three'); await controller.seekTo(const Duration(milliseconds: 700)); - expect(controller.value.caption.text, 'five'); + expect(controller.value.caption.text, 'three'); }); test('setClosedCaptionFile loads caption file', () async { From cedd1a4385ba644338cee90095511f72d164158c Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Fri, 27 Dec 2024 14:23:26 -0400 Subject: [PATCH 09/15] adding more captions checks --- .../video_player/test/video_player_test.dart | 12 ++++++++++++ 1 file changed, 12 insertions(+) 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 a7f2922a36da..3558946ead4e 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -772,6 +772,9 @@ void main() { 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'); + await controller.seekTo(const Duration(milliseconds: 400)); expect(controller.value.caption.text, 'two'); @@ -833,6 +836,9 @@ void main() { 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: 150)); expect(controller.value.caption.text, ''); @@ -878,9 +884,15 @@ void main() { await controller.seekTo(const Duration(milliseconds: 200)); expect(controller.value.caption.text, 'one'); + await controller.seekTo(const Duration(milliseconds: 250)); + expect(controller.value.caption.text, 'one'); + await controller.seekTo(const Duration(milliseconds: 300)); expect(controller.value.caption.text, 'one'); + await controller.seekTo(const Duration(milliseconds: 301)); + expect(controller.value.caption.text, ''); + await controller.seekTo(const Duration(milliseconds: 400)); expect(controller.value.caption.text, 'two'); From 9a23e4787603fcacae64d227fcd3bbe2e4710bcd Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Fri, 27 Dec 2024 16:30:19 -0400 Subject: [PATCH 10/15] refactor: Remove sortedCaptions getter and related test. --- .../video_player/lib/video_player.dart | 2 -- .../video_player/test/video_player_test.dart | 19 ------------------- 2 files changed, 21 deletions(-) diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index b54606ccf372..eeef6aed358b 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -381,8 +381,6 @@ class VideoPlayerController extends ValueNotifier { ClosedCaptionFile? _closedCaptionFile; List? _sortedCaptions; - /// The sorted list of captions from the closed caption file. - List? get sortedCaptions => _sortedCaptions; Timer? _timer; bool _isDisposed = false; Completer? _creatingCompleter; 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 3558946ead4e..50dc871fa471 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -83,9 +83,6 @@ class FakeController extends ValueNotifier Future setClosedCaptionFile( Future? closedCaptionFile, ) async {} - - @override - List? get sortedCaptions => null; } Future _loadClosedCaption() async => @@ -800,22 +797,6 @@ void main() { expect(controller.value.caption.text, 'two'); }); - test('captions are sorted correctly on initialization', () async { - final VideoPlayerController controller = - VideoPlayerController.networkUrl( - _localhostUri, - closedCaptionFile: _loadClosedCaption(), - ); - - await controller.initialize(); - - final List sortedCaptions = controller.sortedCaptions!; - for (int i = 1; i < sortedCaptions.length; i++) { - expect( - sortedCaptions[i - 1].start <= sortedCaptions[i].start, isTrue); - } - }); - test( 'works when seeking with captionOffset positive, includes all captions', () async { From 57260cb65fdb582569b9b39a7e8fcf4be648d039 Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Tue, 31 Dec 2024 14:56:35 -0400 Subject: [PATCH 11/15] test: Add test to verify input captions are unsorted --- .../video_player/test/video_player_test.dart | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) 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 50dc871fa471..d2d5f5ae544c 100644 --- a/packages/video_player/video_player/test/video_player_test.dart +++ b/packages/video_player/video_player/test/video_player_test.dart @@ -749,6 +749,31 @@ void main() { }); group('caption', () { + 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( From 5b383f025c31e918f0992576076c313c2dc9f604 Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Wed, 22 Jan 2025 20:16:13 -0400 Subject: [PATCH 12/15] chore: Update pubspec.yaml deps to be sorted alphabetically. --- 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 0ed822f585f0..07d8df517d0f 100644 --- a/packages/video_player/video_player/pubspec.yaml +++ b/packages/video_player/video_player/pubspec.yaml @@ -22,10 +22,10 @@ flutter: default_package: video_player_web dependencies: + collection: ^1.19.0 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 From 465014f82e782f2397d1ca306370b18ef2c7098c Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Wed, 22 Jan 2025 20:17:47 -0400 Subject: [PATCH 13/15] chore: improve change log to follow guidelines --- packages/video_player/video_player/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md index b3d68431533e..842b0637b96b 100644 --- a/packages/video_player/video_player/CHANGELOG.md +++ b/packages/video_player/video_player/CHANGELOG.md @@ -1,7 +1,7 @@ ## 2.9.3 * Updates minimum supported SDK version to Flutter 3.22/Dart 3.4. -* Use Binary search for finding the correct caption +* Optimizes caption retrieval with Binary search. ## 2.9.2 From d41f6a470e4a1a5c52c85713c129d335b95a87b1 Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Wed, 22 Jan 2025 20:29:25 -0400 Subject: [PATCH 14/15] chore: remove unnecessary blank line in pubspec.yaml --- packages/video_player/video_player/pubspec.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/video_player/video_player/pubspec.yaml b/packages/video_player/video_player/pubspec.yaml index 07d8df517d0f..59049e7ca8b1 100644 --- a/packages/video_player/video_player/pubspec.yaml +++ b/packages/video_player/video_player/pubspec.yaml @@ -31,7 +31,6 @@ dependencies: video_player_platform_interface: ^6.2.0 video_player_web: ^2.1.0 - dev_dependencies: flutter_test: sdk: flutter From edb882c5ce28a952a0f899cb24c231888924ea74 Mon Sep 17 00:00:00 2001 From: Abdelaziz Mahdy Date: Thu, 23 Jan 2025 22:05:34 -0400 Subject: [PATCH 15/15] chore: downgrade collection dependency from 1.19.0 to 1.18.0 --- 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 59049e7ca8b1..6d7fb70585ec 100644 --- a/packages/video_player/video_player/pubspec.yaml +++ b/packages/video_player/video_player/pubspec.yaml @@ -22,7 +22,7 @@ flutter: default_package: video_player_web dependencies: - collection: ^1.19.0 + collection: ^1.18.0 flutter: sdk: flutter html: ^0.15.0