Skip to content

SurfaceView inside Compose AndroidView often shown stretched/cropped on API 34 #1237

Closed

Description

@MykolaKosianchuk

Version

Media3 main branch

More version details

1.3.0, 1.2.1, 1.1.1

Devices that reproduce the issue

Pixel running on Android 14 with updated to 5 March 2024 (important)

Devices that do not reproduce the issue

Pixel running on Android 14 without updated to 5 March 2024

Reproducible in the demo app?

Not tested

Reproduction steps

Here is code that could help to reproduce.

class MainActivity : ComponentActivity() {
    @OptIn(UnstableApi::class)
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContent {
            val context = LocalContext.current

            val exoPlayer = remember { ExoPlayer.Builder(context).build() }

            DisposableEffect(Unit) {
                onDispose {
                    exoPlayer.release()
                }
            }

            Box(
                modifier = Modifier
                    .fillMaxWidth()
                    .fillMaxHeight(0.5f) //important to have
//                    .height(500.dp) - also reproducible
                    .background(Color.Black),
                contentAlignment = Alignment.Center
            ) {
                AndroidView(
                    factory = { ctx ->
                        PlayerView(ctx).apply {
                            useController = false
                            resizeMode = AspectRatioFrameLayout.RESIZE_MODE_ZOOM
                        }
                    },
                    update = {
                        it.player = exoPlayer
                        exoPlayer.setMediaItem(
                            MediaItem.fromUri("https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/BigBuckBunny.mp4")
                        )
                        exoPlayer.playWhenReady = true
                        exoPlayer.prepare()
                    },
                )
            }
        }
    }
}

Expected result

Media should be visible correctly

Actual result

Media shows with a wrong aspect ratio

Media

Here is how it looks

Screenshot 2024-04-02 at 15 52 20

Bug Report

