Skip to content

Commit

Permalink
Merge pull request media-kit#452 from alexmercerind/main
Browse files Browse the repository at this point in the history
perf: move `NativePlayer.seek` to `Isolate`; fix(windows): graceful process exit; fix(video/controls): prevent controls from hiding during seek; build(deps): update package:uuid version constraint
  • Loading branch information
alexmercerind authored Sep 7, 2023
2 parents a65ab9c + f80c7e1 commit 12600d4
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 22 deletions.
50 changes: 39 additions & 11 deletions media_kit/lib/src/player/native/player/real.dart
Original file line number Diff line number Diff line change
Expand Up @@ -727,20 +727,18 @@ class NativePlayer extends PlatformPlayer {
if (disposed) {
throw AssertionError('[Player] has been disposed');
}

await waitForPlayerInitialization;
await waitForVideoControllerInitializationIfAttached;

// Raw `mpv_command` calls cause crash on Windows.
final args = [
'seek',
(duration.inMilliseconds / 1000).toStringAsFixed(4),
'absolute',
].join(' ').toNativeUtf8();
mpv.mpv_command_string(
ctx,
args.cast(),
await compute(
_seek,
_SeekData(
ctx.address,
NativeLibrary.path,
duration,
),
);
calloc.free(args);

// It is self explanatory that PlayerState.completed & PlayerStream.completed must enter the false state if seek is called. Typically after EOF.
// https://github.com/media-kit/media-kit/issues/221
Expand Down Expand Up @@ -2530,9 +2528,38 @@ class NativePlayer extends PlatformPlayer {
// Performance sensitive methods in [Player] are executed in an [Isolate].
// This avoids blocking the Dart event loop for long periods of time.
//
// TBD(@alexmercerind): Maybe eventually move all methods to [Isolate]?
// TODO: Maybe eventually move all methods to [Isolate]?
// --------------------------------------------------

class _SeekData {
final int ctx;
final String lib;
final Duration duration;

_SeekData(
this.ctx,
this.lib,
this.duration,
);
}

/// [NativePlayer.seek]
void _seek(_SeekData data) {
// ---------
final mpv = generated.MPV(DynamicLibrary.open(data.lib));
final ctx = Pointer<generated.mpv_handle>.fromAddress(data.ctx);
// ---------
final duration = data.duration;
// ---------
final value = duration.inMilliseconds / 1000;
final command = 'seek ${value.toStringAsFixed(4)} absolute'.toNativeUtf8();
mpv.mpv_command_string(
ctx,
command.cast(),
);
calloc.free(command);
}

class _ScreenshotData {
final int ctx;
final String lib;
Expand All @@ -2545,6 +2572,7 @@ class _ScreenshotData {
);
}

/// [NativePlayer.screenshot]
Uint8List? _screenshot(_ScreenshotData data) {
// ---------
final mpv = generated.MPV(DynamicLibrary.open(data.lib));
Expand Down
2 changes: 1 addition & 1 deletion media_kit/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ environment:
dependencies:
meta: ^1.8.0
path: ^1.8.0
uuid: ^3.0.7
image: ^4.0.17
uri_parser: ^2.0.2
collection: ^1.17.0
synchronized: ^3.1.0
uuid: ">=2.0.0 <5.0.0"
http: ">=0.13.0 <2.0.0"
safe_local_storage: ^1.0.2
universal_platform: ^1.0.0+1
Expand Down
14 changes: 12 additions & 2 deletions media_kit_native_event_loop/src/media_kit_native_event_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ void MediaKitEventLoopHandler::Notify(int64_t handle) {
}
}

void MediaKitEventLoopHandler::Dispose(int64_t handle) {
void MediaKitEventLoopHandler::Dispose(int64_t handle, bool clean) {
auto context = reinterpret_cast<mpv_handle*>(handle);
if (IsRegistered(handle)) {
mutex_.lock();
Expand All @@ -121,6 +121,10 @@ void MediaKitEventLoopHandler::Dispose(int64_t handle) {
std::cout << "MediaKitEventLoopHandler::Dispose: " << e.code() << " " << e.what() << std::endl;
}

if (!clean) {
return;
}

std::thread([&, context]() {
// In extreme usage, |std::thread::join| does not stop the |std::mutex| from being released upon exit, resulting
// in "mutex destroyed while busy" on Windows. Destroying resources after a voluntary delay of 5 seconds.
Expand Down Expand Up @@ -182,7 +186,13 @@ MediaKitEventLoopHandler::~MediaKitEventLoopHandler() {
mutex_.unlock();

for (auto& context : contexts) {
Dispose(reinterpret_cast<int64_t>(context));
// Here, clean argument is `false` for avoiding redundant removal of entries from |mutexes_|, |threads_| &
// |condition_variables_| |std::unordered_map|s. Since, this destructor is only called upon process termination, it
// is not an issue.
// Specifically on Windows, this is done to prevent a crash because the detached thread (used to clean up the
// entries inside |Dispose| after a voluntary delay) fails to launch/clean-before-exit. Overall, this solution
// ensures a graceful exit.
Dispose(reinterpret_cast<int64_t>(context), false);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class MediaKitEventLoopHandler {

void Notify(int64_t handle);

void Dispose(int64_t handle);
void Dispose(int64_t handle, bool clean = true);

void Initialize();

Expand Down
8 changes: 4 additions & 4 deletions media_kit_test/lib/tests/09.seamless.dart
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,12 @@ class _SeamlessState extends State<Seamless> {
// Play the current page's video.
players[i]?.play();
// Pause other pages' videos.
Future.wait(players.entries.map((e) async {
for (final e in players.entries) {
if (e.key != i) {
await e.value.pause();
await e.value.seek(Duration.zero);
e.value.pause();
e.value.seek(Duration.zero);
}
}));
}

// Create the [Player]s & [VideoController]s for the next & previous page.
// It is obvious that current page's [Player] & [VideoController] will already exist, still checking it redundantly
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,24 @@ class _MaterialVideoControlsState extends State<_MaterialVideoControls> {
alignment: Alignment.bottomCenter,
children: [
if (_theme(context).displaySeekBar)
const MaterialSeekBar(),
MaterialSeekBar(
onSeekStart: () {
_timer?.cancel();
},
onSeekEnd: () {
_timer = Timer(
_theme(context).controlsHoverDuration,
() {
if (mounted) {
setState(() {
visible = false;
});
unshiftSubtitle();
}
},
);
},
),
Container(
height: _theme(context).buttonBarHeight,
margin: _theme(context).bottomButtonBarMargin,
Expand Down Expand Up @@ -1048,8 +1065,15 @@ class _MaterialVideoControlsState extends State<_MaterialVideoControls> {
/// Material design seek bar.
class MaterialSeekBar extends StatefulWidget {
final ValueNotifier<Duration>? delta;
final VoidCallback? onSeekStart;
final VoidCallback? onSeekEnd;

const MaterialSeekBar({Key? key, this.delta}) : super(key: key);
const MaterialSeekBar({
Key? key,
this.delta,
this.onSeekStart,
this.onSeekEnd,
}) : super(key: key);

@override
MaterialSeekBarState createState() => MaterialSeekBarState();
Expand Down Expand Up @@ -1135,12 +1159,14 @@ class MaterialSeekBarState extends State<MaterialSeekBar> {
}

void onPointerDown() {
widget.onSeekStart?.call();
setState(() {
tapped = true;
});
}

void onPointerUp() {
widget.onSeekEnd?.call();
setState(() {
tapped = false;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,25 @@ class _MaterialDesktopVideoControlsState
.isNotEmpty
? const Offset(0.0, 16.0)
: Offset.zero,
child: const MaterialDesktopSeekBar(),
child: MaterialDesktopSeekBar(
onSeekStart: () {
_timer?.cancel();
},
onSeekEnd: () {
_timer = Timer(
_theme(context)
.controlsHoverDuration,
() {
if (mounted) {
setState(() {
visible = false;
});
unshiftSubtitle();
}
},
);
},
),
),
if (_theme(context)
.bottomButtonBar
Expand Down Expand Up @@ -801,8 +819,13 @@ class _MaterialDesktopVideoControlsState

/// Material design seek bar.
class MaterialDesktopSeekBar extends StatefulWidget {
final VoidCallback? onSeekStart;
final VoidCallback? onSeekEnd;

const MaterialDesktopSeekBar({
Key? key,
this.onSeekStart,
this.onSeekEnd,
}) : super(key: key);

@override
Expand Down Expand Up @@ -874,12 +897,14 @@ class MaterialDesktopSeekBarState extends State<MaterialDesktopSeekBar> {
}

void onPointerDown() {
widget.onSeekStart?.call();
setState(() {
click = true;
});
}

void onPointerUp() {
widget.onSeekEnd?.call();
setState(() {
click = false;
});
Expand Down

0 comments on commit 12600d4

Please sign in to comment.