[video_player_android] Correct rotation of videos recorded by the camera #7846

camsim99 · 2024-10-10T20:58:51Z

Hopefully 🤞 corrects the calculation used to determine the rotation correction of the video being played. These are the different cases this addresses:

Strictly below Android API 22: Kept this calculation the same as before because I was not able to test it. For context, it uses an unapplied rotation degrees reported by the video's size to correct the rotation. Note that this is always 0 after Android API 22+ (see its docs)
Above Android API 22, strictly below Android API 29: The SurfaceTexture Impeller backend is used. From my testing, I see the preview is correctly rotated and sized without any further intervention, so the correction is set 0.
Android API 29+: The ImageReader Impelled backend is used and a preview correction was noticed by the community (see [video_player_android] Incorrect orientation when previewing captured videos flutter#154696). To fix this, we now use the rotation correction reported by the video's format. We also now use this rotation to make the fix for a swapped = width and height when the correction is 90 or 270 degrees (indicating that the video is landscape) as the logic did before but instead with the unapplied rotation degrees (see case 1 for context).

Tested this on Android APIs 28, 30, 32, and 35.

A fix attempt for flutter/flutter#154696.

Pre-launch Checklist

...yer_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java

reidbaker · 2024-10-18T14:51:16Z

...yer_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java

    }
    events.onInitialized(width, height, exoPlayer.getDuration(), rotationCorrection);
  }

+  private int getRotationCorrectionFromUnappliedRotation(int unappliedRotationDegrees) {


Consider using an enum instead of an int since our code does not actually handle in between values.

reidbaker · 2024-10-18T14:52:50Z

...yer_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java

+  }
+
+  @OptIn(markerClass = androidx.media3.common.util.UnstableApi.class)
+  // The annotation is used to enable using a Format to determine the rotation


I think this comment should instead of explaining what an unstable api annotation does should instead explain why it is ok to use an unstable api and what the failure modes are that we can expect. (for example should we expect the signature to possibly change across minor versions).

reidbaker · 2024-10-18T14:54:02Z

...android/android/src/test/java/io/flutter/plugins/videoplayer/ExoPlayerEventListenerTest.java

