-
Notifications
You must be signed in to change notification settings - Fork 81
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
Fix PiP aspect ratio issue #57
Conversation
Fix for PiP aspect ratio issue
Tested with a Pixel Fold - not seeing a crash when trying landscape playback now, thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor changes requested
app/src/main/java/com/google/android/samples/socialite/ui/player/VideoPlayerScreen.kt
Outdated
Show resolved
Hide resolved
new fix based off of the player's video size
Spotless
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM |
// set source rect hint, aspect ratio | ||
if (player != null && player.videoSize.width > 0 && player.videoSize.height > 0) { | ||
val sourceRect = layoutCoordinates.boundsInWindow().toAndroidRectF().toRect() | ||
builder.setSourceRectHint(sourceRect) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think ideally, this source rect hint would also be set based on the playback surface's bounds - right? I'm not sure how we can do that though, since the PlayerView is created after we set up the pip modifier, and we use the pip modifier when declaring the PlayerView (i.e. it seems like a chicken and egg situation).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a similar thought - it is definitely worth looking into!
Tested with both a landscape and a portrait video, in both landscape and portrait device orientations - fix LGTM, thank you! |
Fixing the PiP issue where the app crashes when the app starts playing a video from landscape mode.