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

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

Closed
1 task
onlymash opened this issue Aug 3, 2024 · 12 comments
Closed
1 task

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

onlymash opened this issue Aug 3, 2024 · 12 comments
Assignees

Comments

@onlymash
Copy link

onlymash commented Aug 3, 2024

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

  • OnePlus 12 running Android 15 beta
  • Lenovo Legion Tab running Android 14

Devices that do not reproduce the issue

  • Lenovo Legion Tab running Android 13

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

  • You will email the zip file produced by adb bugreport to [email protected] after filing this issue.
@onlymash
Copy link
Author

onlymash commented Aug 5, 2024

This may be caused by this commit
30cb762

@icbaker
Copy link
Collaborator

icbaker commented Aug 5, 2024

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.

@icbaker icbaker self-assigned this Aug 5, 2024
@onlymash
Copy link
Author

onlymash commented Aug 5, 2024

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
Copy link
Author

onlymash commented Aug 5, 2024

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
Copy link
Collaborator

icbaker commented Aug 5, 2024

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 [email protected] with the subject "Issue #1594". If emailing please also update this issue to indicate you’ve done this.

@onlymash
Copy link
Author

onlymash commented Aug 5, 2024

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 [email protected] 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
Copy link
Collaborator

icbaker commented Aug 21, 2024

@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
Copy link
Author

@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
Copy link
Collaborator

icbaker commented Aug 29, 2024

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 (#1237).

@nikclayton
Copy link

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:

media3-bug-textview_view.mp4

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.

media3-bug-surfaceview_view.mp4

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

I found this issue from #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.

pachli/pachli-android#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

nikclayton added a commit to pachli/pachli-android that referenced this issue Sep 3, 2024
androidx.media3 1.4.0-rc01 and above (at the time of writing) has a
bug that breaks shared element transitions with a `PlayerView` (see
androidx/media#1594).

This can be worked around by setting `app:surface_type="texture_view"`.
This uses more power, but for the typical length of social media videos
this shouldn't be a problem at the moment.

Fixes #920.
nikclayton added a commit to pachli/pachli-android that referenced this issue Sep 3, 2024
androidx.media3 1.4.0-rc01 and above (at the time of writing) has a bug
that breaks shared element transitions with a `PlayerView` (see
androidx/media#1594).

This can be worked around by setting `app:surface_type="texture_view"`.
This uses more power, but for the typical length of social media videos
this shouldn't be a problem at the moment.

Fixes #920.
copybara-service bot pushed a commit that referenced this issue Sep 4, 2024
The workaround causes issues with XML-based shared transitions, so we
can't apply it unilaterally.

Issue: #1594

Issue: #1237
PiperOrigin-RevId: 670960693
@icbaker
Copy link
Collaborator

icbaker commented Sep 4, 2024

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.

@icbaker icbaker closed this as completed Sep 4, 2024
@nikclayton
Copy link

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

@androidx androidx locked and limited conversation to collaborators Nov 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants