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

Detect screenshare rotation #552

Merged
merged 6 commits into from
Dec 10, 2024
Merged

Detect screenshare rotation #552

merged 6 commits into from
Dec 10, 2024

Conversation

davidliu
Copy link
Contributor

@davidliu davidliu commented Dec 9, 2024

No description provided.

Copy link

changeset-bot bot commented Dec 9, 2024

🦋 Changeset detected

Latest commit: ccd01e3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
client-sdk-android Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@bennnjamin
Copy link

These changes look very similar to what is required to fix this issue #535. Especially since it's possible for the device to be in landscape while screensharing when switching between apps. I would greatly appreciate if you took another look at a robust orientation handling solution for the camera in addition to screensharing.

@davidliu
Copy link
Contributor Author

@bennnjamin As I noted in #535, the camera is already following the rotation of the device's UI, so I'm not sure what's actionable there.

@davidliu davidliu merged commit dcbb36b into main Dec 10, 2024
2 checks passed
@davidliu davidliu deleted the dl/screenshare_options branch December 10, 2024 04:08
@davidliu davidliu mentioned this pull request Dec 10, 2024
@bennnjamin
Copy link

@davidliu it doesn't follow the device's UI when switching apps or when there is no foreground activity. It's certainly jarring to a subscriber to see the camera swapping between landscape and portrait orientation during normal operation of the phone. The expectation from a UX perspective is that the video orientation matches the device even when switching apps or multitasking.

The action is to use OrientationListener instead of WindowManager for camera video track orientation like in this pull request but even better would be to allow the developer override the frame orientation. Then it's up to them if they would like to override the default behavior with a custom OrientationListener.

@davidliu
Copy link
Contributor Author

davidliu commented Dec 11, 2024

even better would be to allow the developer override the frame orientation.

@bennnjamin that's possible to do with a VideoProcessor, though you shouldn't have to for rotation. If you can provide a repro example, I'll gladly recheck it and provide a fix if needed, but it was working fine for me.

@bennnjamin
Copy link

@davidliu I appreciate your willingness to help with this. Perhaps I was looking in the wrong place to override rotation. Can you link to a method that is overridable? Here is are the methods I found for frame rotation but they are all private.

And in WebRTC:

https://github.com/webrtc-sdk/webrtc/blob/b99fd2c270361aea2d458e61ac4a4cd2443bdbf6/sdk/android/src/java/org/webrtc/Camera2Session.java#L413

https://github.com/webrtc-sdk/webrtc/blob/b99fd2c270361aea2d458e61ac4a4cd2443bdbf6/sdk/android/src/java/org/webrtc/CameraSession.java#L42

@bennnjamin
Copy link

@davidliu kindly checking to see if you are able to provide further guidance on the correct place to override frame rotation for Camera2 and Camerax sessions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants