search

twilio / video-quickstart-android

Twilio Video Quickstart for Android
MIT License
212 stars 159 forks source link

Application crashes on device orientation changes when the application targets to target sdk version of 34 #753

Closed vijimani17 closed 5 months ago

vijimani17 commented 5 months ago

Description

Application crashes on device orientation changes when the application targets to target sdk version of 34

Steps to Reproduce

Sample twilio app tested - https://drive.google.com/file/d/1i3dfaglxS5Nth3PFQRHnsa83BHN4APeI/view

PF video link: https://drive.google.com/file/d/14AUcIzbeMOQdn8dg2kXijDhfsUiEZUVF/view

Expected Behavior

App stays the remote view sharing even on configuration changes

Actual Behavior

App got crash

Reproduces how Often

100%

Code

val options = ConnectOptions.Builder(accessToken)

.roomName(roomId)

.enableNetworkQuality(true)

.encodingParameters(EncodingParameters(16, 0))

.build()

val activeRoom = Video.connect(context, options, this)

This code will be invoked when user accepts the permission

fun startScreenShare(mediaInputResult: ActivityResult) {

val screenCapturer = ScreenCapturer(context, mediaInputResult.resultCode, mediaInputResult.data!!, listener)

val localVideoTrack = LocalVideoTrack.create(context, true, screenCapturer)

if (localVideoTrack != null) {

activeRoom?.localParticipant?.publishTrack(localVideoTrack)

}

}

This code will be invoked to disconnect the room

activeRoom?.disconnect()

activeRoom = null

Initial investigation from our end added here https://docs.google.com/document/d/1NZ09DFuEZQs3RA3RYVDbIS4gJxzOXSUht15Oa948XLw/edit

Logs

Twilio crash on sample app can be seen in https://docs.google.com/document/d/1NZ09DFuEZQs3RA3RYVDbIS4gJxzOXSUht15Oa948XLw/edit -> "Twilio Sample app crash"

Video Android SDK

7.6.4

Android API

TARGET_SDK = 34

Android SDK: 34, Release: 14

Android Device

Pixel 6A

Note : Since we are planning to update our application target sdk, This is a blocker issue for us to proceed further. Copy of Remote Access - Android 14 feature testing.pdf

ocarevs commented 5 months ago

Hey @vijimani17 we've merged a fix in the ScreenShare example that handles API 34 onwards behaviour https://github.com/twilio/video-quickstart-android/pull/754

vijimani17 commented 5 months ago

@ocarevs Fix seems to be provided in the sample test app , Does any fix provided twilio library ??,

We expect the fix to be provided in the twilio library based on android recommendation. so I wonder how this fix will fix the problem what we see in our application because we wont be asking user permission on configuration changes.

With the existing media projection instance , createVirtualDisplay api should not be called again instead we have to follow the below

If your app needs to invoke MediaProjection#createVirtualDisplay to handle configuration changes (such as the screen orientation or screen size changing), you can follow these steps to update the VirtualDisplay for the existing MediaProjection instance:

Invoke VirtualDisplay#resize with the new width and height. Provide a new Surface with the new width and height to VirtualDisplay#setSurface.

please refer https://developer.android.com/about/versions/14/behavior-changes-14#configuration-changes

can you please check how to fix the problem without asking the user permission again on configuration changes. thanks

afalls-twilio commented 5 months ago

@vijimani17 My understanding is that there is no longer a crash in the QuickStart when using API 34. No fix is necessary to the Twilio-Video library, activity configuration changes would at best be handled in the QuickStart or your application. The sdk today allows you to not only create a capturer how you feel fit, our default provided implementations work today and must work across many older version of the Android SDK. Furthermore, the QuickStart is not an end-all be all, it is a demo of how one could implement it screen share. If you would like to implement the handling of screen share differently in your app, feel free to, either way it is outside the scope of the SDK and the QuickStart.

geak commented 4 months ago

I agree with @vijimani17, #754 doesn't fix the issue in Twilio library, it just fixes "Quick Start" sample app, though it does even fix the sample app, it just introduces workaround. Twilio library must be fixed until we fully transition to another library by the end of this year.

