[video_player_android] Fix incorrect dimensions swap #9199

marvin-kolja · 2025-05-04T12:44:59Z

List which issues are fixed by this PR. You must list at least one issue.

Manual Testing Overview

Android API <= 21
Android API >= 29

API	Device	Video requires rotation	Works w/o fix	Works w/ fix	Ref
35	Emulator	yes	no	yes	#9199 (comment) by @marvin-kolja
34	Pixel Tablet	no	no	yes	#9199 (comment) by @camsim99
34	Pixel Tablet	yes	no	yes	@camsim99
33	Samsung A71	?	?	yes	#9199 (comment) by @PaulineMoovency
32	Pixel 3A	yes	no	yes	@camsim99
27	Samsung Galaxy J7	yes	yes	yes	@camsim99

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩² ↩³

…270 rotation Since flutter#6535 `VideoSize` is used to get width and height instead of getting them from the video `Format`. My testing concluded that `VideoSize` dimension values are post rotation and do not need to be swapped when there is a 90 or 270 degree rotation present.

)

marvin-kolja · 2025-05-04T12:49:25Z

Tested changes on an emulator using Android 15 (API 35):

Texture View before changes

Texture View after changes

Video Used

RedApple.MOV

camsim99 · 2025-05-05T22:02:06Z

Just an FYI that I'm working on #9107, which will change the API >= 29 check (that gates the usage of Formats) to be more accurate. I'm not sure if it impacts this change, but it's just something to be aware of.

camsim99 · 2025-05-05T22:11:48Z

Just tested out a Pixel Tablet running API 34 (handles crop and rotation automatically) and it appears to work on the main branch and in this PR (with the videos provided in the example app--not sure if this makes a difference). Will do more testing on physical devices.

camsim99 · 2025-05-05T21:56:00Z

packages/video_player/video_player_android/example/lib/mini_controller.dart

+    return RotatedBox(
+      quarterTurns: rotation ~/ 90,
+      child: child,
+    );


Why this change?

This is just to mirror the changes that were done in #8685 to the _VideoPlayerWithRotation of the main video_player package.

I can revert that change if it doesn't make sense in this PR...But the example is broken otherwise.

Oh ok this definitely needs to be fixed then. Can you mention this change in the CHANGELOG?

marvin-kolja · 2025-05-05T23:15:36Z

Just an FYI that I'm working on #9107, which will change the API >= 29 check (that gates the usage of Formats) to be more accurate. I'm not sure if it impacts this change, but it's just something to be aware of.

Thanks for bringing that up. I also thought about that, but I don't see any reason for conflict.

marvin-kolja · 2025-05-05T23:22:00Z

Just tested out a Pixel Tablet running API 34 (handles crop and rotation automatically) and it appears to work on the main branch and in this PR (with the videos provided in the example app--not sure if this makes a difference). Will do more testing on physical devices.

Appreciate it! They will definitely reflect if this PR messed anything up with videos that do not need a rotation, but I don't think any of the example videos require rotation to begin with. Maybe we can add a new video that would meet that requirement?

PaulineMoovency · 2025-05-06T09:21:43Z

I just tested on API 33, on a Samsung A71, and the issue is resolved with your fix! Thank you so much!

marvin-kolja · 2025-05-06T15:05:01Z

I just tested on API 33, on a Samsung A71, and the issue is resolved with your fix! Thank you so much!

Thank you! Just to make sure, you tried with a video that requires rotation?

camsim99 · 2025-05-12T20:43:19Z

Did some more testing (with your apple video) and updated the table in the PR description! Also tested on main to see if I could see the difference and I did 👍 change seems to work for me!

camsim99

Changes LGTM! You'll need to fix the CHANGELOG merge conflict though. Thanks for taking on this issue!

camsim99 · 2025-05-12T20:45:44Z

packages/video_player/video_player_android/example/lib/mini_controller.dart

+    return RotatedBox(
+      quarterTurns: rotation ~/ 90,
+      child: child,
+    );


Oh ok this definitely needs to be fixed then. Can you mention this change in the CHANGELOG?

marvin-kolja · 2025-05-13T07:48:02Z

Changes LGTM! You'll need to fix the CHANGELOG merge conflict though. Thanks for taking on this issue!

Will do the changes.
Sure thing. It has been fun to work on this :D

