[video_player_videohole] Migrating from Platform Channels to Dart FFI by gin7773 · Pull Request #1073 · flutter-tizen/plugins

gin7773 · 2026-07-28T10:37:49Z

Main changes:

Migrated from Platform Channels to Dart FFI.
Replaced EventChannel with FFI port for event callbacks.
Added JSON serialization for complex parameters.

Threading note

After migrating from Platform Channels to Dart FFI, some native calls are invoked synchronously from the Flutter UI thread rather than through the previous platform-channel handler path. The ecore_wl2 display path is marked with a TODO because those APIs are not thread-safe and will be revisited during the planned ecore-to-GLib migration.

…tore the return type of restore.

- Add nlohmann/json single-header library to tizen/third_party/ - Replace handwritten JSON parser with nlohmann/json in ParseJsonMap() - Simplify ParseCreateMessage() to use json library directly - Add EncodableValueFromJson() helper for JSON to EncodableValue conversion This change improves JSON parsing reliability by supporting: - Escape characters in strings - Nested objects and arrays - Unicode characters - Proper error handling with parse_error exceptions Co-Authored-By: Cline SR

- Add UnregisterAllPlayerEventPorts() function in video_player.cc - Add ffi_unregister_all_player_event_ports() FFI wrapper - Add Dart bindings in ffi_messages.g.dart - Call unregisterAllPlayerEventPorts() in init() to clean up ports on hot restart This fix prevents Dart port leaks when the Dart VM is restarted (e.g., hot restart during development) while the native process continues. Co-Authored-By: Cline SR

- Fix FFI event port symbol name mismatch (ffi_register_dart_port) - Call Prepare() after RestorePlayer for two-phase initialization - Return true from Play() when already playing (idempotent) - Remove duplicate play() call in restored event handler Co-Authored-By: Cline SR

- Move all FFI function implementations from video_player_tizen_plugin.cc to ffi_messages.cc - Add ffi_prepare() function that was missing - Use dependency injection pattern (ffi_set_plugin_registrar) for clean boundaries - Keep video_player_tizen_plugin.cc minimal (plugin registration only) Co-Authored-By: Cline SR

> > 1. Fix potential deadlock in PostEventToDart > - Copy port number under lock, then release lock BEFORE calling Dart_PostCObject_DL > - Use explicit scope block to ensure lock is released immediately > - Prevents deadlock if Dart_PostCObject_DL blocks or callbacks into native code > > 2. Remove dead code (player_index variable) > - player_index in video_player.cc is no longer used > - ID generation moved to media_player.cc (player_id_counter) > > Co-Authored-By: Cline SR

JSUYA · 2026-08-11T06:07:55Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 97da2d750e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Fix the following issues from PR flutter-tizen#1073 reviews: 1. Error event handling missing - Add 'error' event branch in _ensureEventPortRegistered() - Call controller.addError(PlatformException(...)) for error events 2. Seek failure return value not propagated - ffi_seek_to() now returns player->SeekTo(...) ? 0 : -1 3. Plugin lifecycle management missing - Add VideoPlayerTizenPlugin class inheriting flutter::Plugin - Call ffi_dispose_all_players() in destructor - Add ffi_dispose_all_players() function to clean up players and unregister port 4. Dart API DL initialization check missing - Only set _apiDlInitialized if native initialization returns 0 - Throw exception on failure 5. Event port rebinding issue - Change _eventPort from static to instance variable - Each VideoPlayerTizen instance has its own port Co-Authored-By: Cline SR

JSUYA · 2026-08-19T06:00:28Z

+static int64_t g_legacy_dart_port = -1;
+static std::mutex g_legacy_dart_port_mutex;
+
+void ffi_register_event_port(int64_t port) {
+  std::lock_guard<std::mutex> lock(g_legacy_dart_port_mutex);
+  g_legacy_dart_port = port;
+}
+
+void ffi_unregister_event_port() {
+  std::lock_guard<std::mutex> lock(g_legacy_dart_port_mutex);
+  g_legacy_dart_port = -1;
+}


Where are these used? They don't look to be in actual use, and
g_legacy_dart_port also looks to be invalid code.

It has been deleted.

JSUYA · 2026-08-19T06:03:07Z

@@ -0,0 +1,745 @@
+// Copyright 2023 Samsung Electronics Co., Ltd. All rights reserved.


JSUYA · 2026-08-19T06:06:19Z

  return true;
 }

 bool MediaPlayer::StopAndDestroy() {


StopAndDestroy() early-returns when player_stop (line 711) or player_unprepare (line 718) fails. On the Dispose/destructor path that failure now leaks the native player handle — the old destructor unconditionally reached player_destroy. For teardown, please follow the repo's "LOG_ERROR + continue" convention so player_destroy (and player_ = nullptr) is always reached, instead of aborting mid-teardown.

JSUYA · 2026-08-19T06:23:54Z

+    return event_dispatch_state_->disposed;
+  }
+  return true;  // If no event_dispatch_state_, consider as disposed
 }


