Skip to content
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_android] Correct rotation of videos recorded by the camera #7846

Merged
merged 16 commits into from
Oct 21, 2024
Merged
9 changes: 9 additions & 0 deletions packages/video_player/video_player_android/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
## 2.7.14

* 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.13

* When `AndroidVideoPlayer` attempts to operate on a `textureId` that is not
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -49,23 +53,59 @@ 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) {
int reportedRotationCorrection = 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.
reportedRotationCorrection = videoSize.unappliedRotationDegrees;
rotationCorrection = getRotationCorrectionFromUnappliedRotation(reportedRotationCorrection);
} else if (Build.VERSION.SDK_INT < 29) {
camsim99 marked this conversation as resolved.
Show resolved Hide resolved
// When the SurfaceTexture backend for Impeller is used, the preview should already
// be correctly rotated.
rotationCorrection = 0;
} else {
// Above API 21, Exoplayer handles the VideoSize.unappliedRotationDegrees
camsim99 marked this conversation as resolved.
Show resolved Hide resolved
// correction internally. However, 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);
reportedRotationCorrection = rotationCorrection;
}

// Switch the width/height if video was taken in portrait mode and a rotation
// correction was detected.
if (reportedRotationCorrection == 90 || reportedRotationCorrection == 270) {
camsim99 marked this conversation as resolved.
Show resolved Hide resolved
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(int unappliedRotationDegrees) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 enum

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 == 180) {
rotationCorrection = unappliedRotationDegrees;
}

return rotationCorrection;
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

// correction.
private int getRotationCorrectionFromFormat(ExoPlayer exoPlayer) {
Format videoFormat = Objects.requireNonNull(exoPlayer.getVideoFormat());
return videoFormat.rotationDegrees;
}

@Override
public void onPlaybackStateChanged(final int playbackState) {
switch (playbackState) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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}.
Expand All @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@Mock private ExoPlayer mockExoPlayer;
@Mock private VideoPlayerCallbacks mockCallbacks;
private ExoPlayerEventListener eventListener;
Expand All @@ -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);
Expand All @@ -56,7 +59,25 @@ public void onPlaybackStateChangedReadySendInitialized() {
}

@Test
public void onPlaybackStateChangedReadyInPortraitMode90DegreesSwapWidthAndHeight() {
@Config(sdk = 29)
camsim99 marked this conversation as resolved.
Show resolved Hide resolved
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(sdk = 21)
camsim99 marked this conversation as resolved.
Show resolved Hide resolved
public void
onPlaybackStateChangedReadyInPortraitMode90DegreesSwapWidthAndHeight_belowAndroid21() {
VideoSize size = new VideoSize(800, 400, 90, 0);
when(mockExoPlayer.getVideoSize()).thenReturn(size);
when(mockExoPlayer.getDuration()).thenReturn(10L);
Expand All @@ -66,7 +87,38 @@ public void onPlaybackStateChangedReadyInPortraitMode90DegreesSwapWidthAndHeight
}

@Test
public void onPlaybackStateChangedReadyInPortraitMode270DegreesSwapWidthAndHeight() {
@Config(sdk = 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(sdk = 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(sdk = 21)
public void
onPlaybackStateChangedReadyInPortraitMode270DegreesSwapWidthAndHeight_belowAndroid21() {
VideoSize size = new VideoSize(800, 400, 270, 0);
when(mockExoPlayer.getVideoSize()).thenReturn(size);
when(mockExoPlayer.getDuration()).thenReturn(10L);
Expand All @@ -76,7 +128,36 @@ public void onPlaybackStateChangedReadyInPortraitMode270DegreesSwapWidthAndHeigh
}

@Test
public void onPlaybackStateChangedReadyFlipped180DegreesInformEventHandler() {
@Config(sdk = 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(sdk = 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(sdk = 21)
public void onPlaybackStateChangedReadyFlipped180DegreesInformEventHandler_belowAndroid21() {
VideoSize size = new VideoSize(800, 400, 180, 0);
when(mockExoPlayer.getVideoSize()).thenReturn(size);
when(mockExoPlayer.getDuration()).thenReturn(10L);
Expand Down
2 changes: 1 addition & 1 deletion packages/video_player/video_player_android/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.13
version: 2.7.14

environment:
sdk: ^3.5.0
Expand Down