-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[video_player] Separate texture ID on Android #10029
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] Separate texture ID on Android #10029
Conversation
Cleans up the tech debt of having two different methods of generating player IDs on Andoird, as was recently done on iOS. The current code was a result of adding non-texture-based players without refactoring the Dart<->Java communication, and relied on knowing that the engine assigned texture IDs by increasing an incrementing value. This fully separates texture IDs from player IDs, so that the plugin can fully control player ID management without relying on engine internals. This also brings the Android and iOS implementations into better alignment, so that they don't have differences that aren't related to the platforms themselves. Follow-up to work done for flutter/flutter#86613 Prep for flutter/flutter#172763
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request is a well-executed refactoring of the Android video player implementation. It successfully decouples player IDs from texture IDs by introducing separate creation methods for platform views and texture views in the Pigeon API. This change simplifies the logic, removes technical debt, and brings the Android code into closer alignment with the iOS implementation. The refactoring is consistently applied across the native Java code, Dart plugin code, and tests, resulting in a cleaner and more maintainable codebase.
| @@ -1,3 +1,7 @@ | |||
| ## 2.8.14 | |||
|
|
|||
| * Restructures internal logic for player creation and tracking. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current changelog entry is a bit generic. To better inform developers about this significant and beneficial refactoring, consider making the description more specific. A more descriptive entry will help users quickly understand the value of this update.
| * Restructures internal logic for player creation and tracking. | |
| * Separates player IDs from texture IDs on Android, simplifying internal player creation and tracking. |
|
autosubmit label was removed for flutter/packages/10029, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR. |
flutter/packages@34eec78...287739d 2025-09-29 engine-flutter-autoroll@skia.org Roll Flutter (stable) from d693b4b to ac4e799 (4 revisions) (flutter/packages#10124) 2025-09-29 magder@google.com Fix CODEOWNERS error (flutter/packages#10127) 2025-09-29 stuartmorgan@google.com [file_selector] Update Pigeon in Android implementation (flutter/packages#10126) 2025-09-29 31859944+LongCatIsLooong@users.noreply.github.com [In_app_purchase_storekit] Do not throw PigeonError when a transaction is pending / cancelled / unverified (flutter/packages#9627) 2025-09-29 stuartmorgan@google.com [video_player] Separate texture ID on Android (flutter/packages#10029) 2025-09-29 engine-flutter-autoroll@skia.org Roll Flutter from 6cc976e to 96fe3b3 (32 revisions) (flutter/packages#10125) 2025-09-29 43054281+camsim99@users.noreply.github.com [camera_android_camerax] Fix NV21 Format (flutter/packages#10022) 2025-09-29 engine-flutter-autoroll@skia.org Manual roll Flutter from b1a28bc to 6cc976e (4 revisions) (flutter/packages#10116) 2025-09-29 robert.odrowaz@leancode.pl [camera_avfoundation] Implementation swift migration - part 13 (flutter/packages#9930) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…r#176284) flutter/packages@34eec78...287739d 2025-09-29 engine-flutter-autoroll@skia.org Roll Flutter (stable) from d693b4b to ac4e799 (4 revisions) (flutter/packages#10124) 2025-09-29 magder@google.com Fix CODEOWNERS error (flutter/packages#10127) 2025-09-29 stuartmorgan@google.com [file_selector] Update Pigeon in Android implementation (flutter/packages#10126) 2025-09-29 31859944+LongCatIsLooong@users.noreply.github.com [In_app_purchase_storekit] Do not throw PigeonError when a transaction is pending / cancelled / unverified (flutter/packages#9627) 2025-09-29 stuartmorgan@google.com [video_player] Separate texture ID on Android (flutter/packages#10029) 2025-09-29 engine-flutter-autoroll@skia.org Roll Flutter from 6cc976e to 96fe3b3 (32 revisions) (flutter/packages#10125) 2025-09-29 43054281+camsim99@users.noreply.github.com [camera_android_camerax] Fix NV21 Format (flutter/packages#10022) 2025-09-29 engine-flutter-autoroll@skia.org Manual roll Flutter from b1a28bc to 6cc976e (4 revisions) (flutter/packages#10116) 2025-09-29 robert.odrowaz@leancode.pl [camera_avfoundation] Implementation swift migration - part 13 (flutter/packages#9930) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Cleans up the tech debt of having two different methods of generating player IDs on Andoird, as was recently done on iOS. The current code was a result of adding non-texture-based players without refactoring the Dart<->Java communication, and relied on knowing that the engine assigned texture IDs by increasing an incrementing value. This fully separates texture IDs from player IDs, so that the plugin can fully control player ID management without relying on engine internals.
This also brings the Android and iOS implementations into better alignment, so that they don't have differences that aren't related to the platforms themselves.
Follow-up to work done for flutter/flutter#86613 Prep for flutter/flutter#172763
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3