Does isDisposed() work as intended?
After ~VideoPlayer() is called std::shared_ptr event_dispatch_state_; is valid?

and OnPrepared (line 918) is the only callback missing the guard — please align it.

JSUYA · 2026-08-19T06:26:05Z

+    // Use FFI for initialization (synchronous call)
+    final int result = _ffiApi.initialize();
+    if (result != 0) {
+      throw Exception('FFI initialize failed with code: $result');


Throwing plain Exception on FFI failure changes the app-visible error contract — the previous pigeon implementation (and sibling plugins) throw PlatformException, and the event stream already delivers PlatformException after the earlier review fix. Please unify the method paths on PlatformException (code + message) so existing on PlatformException handlers keep working.

JSUYA · 2026-08-19T06:37:45Z

-    return _api.seekTo(
-      PositionMessage(playerId: playerId, position: position.inMilliseconds),
-    );
+  Future<void> seekTo(int playerId, Duration position) async {


It seems there are some changes from the existing behavior.
seekTo()'s Future now completes when the seek starts (empty callback in ffi_seek_to), not when it finishes as before — so post-await position reads can be stale and rapid consecutive seeks can fail; please complete it via a seekCompleted event through the existing port.
Please check this

JSUYA · 2026-08-19T06:40:30Z

+    // Phase 2: Register global Dart port (only needs to be done once)
+    // Note: registerDartPort is now a no-op since we use global port
+    // The port is already registered in _ensureEventPortRegistered()


Is this comment valid?

It has been deleted.

JSUYA · 2026-08-19T06:41:04Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fd3e299820

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

gin7773 added 12 commits July 16, 2026 17:39

FFI initialize and create

2f0fd5d

Migration of methods other than (de)activate

19c4bd1

remove BasicMessageChannel and VideoPlayerVideoholeApi

51571e3

code format

643a04d

replace EventChannel with ffi

1815a22

fixed dart analyze issue

93de4a2

Code refactoring

3a558d6

Ensure minimal changes

46dee32

remove VideoPlayerTizenPlugin

7efc252

code format

c6c0570

fixed UI not refreshing issue and code structure updated

29d58da

code format

619dc62

gin7773 marked this pull request as draft July 28, 2026 10:38

github-actions Bot added the p: video_player_videohole label Jul 28, 2026

gin7773 added 6 commits July 29, 2026 18:01

Complete the registration and destruction of playerEventPort, and res…

45eafa5

…tore the return type of restore.

code format

16a4f83

Fixed the potential deadlock issue and unescaped JSON issue

b4dbc98

xiaowei-guan self-requested a review July 30, 2026 09:24

gin7773 added 7 commits July 31, 2026 14:19

remove unused log

534df84

fixed some review issues

4530daf

refactor Restore()

2fb3939

remove unused code

06b983b

fix crash issue

9996e07

update version

09c5443

remove unused code

e08ba75

gin7773 marked this pull request as ready for review August 5, 2026 08:19

gin7773 requested a review from JSUYA August 5, 2026 08:19

gin7773 requested a review from seungsoo47 August 5, 2026 08:20

add ffi

daa0dd8

xiaowei-guan reviewed Aug 5, 2026

View reviewed changes

Comment thread packages/video_player_videohole/tizen/src/video_player_tizen_plugin.cc Outdated

xiaowei-guan reviewed Aug 5, 2026

View reviewed changes

Comment thread packages/video_player_videohole/tizen/src/video_player.cc Outdated

Comment thread packages/video_player_videohole/tizen/src/media_player.cc

gin7773 added 3 commits August 5, 2026 18:45

Simplify ffi_messages.h internal API

6e9dd49

JSUYA reviewed Aug 11, 2026

View reviewed changes

Comment thread packages/video_player_videohole/lib/src/video_player_tizen.dart

Comment thread packages/video_player_videohole/tizen/src/media_player.cc

chatgpt-codex-connector Bot reviewed Aug 11, 2026

View reviewed changes

gin7773 added 5 commits August 12, 2026 09:05

Add TODO for ecore_wl2 thread safety issue

3cff139

Fixed review issues

0387b7c

Unified code architecture

45342b6

Corrected the state check in StopAndDestroy()

fd3e299

JSUYA reviewed Aug 19, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Aug 19, 2026

View reviewed changes

Comment thread packages/video_player_videohole/tizen/src/ffi_messages.h Outdated

Comment thread packages/video_player_videohole/tizen/src/media_player.cc

gin7773 added 3 commits August 20, 2026 19:48

fixed some review issues

1beebbe

Clean up FFI event port initialization

6b74187

Fix seekTo completion

bb93681

		@@ -0,0 +1,745 @@
		// Copyright 2023 Samsung Electronics Co., Ltd. All rights reserved.

Uh oh!

Conversation

gin7773 commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Main changes:

Threading note

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JSUYA commented Aug 11, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JSUYA commented Aug 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gin7773 commented Jul 28, 2026 •

edited

Loading