@@ -33,7 +35,7 @@
 * ({@link VideoPlayerCallbacks} and/or interface with the player instance as expected.
 */
 @RunWith(RobolectricTestRunner.class)
-public final class ExoPlayerEventListenerTests {
+public final class ExoPlayerEventListenerTest {


Did you mean to remove the "s" in tests?

Yes to make it consistent with the other tests in this plugin (and across plugins).

...android/android/src/test/java/io/flutter/plugins/videoplayer/ExoPlayerEventListenerTest.java

...yer_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java

matanlurey · 2024-10-18T17:14:52Z

...yer_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java

    }
    events.onInitialized(width, height, exoPlayer.getDuration(), rotationCorrection);
  }

+  private int getRotationCorrectionFromUnappliedRotation(int unappliedRotationDegrees) {


packages/video_player/video_player_android/android/build.gradle

...yer_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java

camsim99 · 2024-10-21T17:48:47Z

cc @matanlurey the video_player_android test can pause that we added is flaking pretty badly. I can file an issue but I think we should revisit it.

camsim99 · 2024-10-21T23:41:26Z

cc @matanlurey the video_player_android test can pause that we added is flaking pretty badly. I can file an issue but I think we should revisit it.

Filed flutter/flutter#157307

…by the camera (flutter/packages#7846)

flutter/packages@b6f7e47...5e03bb1 2024-10-21 43054281+camsim99@users.noreply.github.com [video_player_android] Correct rotation of videos recorded by the camera (flutter/packages#7846) 2024-10-21 gspencergoog@users.noreply.github.com Allow custom blocks to be something other than `Column` or `SizedBox` (flutter/packages#7859) 2024-10-21 49699333+dependabot[bot]@users.noreply.github.com [camera]: Bump androidx.annotation:annotation from 1.8.2 to 1.9.0 in /packages/camera/camera_android/android (flutter/packages#7905) 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#157349) flutter/packages@b6f7e47...5e03bb1 2024-10-21 43054281+camsim99@users.noreply.github.com [video_player_android] Correct rotation of videos recorded by the camera (flutter/packages#7846) 2024-10-21 gspencergoog@users.noreply.github.com Allow custom blocks to be something other than `Column` or `SizedBox` (flutter/packages#7859) 2024-10-21 49699333+dependabot[bot]@users.noreply.github.com [camera]: Bump androidx.annotation:annotation from 1.8.2 to 1.9.0 in /packages/camera/camera_android/android (flutter/packages#7905) 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

camsim99 added 4 commits October 9, 2024 09:55

debugging

5b8ad64

theoretical fix

bbac0c3

Mild cleanup

43898cb

Undo extra import

95d5e43

github-actions bot added p: camera platform-android p: video_player labels Oct 10, 2024

camsim99 mentioned this pull request Oct 14, 2024

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

Open

camsim99 and others added 7 commits October 15, 2024 16:33

Fix logic + start tests

3306352

Merge remote-tracking branch 'upstream/main' into vp_fix

3524885

Fix code + add tests

2664b51

Remove print statement, bump version

1ba7efc

Add Impeller case

0ff09c6

Merge remote-tracking branch 'upstream/main' into vp_fix

a374698

Cleanup

a74cea9

github-actions bot removed the p: camera label Oct 17, 2024

camsim99 added 2 commits October 17, 2024 11:09

Add comment

da92163

Correct changelog

1a65eea

camsim99 marked this pull request as ready for review October 17, 2024 19:47

camsim99 requested a review from matanlurey as a code owner October 17, 2024 19:47

camsim99 requested a review from a team October 17, 2024 19:47

reidbaker reviewed Oct 18, 2024

View reviewed changes

matanlurey approved these changes Oct 18, 2024

View reviewed changes

camsim99 mentioned this pull request Oct 18, 2024

[video_player_android] Use handlesCropAndRotation to detect the SurfaceTexture Impeller backend flutter/flutter#157198

Open

camsim99 added 2 commits October 18, 2024 15:39

Address review

ee0f7de

Merge remote-tracking branch 'upstream/main' into vp_fix

fc22ee6

reidbaker reviewed Oct 21, 2024

View reviewed changes

address review

5f2a824

camsim99 requested a review from reidbaker October 21, 2024 18:31

reidbaker approved these changes Oct 21, 2024

View reviewed changes

camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 21, 2024

auto-submit bot merged commit 5e03bb1 into flutter:main Oct 21, 2024
76 checks passed

engine-flutter-autoroll mentioned this pull request Oct 22, 2024

Roll Packages from b6f7e47c7586 to 5e03bb1da411 (3 revisions) flutter/flutter#157349

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 22, 2024

5e03bb1da [video_player_android] Correct rotation of videos recorded …

1e70209

…by the camera (flutter/packages#7846)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[video_player_android] Correct rotation of videos recorded by the camera #7846

[video_player_android] Correct rotation of videos recorded by the camera #7846

camsim99 commented Oct 10, 2024 •

edited

Loading

reidbaker Oct 18, 2024

matanlurey Oct 18, 2024

reidbaker Oct 18, 2024

reidbaker Oct 18, 2024

camsim99 Oct 18, 2024

matanlurey Oct 18, 2024

camsim99 commented Oct 21, 2024 •

edited

Loading

camsim99 commented Oct 21, 2024

[video_player_android] Correct rotation of videos recorded by the camera #7846

[video_player_android] Correct rotation of videos recorded by the camera #7846

Conversation

camsim99 commented Oct 10, 2024 • edited Loading

Pre-launch Checklist

reidbaker Oct 18, 2024

Choose a reason for hiding this comment

matanlurey Oct 18, 2024

Choose a reason for hiding this comment

reidbaker Oct 18, 2024

Choose a reason for hiding this comment

reidbaker Oct 18, 2024

Choose a reason for hiding this comment

camsim99 Oct 18, 2024

Choose a reason for hiding this comment

matanlurey Oct 18, 2024

Choose a reason for hiding this comment

camsim99 commented Oct 21, 2024 • edited Loading

camsim99 commented Oct 21, 2024

camsim99 commented Oct 10, 2024 •

edited

Loading

camsim99 commented Oct 21, 2024 •

edited

Loading