From 5e03bb1da411c99d80727f46e1821c4fdc6ea12c Mon Sep 17 00:00:00 2001 From: Camille Simon <43054281+camsim99@users.noreply.github.com> Date: Mon, 21 Oct 2024 16:42:13 -0700 Subject: [PATCH] [video_player_android] Correct rotation of videos recorded by the camera (#7846) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hopefully 🤞 corrects the calculation used to determine the rotation correction of the video being played. These are the different cases this addresses: 1. **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](https://github.com/google/ExoPlayer/blob/dd430f7053a1a3958deea3ead6a0565150c06bfc/library/common/src/main/java/com/google/android/exoplayer2/video/VideoSize.java#L65)) 2. **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. 3. **Android API 29+:** The `ImageReader` Impelled backend is used and a preview correction was noticed by the community (see https://github.com/flutter/flutter/issues/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 https://github.com/flutter/flutter/issues/154696. --- .../video_player_android/CHANGELOG.md | 11 +- .../video_player_android/android/build.gradle | 5 + .../videoplayer/ExoPlayerEventListener.java | 107 ++++++++++++++++-- ...s.java => ExoPlayerEventListenerTest.java} | 91 ++++++++++++++- .../video_player_android/pubspec.yaml | 2 +- 5 files changed, 199 insertions(+), 17 deletions(-) rename packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/{ExoPlayerEventListenerTests.java => ExoPlayerEventListenerTest.java} (64%) diff --git a/packages/video_player/video_player_android/CHANGELOG.md b/packages/video_player/video_player_android/CHANGELOG.md index 5b0edb5398bc..8951fff9f512 100644 --- a/packages/video_player/video_player_android/CHANGELOG.md +++ b/packages/video_player/video_player_android/CHANGELOG.md @@ -1,7 +1,16 @@ +## 2.7.15 + +* Changes the rotation correction calculation for Android API 29+ to use + the one that is reported by the video's format instead of the unapplied + rotation degrees that Exoplayer does not report on Android API 21+. +* Changes the rotation correction calculation for Android APIs 21-28 to 0 + because the Impeller backend used on those API versions correctly rotates + the video being played automatically. + ## 2.7.14 * Removes SSL workaround for API 19, which is no longer supported. -* + ## 2.7.13 * When `AndroidVideoPlayer` attempts to operate on a `textureId` that is not diff --git a/packages/video_player/video_player_android/android/build.gradle b/packages/video_player/video_player_android/android/build.gradle index b8ceea25cd70..92decbdda718 100644 --- a/packages/video_player/video_player_android/android/build.gradle +++ b/packages/video_player/video_player_android/android/build.gradle @@ -57,6 +57,11 @@ android { unitTests.includeAndroidResources = true unitTests.returnDefaultValues = true unitTests.all { + // The org.gradle.jvmargs property that may be set in gradle.properties does not impact + // the Java heap size when running the Android unit tests. The following property here + // sets the heap size to a size large enough to run the robolectric tests across + // multiple SDK levels. + jvmArgs "-Xmx1g" testLogging { events "passed", "skipped", "failed", "standardOut", "standardError" outputs.upToDateWhen {false} diff --git a/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java b/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java index c8f2816571de..df6115fd5846 100644 --- a/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java +++ b/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java @@ -4,11 +4,15 @@ package io.flutter.plugins.videoplayer; +import android.os.Build; import androidx.annotation.NonNull; +import androidx.annotation.OptIn; +import androidx.media3.common.Format; import androidx.media3.common.PlaybackException; import androidx.media3.common.Player; import androidx.media3.common.VideoSize; import androidx.media3.exoplayer.ExoPlayer; +import java.util.Objects; final class ExoPlayerEventListener implements Player.Listener { private final ExoPlayer exoPlayer; @@ -16,6 +20,32 @@ final class ExoPlayerEventListener implements Player.Listener { private boolean isBuffering = false; private boolean isInitialized; + private enum RotationDegrees { + ROTATE_0(0), + ROTATE_90(90), + ROTATE_180(180), + ROTATE_270(270); + + private final int degrees; + + RotationDegrees(int degrees) { + this.degrees = degrees; + } + + public static RotationDegrees fromDegrees(int degrees) { + for (RotationDegrees rotationDegrees : RotationDegrees.values()) { + if (rotationDegrees.degrees == degrees) { + return rotationDegrees; + } + } + throw new IllegalArgumentException("Invalid rotation degrees specified: " + degrees); + } + + public int getDegrees() { + return this.degrees; + } + } + ExoPlayerEventListener(ExoPlayer exoPlayer, VideoPlayerCallbacks events) { this(exoPlayer, events, false); } @@ -49,23 +79,80 @@ private void sendInitialized() { int width = videoSize.width; int height = videoSize.height; if (width != 0 && height != 0) { - int rotationDegrees = videoSize.unappliedRotationDegrees; - // Switch the width/height if video was taken in portrait mode - if (rotationDegrees == 90 || rotationDegrees == 270) { + RotationDegrees reportedRotationCorrection = RotationDegrees.ROTATE_0; + + if (Build.VERSION.SDK_INT <= 21) { + // On API 21 and below, Exoplayer may not internally handle rotation correction + // and reports it through VideoSize.unappliedRotationDegrees. We may apply it to + // fix the case of upside-down playback. + try { + reportedRotationCorrection = + RotationDegrees.fromDegrees(videoSize.unappliedRotationDegrees); + rotationCorrection = + getRotationCorrectionFromUnappliedRotation(reportedRotationCorrection); + } catch (IllegalArgumentException e) { + // Unapplied rotation other than 0, 90, 180, 270 reported by VideoSize. Because this is unexpected, + // we apply no rotation correction. + reportedRotationCorrection = RotationDegrees.ROTATE_0; + rotationCorrection = 0; + } + } + // TODO(camsim99): Replace this with a call to `handlesCropAndRotation` when it is + // available in stable. https://github.com/flutter/flutter/issues/157198 + else if (Build.VERSION.SDK_INT < 29) { + // When the SurfaceTexture backend for Impeller is used, the preview should already + // be correctly rotated. + rotationCorrection = 0; + } else { + // The video's Format also provides a rotation correction that may be used to + // correct the rotation, so we try to use that to correct the video rotation + // when the ImageReader backend for Impeller is used. + rotationCorrection = getRotationCorrectionFromFormat(exoPlayer); + + try { + reportedRotationCorrection = RotationDegrees.fromDegrees(rotationCorrection); + } catch (IllegalArgumentException e) { + // Rotation correction other than 0, 90, 180, 270 reported by Format. Because this is unexpected, + // we apply no rotation correction. + reportedRotationCorrection = RotationDegrees.ROTATE_0; + rotationCorrection = 0; + } + } + + // Switch the width/height if video was taken in portrait mode and a rotation + // correction was detected. + if (reportedRotationCorrection == RotationDegrees.ROTATE_90 + || reportedRotationCorrection == RotationDegrees.ROTATE_270) { width = videoSize.height; height = videoSize.width; } - // Rotating the video with ExoPlayer does not seem to be possible with a Surface, - // so inform the Flutter code that the widget needs to be rotated to prevent - // upside-down playback for videos with rotationDegrees of 180 (other orientations work - // correctly without correction). - if (rotationDegrees == 180) { - rotationCorrection = rotationDegrees; - } } events.onInitialized(width, height, exoPlayer.getDuration(), rotationCorrection); } + private int getRotationCorrectionFromUnappliedRotation(RotationDegrees unappliedRotationDegrees) { + int rotationCorrection = 0; + + // Rotating the video with ExoPlayer does not seem to be possible with a Surface, + // so inform the Flutter code that the widget needs to be rotated to prevent + // upside-down playback for videos with unappliedRotationDegrees of 180 (other orientations + // work correctly without correction). + if (unappliedRotationDegrees == RotationDegrees.ROTATE_180) { + rotationCorrection = unappliedRotationDegrees.getDegrees(); + } + + return rotationCorrection; + } + + @OptIn(markerClass = androidx.media3.common.util.UnstableApi.class) + // A video's Format and its rotation degrees are unstable because they are not guaranteed + // the same implementation across API versions. It is possible that this logic may need + // revisiting should the implementation change across versions of the Exoplayer API. + private int getRotationCorrectionFromFormat(ExoPlayer exoPlayer) { + Format videoFormat = Objects.requireNonNull(exoPlayer.getVideoFormat()); + return videoFormat.rotationDegrees; + } + @Override public void onPlaybackStateChanged(final int playbackState) { switch (playbackState) { diff --git a/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/ExoPlayerEventListenerTests.java b/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/ExoPlayerEventListenerTest.java similarity index 64% rename from packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/ExoPlayerEventListenerTests.java rename to packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/ExoPlayerEventListenerTest.java index 1d00d31b8ee5..65dfb311c31c 100644 --- a/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/ExoPlayerEventListenerTests.java +++ b/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/ExoPlayerEventListenerTest.java @@ -12,6 +12,7 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import androidx.media3.common.Format; import androidx.media3.common.PlaybackException; import androidx.media3.common.Player; import androidx.media3.common.VideoSize; @@ -24,6 +25,7 @@ import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; /** * Unit tests for {@link ExoPlayerEventListener}. @@ -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 { @Mock private ExoPlayer mockExoPlayer; @Mock private VideoPlayerCallbacks mockCallbacks; private ExoPlayerEventListener eventListener; @@ -46,7 +48,8 @@ public void setUp() { } @Test - public void onPlaybackStateChangedReadySendInitialized() { + @Config(maxSdk = 28) + public void onPlaybackStateChangedReadySendInitialized_belowAndroid29() { VideoSize size = new VideoSize(800, 400, 0, 0); when(mockExoPlayer.getVideoSize()).thenReturn(size); when(mockExoPlayer.getDuration()).thenReturn(10L); @@ -56,7 +59,25 @@ public void onPlaybackStateChangedReadySendInitialized() { } @Test - public void onPlaybackStateChangedReadyInPortraitMode90DegreesSwapWidthAndHeight() { + @Config(minSdk = 29) + public void + onPlaybackStateChangedReadySendInitializedWithRotationCorrectionAndWidthAndHeightSwap_aboveAndroid29() { + VideoSize size = new VideoSize(800, 400, 0, 0); + int rotationCorrection = 90; + Format videoFormat = new Format.Builder().setRotationDegrees(rotationCorrection).build(); + + when(mockExoPlayer.getVideoSize()).thenReturn(size); + when(mockExoPlayer.getDuration()).thenReturn(10L); + when(mockExoPlayer.getVideoFormat()).thenReturn(videoFormat); + + eventListener.onPlaybackStateChanged(Player.STATE_READY); + verify(mockCallbacks).onInitialized(400, 800, 10L, rotationCorrection); + } + + @Test + @Config(maxSdk = 21) + public void + onPlaybackStateChangedReadyInPortraitMode90DegreesSwapWidthAndHeight_belowAndroid21() { VideoSize size = new VideoSize(800, 400, 90, 0); when(mockExoPlayer.getVideoSize()).thenReturn(size); when(mockExoPlayer.getDuration()).thenReturn(10L); @@ -66,7 +87,38 @@ public void onPlaybackStateChangedReadyInPortraitMode90DegreesSwapWidthAndHeight } @Test - public void onPlaybackStateChangedReadyInPortraitMode270DegreesSwapWidthAndHeight() { + @Config(minSdk = 22, maxSdk = 28) + public void + onPlaybackStateChangedReadyInPortraitMode90DegreesDoesNotSwapWidthAndHeight_aboveAndroid21belowAndroid29() { + VideoSize size = new VideoSize(800, 400, 90, 0); + + when(mockExoPlayer.getVideoSize()).thenReturn(size); + when(mockExoPlayer.getDuration()).thenReturn(10L); + + eventListener.onPlaybackStateChanged(Player.STATE_READY); + verify(mockCallbacks).onInitialized(800, 400, 10L, 0); + } + + @Test + @Config(minSdk = 29) + public void + onPlaybackStateChangedReadyInPortraitMode90DegreesSwapWidthAndHeight_aboveAndroid29() { + VideoSize size = new VideoSize(800, 400, 0, 0); + int rotationCorrection = 90; + Format videoFormat = new Format.Builder().setRotationDegrees(rotationCorrection).build(); + + when(mockExoPlayer.getVideoSize()).thenReturn(size); + when(mockExoPlayer.getDuration()).thenReturn(10L); + when(mockExoPlayer.getVideoFormat()).thenReturn(videoFormat); + + eventListener.onPlaybackStateChanged(Player.STATE_READY); + verify(mockCallbacks).onInitialized(400, 800, 10L, 90); + } + + @Test + @Config(maxSdk = 21) + public void + onPlaybackStateChangedReadyInPortraitMode270DegreesSwapWidthAndHeight_belowAndroid21() { VideoSize size = new VideoSize(800, 400, 270, 0); when(mockExoPlayer.getVideoSize()).thenReturn(size); when(mockExoPlayer.getDuration()).thenReturn(10L); @@ -76,7 +128,36 @@ public void onPlaybackStateChangedReadyInPortraitMode270DegreesSwapWidthAndHeigh } @Test - public void onPlaybackStateChangedReadyFlipped180DegreesInformEventHandler() { + @Config(minSdk = 22, maxSdk = 28) + public void + onPlaybackStateChangedReadyInPortraitMode270DegreesDoesNotSwapWidthAndHeight_aboveAndroid21belowAndroid29() { + VideoSize size = new VideoSize(800, 400, 270, 0); + when(mockExoPlayer.getVideoSize()).thenReturn(size); + when(mockExoPlayer.getDuration()).thenReturn(10L); + + eventListener.onPlaybackStateChanged(Player.STATE_READY); + verify(mockCallbacks).onInitialized(800, 400, 10L, 0); + } + + @Test + @Config(minSdk = 29) + public void + onPlaybackStateChangedReadyInPortraitMode270DegreesSwapWidthAndHeight_aboveAndroid29() { + VideoSize size = new VideoSize(800, 400, 0, 0); + int rotationCorrection = 270; + Format videoFormat = new Format.Builder().setRotationDegrees(rotationCorrection).build(); + + when(mockExoPlayer.getVideoSize()).thenReturn(size); + when(mockExoPlayer.getDuration()).thenReturn(10L); + when(mockExoPlayer.getVideoFormat()).thenReturn(videoFormat); + + eventListener.onPlaybackStateChanged(Player.STATE_READY); + verify(mockCallbacks).onInitialized(400, 800, 10L, 270); + } + + @Test + @Config(maxSdk = 21) + public void onPlaybackStateChangedReadyFlipped180DegreesInformEventHandler_belowAndroid21() { VideoSize size = new VideoSize(800, 400, 180, 0); when(mockExoPlayer.getVideoSize()).thenReturn(size); when(mockExoPlayer.getDuration()).thenReturn(10L); diff --git a/packages/video_player/video_player_android/pubspec.yaml b/packages/video_player/video_player_android/pubspec.yaml index 0eee37495de4..eaaec350813e 100644 --- a/packages/video_player/video_player_android/pubspec.yaml +++ b/packages/video_player/video_player_android/pubspec.yaml @@ -2,7 +2,7 @@ name: video_player_android description: Android implementation of the video_player plugin. repository: https://github.com/flutter/packages/tree/main/packages/video_player/video_player_android issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22 -version: 2.7.14 +version: 2.7.15 environment: sdk: ^3.5.0