Activity

  1. phcannesson commented on Apr 3, 2024

    @phcannesson
    Contributor

    +1 this
    We've spent days trying to understand issues we have with Android 14 devices recently.
    Going back to our previous app versions doesn't fix anything, so we came up with the same conclusion as @MykolaKosianchuk , it's probably related to a recent Android patch.

  2. MykolaKosianchuk commented on Apr 3, 2024

    @MykolaKosianchuk
    Author

    @phcannesson Yes, we found this issue and tried to reproduce it on the device without the last patch but we can't. However, once we updated the phone, the issue appeared.

  3. harry248 commented on Apr 3, 2024

    @harry248

    We have the same problem. We had already observed this in the QPR but unfortunately did not create an issue. Interestingly, we "only" have the problem if we use the AndroidView (which creates the PlayerView) in an if block, i.e. the composable is not called immediately.

  4. icbaker commented on Apr 8, 2024

    @icbaker
    Collaborator

    Reproducible in the demo app?

    Not tested

    Please can you test this? It would help us to debug if we know whether it can be reproduced in the demo app.


    Interestingly, we "only" have the problem if we use the AndroidView (which creates the PlayerView) in an if block, i.e. the composable is not called immediately.

    Is this true for everyone else that has commented on this bug? Is everyone else experiencing this only in Compose contexts, or also in conventional views? This looks like it might be a duplicate of #1123 but it's interesting to hear that the behaviour changes between Android 14 before the March 5th update and after it.

  5. phcannesson commented on Apr 8, 2024

    @phcannesson
    Contributor

    Reproducible in the demo app?

    Not tested

    Please can you test this? It would help us to debug if we know whether it can be reproduced in the demo app.

    Interestingly, we "only" have the problem if we use the AndroidView (which creates the PlayerView) in an if block, i.e. the composable is not called immediately.

    Is this true for everyone else that has commented on this bug? Is everyone else experiencing this only in Compose contexts, or also in conventional views? This looks like it might be a duplicate of #1123 but it's interesting to hear that the behaviour changes between Android 14 before the March 5th update and after it.

    On our side we're using Compose.
    We did not test with conventional views.

  6. promanowicz commented on Apr 8, 2024

    @promanowicz

    I have faced exact same issue, except I was using resizeMode with fixed height.
    I addition I have noticed that the video gets right aspect ratio after I switched orientation to landscape and then back to portrait.

  7. MykolaKosianchuk commented on Apr 8, 2024

    @MykolaKosianchuk
    Author

    @icbaker It is not reproducible in the demo app
    Looks like it is related to the compose

  8. speedclaud commented on Apr 11, 2024

    @speedclaud

    Hi @icbaker , I was also not able to reproduce the issue on your demo app on Pixel 7, Android 14, march 5th update, but we have the issue in our compose app using AndroidView. The issue is present in this sample app that uses compose, (press on the "Exo PlayerView" button):

    compose-media.zip

    This is what it looks like:

    Screenshot_20240411_121959

  9. 30 remaining items

  10. devno44 commented on Jul 30, 2024

    @devno44

    Is there any workaround for older versions ?
    We can not change the ExoPlayer version right now, and can not use TextureView instead of SurfaceView.

  11. icbaker commented on Jul 30, 2024

    @icbaker
    Collaborator

    @devno44 In order to backport the code change to an older version of the library you will need to build your chosen version locally (media3 instructions, ExoPlayer 2.19.0 instructions), and then apply the changes in 968f72f and 9980306 to your local copy of the code (note that in the exoplayer2, the equivalent of media3's PlayerView is StyledPlayerView.

  12. kotya341 commented on Jul 30, 2024

    @kotya341

    @oceanjules
    could you please reopen our thread, I can't reply on it
    This conversation has been locked and limited to collaborators.

  13. icbaker commented on Jul 30, 2024

    @icbaker
    Collaborator

    @kotya341 Looks like @tonihei has unlocked the thread and you should be able to reply - apologies for missing that when I re-opened it.

  14. phcannesson commented on Jul 30, 2024

    @phcannesson
    Contributor

    Should we also reopen this one or does 1.4.0 contain a valid fix ?

  15. icbaker commented on Jul 30, 2024

    @icbaker
    Collaborator

    @phcannesson This issue is fixed in 1.4.0-rc01. If you're experiencing similar problems on that version or later, please open a new issue with details & repro steps.

  16. Omkar-Amberkar commented on Aug 2, 2024

    @Omkar-Amberkar

    @icbaker how would we be able to replicate your fix when we aren't using the PlayerView directly but instead use the AspectRatioFrameLayout and SurfaceView inside the AndroidView. I have created something similar in compose for our use case as below. let me know if this looks good

    const val RESIZE_MODE_FIT = 0
    const val RESIZE_MODE_FIXED_WIDTH = 1
    const val RESIZE_MODE_FIXED_HEIGHT = 2
    const val RESIZE_MODE_FILL = 3
    const val RESIZE_MODE_ZOOM = 4
    
    private const val MAX_ASPECT_RATIO_DEFORMATION_FRACTION = 0.01f
    
    @Composable
    fun Surface(
        modifier: Modifier = Modifier,
        aspectRatio: Float = 16f / 9f,
        resizeMode: Int = RESIZE_MODE_FIT,
        onSurfaceCreated: (SurfaceView) -> Unit = {},
        onSurfaceDestroyed: (SurfaceView) -> Unit = {},
    ) {
        val mainLooperHandler = remember { Handler(Looper.getMainLooper()) }
        val surfaceSyncGroupV34 = remember {
            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
                SurfaceSyncGroupCompatV34()
            } else {
                null
            }
        }
        AspectRatioLayout(
            aspectRatio = aspectRatio,
            resizeMode = resizeMode,
            modifier = modifier
                .drawWithContent {
                    // Draw the existing content
                    drawContent()
    
                    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
                        surfaceSyncGroupV34?.maybeMarkSyncReadyAndClear()
                    }
                },
        ) {
            Box {
                AndroidExternalSurface(
                    content = { context, state ->
                        SurfaceView(context).apply {
                            state.onSurface { surface, width, height ->
                                onSurfaceCreated(this@apply)
                                surface.onChanged { width, height ->
                                    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
                                        surfaceSyncGroupV34?.postRegister(
                                            mainLooperHandler,
                                            this@apply,
                                            this@apply::invalidate
                                        )
                                    }
                                }
                                surface.onDestroyed {
                                    onSurfaceDestroyed(this@apply)
                                }
                            }
                            holder.addCallback(state)
                        }
                    }
                )
            }
        }
    }
    
    @Composable
    private fun AspectRatioLayout(
        modifier: Modifier,
        aspectRatio: Float,
        resizeMode: Int,
        content: @Composable () -> Unit,
    ) {
        if (aspectRatio <= 0) return
        Layout(
            content = content,
            modifier = modifier
        ) { measurables, constraints ->
            var width = constraints.maxWidth
            var height = constraints.maxHeight
            val viewAspectRatio = width.toFloat() / height
            val aspectDeformation = aspectRatio / viewAspectRatio - 1
            if (abs(aspectDeformation) > MAX_ASPECT_RATIO_DEFORMATION_FRACTION) {
                when (resizeMode) {
                    RESIZE_MODE_FIXED_WIDTH -> height = (width / aspectRatio).toInt()
                    RESIZE_MODE_FIXED_HEIGHT -> width = (height * aspectRatio).toInt()
                    RESIZE_MODE_ZOOM -> if (aspectDeformation > 0) {
                        width = (height * aspectRatio).toInt()
                    } else {
                        height = (width / aspectRatio).toInt()
                    }
    
                    RESIZE_MODE_FIT -> if (aspectDeformation > 0) {
                        height = (width / aspectRatio).toInt()
                    } else {
                        width = (height * aspectRatio).toInt()
                    }
                }
            }
    
            layout(width, height) {
                val childConstraints = constraints.copy(
                    minWidth = 0,
                    minHeight = 0,
                    maxWidth = width,
                    maxHeight = height
                )
                measurables.forEach { measurable ->
                    val placeable = measurable.measure(childConstraints)
                    placeable.placeRelative(0, 0)
                }
            }
        }
    }
    
    
    @RequiresApi(34)
    private class SurfaceSyncGroupCompatV34 {
        private var surfaceSyncGroup: SurfaceSyncGroup? = null
    
        fun postRegister(
            mainLooperHandler: Handler,
            surfaceView: SurfaceView,
            invalidate: Runnable
        ) {
            mainLooperHandler.post {
                val rootSurfaceControl = surfaceView.rootSurfaceControl ?: return@post
                surfaceSyncGroup = SurfaceSyncGroup("exo-sync-b-334901521")
                    .apply { add(rootSurfaceControl) {} }
                invalidate.run()
                rootSurfaceControl.applyTransactionOnDraw(SurfaceControl.Transaction())
            }
        }
    
        fun maybeMarkSyncReadyAndClear() {
            surfaceSyncGroup?.markSyncReady()
        }
    }
    
  17. icbaker commented on Aug 5, 2024

    @icbaker
    Collaborator

    Heads up for those subscribed to this thread: We have received a report (#1594) that the workaround we introduced in 30cb762 has introduced a regression in the behaviour of Shared Element Transitions. We are currently investigating this, and will update #1594 when we have more info.


    @Omkar-Amberkar Does it work? That's probably the best/first test. In general we don't have the capacity for 1:1 code review, and we are also not the experts in these graphics APIs - so I'm afraid not best placed to debug your integration with them, if you're unable to adapt our published workaround to your needs.

  18. Omkar-Amberkar commented on Aug 5, 2024

    @Omkar-Amberkar

    @icbaker yes it does but wanted to check in with you guys to see if that seems correct when using compose.

  19. efemoney commented on Aug 9, 2024

    @efemoney

    @Omkar-Amberkar it is correct. It does the exact same thing. I have the same workaround in my code.

  20. locked and limited conversation to collaborators on Aug 19, 2024
  21. unlocked this conversation on Sep 4, 2024
  22. icbaker commented on Sep 4, 2024

    @icbaker
    Collaborator

    I'm afraid that due to the unintended side-effects of the workaround reported in #1594 (for non-Compose use-cases), I've had to make the workaround opt-in (rather than automatically enabled) in 87bd9ba.

    This means that those of you using PlayerView inside AndroidView will need to call playerView.setEnableComposeSurfaceSyncWorkaround(true) to enable the workaround. This change will probably go live in 1.5.0-beta01.

  23. locked and limited conversation to collaborators on Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions