Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[video_player] Optimize caption retrieval with binary search in VideoPlayerController #8347

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a67e5ef
Optimize caption retrieval with binary search in VideoPlayerController
abdelaziz-mahdy Dec 26, 2024
106f838
Update Change Log
abdelaziz-mahdy Dec 26, 2024
44aefb8
Refactor: Optimize caption lookup using binary search with `package:c…
abdelaziz-mahdy Dec 26, 2024
08f21d9
Refactor: Enhance closed caption retrieval using binary search from `…
abdelaziz-mahdy Dec 26, 2024
be6ba80
Merge branch 'main' into binary-search-captions
abdelaziz-mahdy Dec 26, 2024
246ee3f
Bump version to 2.9.3
abdelaziz-mahdy Dec 27, 2024
1fd8e70
Merge branch 'binary-search-captions' of https://github.com/zezo357/p…
abdelaziz-mahdy Dec 27, 2024
2142659
test: Enhance closed caption tests for sorting and seeking behavior
abdelaziz-mahdy Dec 27, 2024
5e9ace9
feat: Add sortedCaptions getter to VideoPlayerController
abdelaziz-mahdy Dec 27, 2024
b74b352
fix: Correct binary search logic for caption retrieval and update tes…
abdelaziz-mahdy Dec 27, 2024
cedd1a4
adding more captions checks
abdelaziz-mahdy Dec 27, 2024
9a23e47
refactor: Remove sortedCaptions getter and related test.
abdelaziz-mahdy Dec 27, 2024
57260cb
test: Add test to verify input captions are unsorted
abdelaziz-mahdy Dec 31, 2024
5b383f0
chore: Update pubspec.yaml deps to be sorted alphabetically.
abdelaziz-mahdy Jan 23, 2025
465014f
chore: improve change log to follow guidelines
abdelaziz-mahdy Jan 23, 2025
d41f6a4
chore: remove unnecessary blank line in pubspec.yaml
abdelaziz-mahdy Jan 23, 2025
edb882c
chore: downgrade collection dependency from 1.19.0 to 1.18.0
abdelaziz-mahdy Jan 24, 2025
a7c1159
Merge branch 'main' into binary-search-captions
abdelaziz-mahdy Jan 27, 2025
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
3 changes: 2 additions & 1 deletion packages/video_player/video_player/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## NEXT
## 2.9.3

* Updates minimum supported SDK version to Flutter 3.22/Dart 3.4.
* Optimizes caption retrieval with Binary search.

## 2.9.2

Expand Down
51 changes: 44 additions & 7 deletions packages/video_player/video_player/lib/video_player.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -378,6 +379,8 @@ class VideoPlayerController extends ValueNotifier<VideoPlayerValue> {

Future<ClosedCaptionFile>? _closedCaptionFileFuture;
ClosedCaptionFile? _closedCaptionFile;
List<Caption>? _sortedCaptions;

Timer? _timer;
bool _isDisposed = false;
Completer<void>? _creatingCompleter;
Expand Down Expand Up @@ -728,20 +731,46 @@ class VideoPlayerController extends ValueNotifier<VideoPlayerValue> {
///
/// 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<Caption>(
_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.
Expand All @@ -763,6 +792,14 @@ class VideoPlayerController extends ValueNotifier<VideoPlayerValue> {
Future<ClosedCaptionFile>? 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));
}

Expand Down
3 changes: 2 additions & 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/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
Expand All @@ -22,6 +22,7 @@ flutter:
default_package: video_player_web

dependencies:
collection: ^1.18.0
flutter:
sdk: flutter
html: ^0.15.0
Expand Down
109 changes: 92 additions & 17 deletions packages/video_player/video_player/test/video_player_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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),
),
];
}
}
Expand Down Expand Up @@ -754,7 +776,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<Caption> 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(),
<String>['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,
Expand All @@ -778,17 +825,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,
Expand All @@ -801,32 +865,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,
Expand Down Expand Up @@ -861,10 +936,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 {
Expand Down
Loading