Skip to content

[video_player_android] Fix incorrect dimensions swap #9199

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/video_player/video_player_android/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.8.3

* Fixes incorrect width/height swap ([bug](https://github.com/flutter/flutter/issues/166097)). The swap was originally required for the uncorrected width/height of `Format` but was mistakenly retained after [switching to `VideoSize`](https://github.com/flutter/packages/pull/6535), which already accounts for rotation.

## 2.8.2

* Fixes a [bug](https://github.com/flutter/flutter/issues/164689) that can cause video to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,70 +30,58 @@ public TextureExoPlayerEventListener(
@Override
protected void sendInitialized() {
VideoSize videoSize = exoPlayer.getVideoSize();
int rotationCorrection = 0;
RotationDegrees rotationCorrection = RotationDegrees.ROTATE_0;
int width = videoSize.width;
int height = videoSize.height;
if (width != 0 && height != 0) {
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 unappliedRotation =
RotationDegrees.fromDegrees(videoSize.unappliedRotationDegrees);
rotationCorrection =
getRotationCorrectionFromUnappliedRotation(reportedRotationCorrection);
rotationCorrection = getRotationCorrectionFromUnappliedRotation(unappliedRotation);
} 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;
rotationCorrection = RotationDegrees.ROTATE_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;
rotationCorrection = RotationDegrees.ROTATE_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);
int rawVideoFormatRotation = getRotationCorrectionFromFormat(exoPlayer);

try {
reportedRotationCorrection = RotationDegrees.fromDegrees(rotationCorrection);
rotationCorrection = RotationDegrees.fromDegrees(rawVideoFormatRotation);
} 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;
rotationCorrection = RotationDegrees.ROTATE_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;
}
}
events.onInitialized(width, height, exoPlayer.getDuration(), rotationCorrection);
events.onInitialized(width, height, exoPlayer.getDuration(), rotationCorrection.getDegrees());
}

private int getRotationCorrectionFromUnappliedRotation(RotationDegrees unappliedRotationDegrees) {
int rotationCorrection = 0;
private RotationDegrees getRotationCorrectionFromUnappliedRotation(
RotationDegrees unappliedRotationDegrees) {
RotationDegrees rotationCorrection = RotationDegrees.ROTATE_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();
rotationCorrection = unappliedRotationDegrees;
}

return rotationCorrection;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void onPlaybackStateChangedReadySendInitialized_belowAndroid29() {
when(mockExoPlayer.getVideoFormat()).thenReturn(videoFormat);

eventListener.onPlaybackStateChanged(Player.STATE_READY);
verify(mockCallbacks).onInitialized(400, 800, 10L, rotationCorrection);
verify(mockCallbacks).onInitialized(800, 400, 10L, rotationCorrection);
}

@Test
Expand All @@ -78,7 +78,7 @@ public void onPlaybackStateChangedReadySendInitialized_belowAndroid29() {
when(mockExoPlayer.getDuration()).thenReturn(10L);

eventListener.onPlaybackStateChanged(Player.STATE_READY);
verify(mockCallbacks).onInitialized(400, 800, 10L, 0);
verify(mockCallbacks).onInitialized(800, 400, 10L, 0);
}

@Test
Expand Down Expand Up @@ -107,7 +107,7 @@ public void onPlaybackStateChangedReadySendInitialized_belowAndroid29() {
when(mockExoPlayer.getVideoFormat()).thenReturn(videoFormat);

eventListener.onPlaybackStateChanged(Player.STATE_READY);
verify(mockCallbacks).onInitialized(400, 800, 10L, 90);
verify(mockCallbacks).onInitialized(800, 400, 10L, 90);
}

@Test
Expand All @@ -119,7 +119,7 @@ public void onPlaybackStateChangedReadySendInitialized_belowAndroid29() {
when(mockExoPlayer.getDuration()).thenReturn(10L);

eventListener.onPlaybackStateChanged(Player.STATE_READY);
verify(mockCallbacks).onInitialized(400, 800, 10L, 0);
verify(mockCallbacks).onInitialized(800, 400, 10L, 0);
}

@Test
Expand Down Expand Up @@ -147,7 +147,7 @@ public void onPlaybackStateChangedReadySendInitialized_belowAndroid29() {
when(mockExoPlayer.getVideoFormat()).thenReturn(videoFormat);

eventListener.onPlaybackStateChanged(Player.STATE_READY);
verify(mockCallbacks).onInitialized(400, 800, 10L, 270);
verify(mockCallbacks).onInitialized(800, 400, 10L, 270);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import 'dart:async';
import 'dart:io';
import 'dart:math';

import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
Expand Down Expand Up @@ -467,12 +466,13 @@ class _VideoPlayerWithRotation extends StatelessWidget {

@override
Widget build(BuildContext context) {
return rotation == 0
? child
: Transform.rotate(
angle: rotation * pi / 180,
child: child,
);
if (rotation == 0) {
return child;
}
return RotatedBox(
quarterTurns: rotation ~/ 90,
child: child,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Author

Choose a reason for hiding this comment

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

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.

}
}

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.8.2
version: 2.8.3

environment:
sdk: ^3.6.0
Expand Down