java.lang.RuntimeException: java.lang.SecurityException: Don't re-use the resultData to retrieve the same projection instance, and don't use a token that has timed out. Don't take multiple captures by invoking MediaProjection#createVirtualDisplay multiple times on the same instance. at tvi.webrtc.ThreadUtils.invokeAtFrontUninterruptibly(ThreadUtils.java:156) at tvi.webrtc.ThreadUtils.invokeAtFrontUninterruptibly(ThreadUtils.java:196) at tvi.webrtc.ScreenCapturerAndroid.changeCaptureFormat(ScreenCapturerAndroid.java:181) at com.twilio.video.ScreenCapturer$2.onFrameCaptured(ScreenCapturer.java:97) at tvi.webrtc.ScreenCapturerAndroid.onFrame(ScreenCapturerAndroid.java:201) at tvi.webrtc.SurfaceTextureHelper.tryDeliverTextureFrame(SurfaceTextureHelper.java:370) at tvi.webrtc.SurfaceTextureHelper.lambda$new$0$tvi-webrtc-SurfaceTextureHelper(SurfaceTextureHelper.java:207) at tvi.webrtc.SurfaceTextureHelper$$ExternalSyntheticLambda7.onFrameAvailable(D8$$SyntheticClass:0) at android.graphics.SurfaceTexture$1.handleMessage(SurfaceTexture.java:214) at android.os.Handler.dispatchMessage(Handler.java:106) at android.os.Looper.loopOnce(Looper.java:205) at android.os.Looper.loop(Looper.java:294) at android.os.HandlerThread.run(HandlerThread.java:67) Caused by: java.lang.SecurityException: Don't re-use the resultData to retrieve the same projection instance, and don't use a token that has timed out. Don't take multiple captures by invoking MediaProjection#createVirtualDisplay multiple times on the same instance. at android.os.Parcel.createExceptionOrNull(Parcel.java:3057) at android.os.Parcel.createException(Parcel.java:3041) at android.os.Parcel.readException(Parcel.java:3024) at android.os.Parcel.readException(Parcel.java:2966) at android.hardware.display.IDisplayManager$Stub$Proxy.createVirtualDisplay(IDisplayManager.java:1425) at android.hardware.display.DisplayManagerGlobal.createVirtualDisplay(DisplayManagerGlobal.java:643) at android.hardware.display.DisplayManager.createVirtualDisplay(DisplayManager.java:1134) at android.media.projection.MediaProjection.createVirtualDisplay(MediaProjection.java:247) at android.media.projection.MediaProjection.createVirtualDisplay(MediaProjection.java:216) at tvi.webrtc.ScreenCapturerAndroid.createVirtualDisplay(ScreenCapturerAndroid.java:192) at tvi.webrtc.ScreenCapturerAndroid.-$$Nest$mcreateVirtualDisplay(Unknown Source:0) at tvi.webrtc.ScreenCapturerAndroid$2.run(ScreenCapturerAndroid.java:185) at tvi.webrtc.ThreadUtils$4.call(ThreadUtils.java:199) at tvi.webrtc.ThreadUtils$4.call(ThreadUtils.java:196) at tvi.webrtc.ThreadUtils.invokeAtFrontUninterruptibly(ThreadUtils.java:154) at tvi.webrtc.ThreadUtils.invokeAtFrontUninterruptibly(ThreadUtils.java:196)  at tvi.webrtc.ScreenCapturerAndroid.changeCaptureFormat(ScreenCapturerAndroid.java:181)  at com.twilio.video.ScreenCapturer$2.onFrameCaptured(ScreenCapturer.java:97)  at tvi.webrtc.ScreenCapturerAndroid.onFrame(ScreenCapturerAndroid.java:201)  at tvi.webrtc.SurfaceTextureHelper.tryDeliverTextureFrame(SurfaceTextureHelper.java:370)  at tvi.webrtc.SurfaceTextureHelper.lambda$new$0$tvi-webrtc-SurfaceTextureHelper(SurfaceTextureHelper.java:207)  at tvi.webrtc.SurfaceTextureHelper$$ExternalSyntheticLambda7.onFrameAvailable(D8$$SyntheticClass:0)  at android.graphics.SurfaceTexture$1.handleMessage(SurfaceTexture.java:214)  at android.os.Handler.dispatchMessage(Handler.java:106)  at android.os.Looper.loopOnce(Looper.java:205)  at android.os.Looper.loop(Looper.java:294)  at android.os.HandlerThread.run(HandlerThread.java:67)  Caused by: android.os.RemoteException: Remote stack trace: at com.android.server.media.projection.MediaProjectionManagerService$MediaProjection.isValid(MediaProjectionManagerService.java:1094) at com.android.server.display.DisplayManagerService.createVirtualDisplayInternal(DisplayManagerService.java:1447) at com.android.server.display.DisplayManagerService.-$$Nest$mcreateVirtualDisplayInternal(DisplayManagerService.java:0) at com.android.server.display.DisplayManagerService$BinderService.createVirtualDisplay(DisplayManagerService.java:3706) at android.hardware.display.IDisplayManager$Stub.onTransact(IDisplayManager.java:730)

This must be fixed by invoking VirtualDisplay#resize method according to Android documentation.

geak commented 4 months ago

@vijimani17, seems I was able to work around the issue. I have made my own ScreenCapturer implementation and have commented out this piece of code in observer's onFrameCaptured method:

                    videoDimensions = new VideoDimensions(buffer.getWidth(), buffer.getHeight());
                    int currentOrientation = getDeviceOrientation();
                    if (updateCaptureDimensions(orientation, currentOrientation)) {
                        // Swap width and height capture dimensions due to orientation update
                        Log.d("ScreenCapturer", "Swapping width and height of frame due to orientation");
                        orientation = currentOrientation;
                        webRtcScreenCapturer.changeCaptureFormat(
                                buffer.getHeight(), buffer.getWidth(), (int) (1000 / frameDelayMS));
                    }

The idea is to prevent ScreenCapturer from re-creating a VirtualDisplay on screen rotations. Can you try this approach and ping back with results please?

vijimani17 commented 4 months ago

Hi @geak ,

Thank you for jumping in and confirm twilio has to fix the issue. I appreciate it.

We might be having work around, But fix should present in library should be the right approach.

For example , If there are 1000 clients , every client implementing work around is not the suggestable solution. Rather library is the one doing screen capture so I would say fix from twilio library is the right approach until clients' migrate to different video library by considering the android OS update time lines.

I would still request twilio team to fix and its going to be a simple one in my assumption.

Thanks.

geak commented 4 months ago

Hello, yeah, I understand that you want it to be fixed in the library. I also have 1000+ clients who want to use screen sharing in our app. But as far as I understand Twilio is not going to fix the issue in their library unfortunately :( Especially considering the end of life of the library..