Did some more testing (with your apple video) and updated the table in the PR description! Also tested on main to see if I could see the difference and I did 👍 change seems to work for me!

Very nice. That's a good amount of testing, cheers!

marvin-kolja · 2025-05-13T08:09:40Z

Alright, I've resolved the conflicts and added the example app fix to the CHANGELOG. Thanks for your attentive support on this issue!

bparrishMines

LGTM

…er/packages#9199)

flutter/packages@af0b9a9...6eebe72 2025-05-27 [email protected] [flutter_lints] Bump lints dependency to 6.0.0 and bump package to 6.0.0 (flutter/packages#9108) 2025-05-26 [email protected] Roll Flutter from 60050a0 to 4372bfb (6 revisions) (flutter/packages#9322) 2025-05-26 [email protected] [video_player_android] Add note about Android platform views known issue (flutter/packages#9312) 2025-05-24 [email protected] Roll Flutter from 85564cb to 60050a0 (17 revisions) (flutter/packages#9318) 2025-05-23 [email protected] [camera_android_camerax] Implement `enableAudio` for video recording (flutter/packages#9264) 2025-05-23 [email protected] [video_player_android] isBuffering flag always return true after calling seekTo method - fixed (flutter/packages#9198) 2025-05-23 [email protected] [camera,video_player] replace onSurfaceDestroyed with onSurfaceCleanup (flutter/packages#9316) 2025-05-23 [email protected] [camera,video_player] Update Android to 3.29 minimum (flutter/packages#9317) 2025-05-23 [email protected] Roll Flutter from 33cdd8e to 85564cb (39 revisions) (flutter/packages#9315) 2025-05-23 [email protected] Update repo for 3.32 stable (flutter/packages#9311) 2025-05-23 [email protected] [camera_avfoundation] Correct pigeon configuration output paths (flutter/packages#9313) 2025-05-23 [email protected] [process] Remove from the repository (flutter/packages#9310) 2025-05-22 [email protected] Ensure Example Packages Work on Android API 36 (flutter/packages#9241) 2025-05-22 [email protected] Roll Flutter (stable) from c236373 to be698c4 (1274 revisions) (flutter/packages#9300) 2025-05-22 [email protected] Remove CODEOWNER for generated iOS files (flutter/packages#9272) 2025-05-22 [email protected] [google_maps_flutter] Removes unnecessary parentheses (flutter/packages#9273) 2025-05-21 [email protected] Rebalance Android plugin CODEOWNERS (flutter/packages#9298) 2025-05-21 [email protected] Roll Flutter from 9a78af5 to 33cdd8e (60 revisions) (flutter/packages#9305) 2025-05-21 [email protected] [ci] Update repository for 3.29 release (flutter/packages#9270) 2025-05-21 [email protected] Ignore .cxx/ directories (flutter/packages#9268) 2025-05-21 [email protected] [video_player] Skip some integration tests on web (flutter/packages#9302) 2025-05-21 [email protected] [webview_flutter] Adds support to respond to recoverable SSL certificate errors (flutter/packages#9150) 2025-05-19 [email protected] [camera_android_camerax] Swap out `BroadcastReceiver` for `OrientationEventListener` (flutter/packages#9261) 2025-05-19 [email protected] [video_player_android] Fix incorrect dimensions swap (flutter/packages#9199) 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 [email protected] 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

@marvin-kolja

Fixes flutter/flutter#166097 *List which issues are fixed by this PR. You must list at least one issue.* ## Manual Testing Overview | API | Device | Video requires rotation | Works w/o fix| Works w/ fix | Ref | |---|---|---|---|---|---| | 35 | Emulator | yes | no | yes | flutter#9199 (comment) by @marvin-kolja | | 34 | Pixel Tablet | no | no | yes | flutter#9199 (comment) by @camsim99 | | 34 | Pixel Tablet | yes | no | yes | @camsim99 | | 33 | Samsung A71 | ? | ? | yes | flutter#9199 (comment) by @PaulineMoovency | | 32 | Pixel 3A | yes | no | yes | @camsim99 | | 27 | Samsung Galaxy J7 | yes | yes | yes | @camsim99 | ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.

@marvin-kolja

Fixes flutter/flutter#166097 *List which issues are fixed by this PR. You must list at least one issue.* ## Manual Testing Overview | API | Device | Video requires rotation | Works w/o fix| Works w/ fix | Ref | |---|---|---|---|---|---| | 35 | Emulator | yes | no | yes | flutter#9199 (comment) by @marvin-kolja | | 34 | Pixel Tablet | no | no | yes | flutter#9199 (comment) by @camsim99 | | 34 | Pixel Tablet | yes | no | yes | @camsim99 | | 33 | Samsung A71 | ? | ? | yes | flutter#9199 (comment) by @PaulineMoovency | | 32 | Pixel 3A | yes | no | yes | @camsim99 | | 27 | Samsung Galaxy J7 | yes | yes | yes | @camsim99 | ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.

marvin-kolja added 2 commits May 4, 2025 13:52

fix(video_player_android): rotation layout issue (following flutter#8685

f579d5e

)

marvin-kolja requested a review from camsim99 as a code owner May 4, 2025 12:44

github-actions bot added platform-android p: video_player labels May 4, 2025

marvin-kolja mentioned this pull request May 4, 2025

[video_player_android] Expose correct AspectRatio with rotationCorrection into consideration flutter/flutter#166097

Closed

marvin-kolja added 2 commits May 4, 2025 15:16

feat(video_player_android): remove unused import

12a98ad

chore(video_player_android): version bump + changelog entry

242040e

camsim99 reviewed May 5, 2025

View reviewed changes

camsim99 self-requested a review May 12, 2025 15:52

camsim99 approved these changes May 12, 2025

View reviewed changes

camsim99 requested a review from bparrishMines May 12, 2025 20:47

marvin-kolja added 2 commits May 13, 2025 09:58

Merge branch 'main' into fix/166097

817f6db

chore(video_player_android): add example app fix to changelog

4cd6ad7

chore(video_player_android): add a proper link to PR ref

9a03377

bparrishMines approved these changes May 15, 2025

View reviewed changes

camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label May 19, 2025

camsim99 mentioned this pull request May 19, 2025

[video_player_android] Incorrect orientation when previewing captured videos flutter/flutter#154696

Closed

auto-submit bot merged commit f444a1e into flutter:main May 19, 2025
82 checks passed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 20, 2025

f444a1e3f [video_player_android] Fix incorrect dimensions swap (flutt…

a366b1c

…er/packages#9199)

engine-flutter-autoroll mentioned this pull request May 20, 2025

Roll Packages from af0b9a941dd3 to c2f8201e9998 (2 revisions) flutter/flutter#169137

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 27, 2025

f444a1e3f [video_player_android] Fix incorrect dimensions swap (flutt…

67d46cc

…er/packages#9199)

engine-flutter-autoroll mentioned this pull request May 27, 2025

Roll Packages from af0b9a941dd3 to 6eebe72ca397 (24 revisions) flutter/flutter#169514

Merged

[video_player_android] Fix incorrect dimensions swap #9199

[video_player_android] Fix incorrect dimensions swap #9199

Uh oh!

Conversation

marvin-kolja commented May 4, 2025 • edited by camsim99 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Manual Testing Overview

Pre-Review Checklist

Footnotes

Uh oh!

marvin-kolja commented May 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

camsim99 commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

camsim99 commented May 5, 2025

Uh oh!

camsim99 May 5, 2025

Choose a reason for hiding this comment

Uh oh!

marvin-kolja May 5, 2025

Choose a reason for hiding this comment

Uh oh!

camsim99 May 12, 2025

Choose a reason for hiding this comment

Uh oh!

marvin-kolja commented May 5, 2025

Uh oh!

marvin-kolja commented May 5, 2025

Uh oh!

PaulineMoovency commented May 6, 2025

Uh oh!

marvin-kolja commented May 6, 2025

Uh oh!

camsim99 commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

camsim99 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camsim99 May 12, 2025

Choose a reason for hiding this comment

Uh oh!

marvin-kolja commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marvin-kolja commented May 13, 2025

Uh oh!

bparrishMines left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

marvin-kolja commented May 4, 2025 •

edited by camsim99

Loading

marvin-kolja commented May 4, 2025 •

edited

Loading

camsim99 commented May 5, 2025 •

edited

Loading

camsim99 commented May 12, 2025 •

edited

Loading

camsim99 left a comment •

edited

Loading

marvin-kolja commented May 13, 2025 •

edited

Loading