androidx / media

Jetpack Media3 support libraries for media use cases, including ExoPlayer, an extensible media player for Android
https://developer.android.com/media/media3
Apache License 2.0
1.67k stars 395 forks source link

[PlayerView] Shared element transitions not work on 1.4.0 #1594

Closed onlymash closed 1 month ago

onlymash commented 2 months ago

Version

Media3 1.4.0

More version details

After updating media3 to version 1.4.0, Shared element transitions fail. Specifically, PlayerView does not scale properly on Android 14 and above. The issue does not occur on Android 13. Reverting back to version 1.3.1 resolves the problem.

Devices that reproduce the issue

Devices that do not reproduce the issue

Reproducible in the demo app?

Not tested

Reproduction steps

none

Expected result

none

Actual result

none

Media

example 1.3.1.webm 1.4.0.webm

Bug Report

onlymash commented 2 months ago

This may be caused by this commit https://github.com/androidx/media/commit/30cb76269a67e09f6e1662ea9ead6aac70667028

icbaker commented 2 months ago

This may be caused by this commit 30cb762

That commit shouldn't affect Android 15 (SDK_INT == 35), because all the logic is gated by a SDK_INT == 34 check, but you said that Android 15 did reproduce the issue. Please can you check that repro case again, and confirm the value of Build.VERSION.SDK_INT that you see?

If you are able to build locally and revert that change on top of 1.4.0 to confirm it as the culprit, that would also be very helpful.

onlymash commented 2 months ago

This may be caused by this commit 30cb762

That commit shouldn't affect Android 15 (SDK_INT == 35), because all the logic is gated by a SDK_INT == 34 check, but you said that Android 15 did reproduce the issue. Please can you check that repro case again, and confirm the value of Build.VERSION.SDK_INT that you see?

If you are able to build locally and revert that change on top of 1.4.0 to confirm it as the culprit, that would also be very helpful.

OnePlus 12's A15 beta is an early version, and the SDK_INT is still 34

onlymash commented 2 months ago

This may be caused by this commit 30cb762

That commit shouldn't affect Android 15 (SDK_INT == 35), because all the logic is gated by a SDK_INT == 34 check, but you said that Android 15 did reproduce the issue. Please can you check that repro case again, and confirm the value of Build.VERSION.SDK_INT that you see?

If you are able to build locally and revert that change on top of 1.4.0 to confirm it as the culprit, that would also be very helpful.

I reverted this commit and build it locally, and I can confirm that this commit caused it.

icbaker commented 2 months ago

Thank for you for confirming.

Are you able to explain your repro set-up in more detail? You mention 'shared element transitions' - is that referring to this Compose feature? https://developer.android.com/develop/ui/compose/animation/shared-elements

If you have a code snippet describing how your UI is wired together, or even a minimal reproducible example that demonstrates the problem in a way that we can build locally, that would be really helpful.

The easiest way to do this is probably to fork this project and make changes in one of the demo apps, then link us to the fork.

Alternatively it could be an Android Studio project zipped up and sent to android-media-github@google.com with the subject "Issue #1594". If emailing please also update this issue to indicate you’ve done this.

onlymash commented 2 months ago

Thank for you for confirming.

Are you able to explain your repro set-up in more detail? You mention 'shared element transitions' - is that referring to this Compose feature? https://developer.android.com/develop/ui/compose/animation/shared-elements

If you have a code snippet describing how your UI is wired together, or even a minimal reproducible example that demonstrates the problem in a way that we can build locally, that would be really helpful.

The easiest way to do this is probably to fork this project and make changes in one of the demo apps, then link us to the fork.

Alternatively it could be an Android Studio project zipped up and sent to android-media-github@google.com with the subject "Issue #1594". If emailing please also update this issue to indicate you’ve done this.

Activity to activity (views), not compose. Example: https://github.com/material-components/material-components-android/blob/master/catalog/java/io/material/catalog/transition/TransitionContainerTransformStartDemoActivity.java

https://developer.android.com/develop/ui/views/animations/transitions/start-activity#start-with-element

My use case is imageview to playerview, they have the same transitionName

icbaker commented 2 months ago

@onlymash I'm afraid investigating this further is blocked on us reproducing the issue. I'm not sure when I'll have the time to build a repro example from scratch (especially since I'm not very familiar with the shared element transitions you're referring to). If you are able to put together an example project that reproduces the issue that would speed up the investigation.

onlymash commented 2 months ago

@onlymash I'm afraid investigating this further is blocked on us reproducing the issue. I'm not sure when I'll have the time to build a repro example from scratch (especially since I'm not very familiar with the shared element transitions you're referring to). If you are able to put together an example project that reproduces the issue that would speed up the investigation.

https://github.com/onlymash/shared-element-demo

icbaker commented 2 months ago

Thank you, I can reproduce a difference in behaviour when toggling the media3 dependency in your project between 1.3.0 and 1.4.0.

I have passed this on to the graphics team who helped us with the original workaround for the compose issue (https://github.com/androidx/media/issues/1237).

nikclayton commented 1 month ago

I can reproduce this at will in Pachli (https://github.com/pachli/pachli-android). Pachli shows a social media feed that may contain videos. In the feed the video is represented as a still image and if the user taps the image a new activity is launched, with a shared element transition from the image to the video.

This works fine up to and including 1.4.0-beta01. Here's a video showing what it's supposed to look like:

https://github.com/user-attachments/assets/6c450ff6-6391-42de-a17e-46f20bbcc56b

Starting from 1.4.0-rc01 this is broken. Instead of smoothly transitioning the layout "sticks" for a second or more, before immediately switching to the new layout. There's no transition.

Here's a video showing what that looks like.

https://github.com/user-attachments/assets/9bf70b24-05ae-4d99-ba2a-4f8a57e80587

Both videos from a Pixel 4a 5G, Android version: 14, SDK level 34

I found this issue from https://github.com/androidx/media/issues/1237, which is about an issue with Compose layouts. However, Pachli uses XML layouts, not Compose.

If I modify the androidx.media3.ui.PlayerView element in the layout and add app:surface_type="texture_view" (this attribute was previously not necessary, so I was getting the default surface_view) I get the expected behaviour, as in the first video above.

To confirm, I also tried setting app:surface_type="surface_view", and as expected, I get the buggy behaviour in 1.4.0-rc01, and it's still fine on 1.4.0-beta01 and below.

https://github.com/pachli/pachli-android/issues/920 is a report of this issue from a Pachli user which has another video showing the problem. That's on a Pixel 6a, Android version: 14, SDK level: 34

icbaker commented 1 month ago

I've changed the Compose workaround to be opt-in, which should fix the issue reported here - thanks for reporting. This change should be included in 1.5.0-beta01.

nikclayton commented 1 month ago

Can the 1.4.0 and 1.4.1 release notes please be updated to reference this issue.