[SES-1652] Swap video views in calls #1533

ThomasSession · 2024-07-08T00:30:04Z

Added the ability to swap videos during a call.
I changed Ryan's approach to only use two renderer instead of four for optimisation.

I found several issues with the video that I hopefully addressed in this PR:

Proper device rotation handling. The old code was relying on OrientationEventListener > onOrientationChanged, which gives us a rotation angle that doesn't take into account pitch vs roll, so tipping the device from front to back would trigger the video rotation logic, while we really only want it when the device is in portrait or landscape.
Floating video inset state when the camera is turned off
Fixed aspect ratio of video streams. The videos now scale appropriately based on the device's orientation, including the incoming video.
Animated control rotation. With the new device orientation detection, I added animated control rotation when the device changes its orientation
Fixed styling inconsistencies

Known issue:

There is a system limitation due to the webrtc library we are using: We can't round the corners of the floating video inset when the fullscreen video stream is also enabled. That is because we can't have two surfaceViews on at the same time while still doing clipping work on them. We also can't use a TextureView as we are limited by what WebRTC supports.

This reverts commit 0d386e7.

Only using two sinks and swapping between them Reworked the device rotation logic as it didn't work well with pitch ( you could tip the device front to back and the rotation went out of whack, so had to resort to more robust calculation for the device orientation. Had to use a deprecated sensor setting but it's the only one I could use that works.

Rounded corners for floating inset Proper handling of video scaling based on video proportions Proper handling of mirroring logic for floating/fullscreen videos depending on whether they are the user or the remote video and whether the camera is front facing or not

…ream active, hiding it when both are inactive

simophin

LGTM

bemusementpark

Awesome!

Maybe we should...

follow the users portrait-lock
fix that lateinit
refactor the sensorListener
Fix the rounded corners when you both have video on (might just be )

app/src/main/java/org/thoughtcrime/securesms/calls/WebRtcCallActivity.kt

bemusementpark · 2024-07-08T04:44:24Z

app/src/main/java/org/thoughtcrime/securesms/calls/WebRtcCallActivity.kt

+ super.onPause()
+ try {
+ sensorManager.unregisterListener(this)
+ } catch (e: Exception) {


There is a sensorManager::isInitialized property on lateinits so you don't need to catch the NPE... but it's probably a code smell to use it, or to catch this exception.

app/src/main/java/org/thoughtcrime/securesms/calls/WebRtcCallActivity.kt

app/src/main/java/org/thoughtcrime/securesms/webrtc/CallManager.kt

bemusementpark · 2024-07-08T05:08:48Z

app/src/main/java/org/thoughtcrime/securesms/webrtc/CallManager.kt

+ _videoState.value = _videoState.value.copy(swapped = videoSwapped)
+ handleMirroring()
+
+ if (!videoSwapped) {


app/src/main/java/org/thoughtcrime/securesms/webrtc/CallManager.kt

bemusementpark · 2024-07-08T05:11:53Z

app/src/main/java/org/thoughtcrime/securesms/webrtc/CallManager.kt

@@ -721,7 +790,7 @@ class CallManager(
 connection.setCommunicationMode()
 setAudioEnabled(true)
 dataChannel?.let { channel ->
- val toSend = if (!_videoEvents.value.isEnabled) VIDEO_DISABLED_JSON else VIDEO_ENABLED_JSON
+ val toSend = if (!_videoState.value.userVideoEnabled) VIDEO_DISABLED_JSON else VIDEO_ENABLED_JSON


Suggested change

val toSend = if (!_videoState.value.userVideoEnabled) VIDEO_DISABLED_JSON else VIDEO_ENABLED_JSON

val toSend = if (_videoState.value.userVideoEnabled) VIDEO_ENABLED_JSON else VIDEO_DISABLED_JSON

d'you want to do this one? it's easier to read and reason about without the negation. It's easy to miss a !.

ThomasSession · 2024-07-08T05:41:46Z

follow the users portrait-lock

It should already follow the portrait lock from the isAutoRotateOn().
It will only check once but I don't think there is a way to know the device orientation otherwise.
We could of course add the config change handling form the manifest, and request to self handle orientation changes.
This would void the need for the sensor as you get that information for free, and which is what I initially started the code with.
But it also means we need a separate layout for landscape instead of forcing to portrait and rotating only certain elements like we are doing now.

bemusementpark · 2024-07-08T07:58:13Z

It should already follow the portrait lock from the isAutoRotateOn().

yeah, but we could just add the listener, and then check this in the listener as the dead simple solution, no? possible issue if you're landscape and then you set portrait-lock. Depends how pedantic we want to get. But you could foresee that a user would have portrait lock (on/off) and start a call, and realise they want it (off/on) change the system setting and think it doesn't work, but they just have to press back and enter the call again for it to take effect... :/

The new OrientationManager encapsulate the orientation logic and sends out a mutable state flow

bemusementpark

🎉

bemusementpark · 2024-07-10T05:19:06Z

app/src/main/java/org/thoughtcrime/securesms/calls/WebRtcCallActivity.kt

+ if(!state.userVideoEnabled && !state.remoteVideoEnabled){
+ binding.floatingRendererContainer.isVisible = false
+ binding.swapViewIcon.isVisible = false
+ } else {
+ binding.floatingRendererContainer.isVisible = true
+ binding.swapViewIcon.isVisible = true
+ }


Suggested change

if(!state.userVideoEnabled && !state.remoteVideoEnabled){

binding.floatingRendererContainer.isVisible = false

binding.swapViewIcon.isVisible = false

} else {

binding.floatingRendererContainer.isVisible = true

binding.swapViewIcon.isVisible = true

}

state.run { userVideoEnabled || remoteVideoEnabled }.let {

binding.floatingRendererContainer.isVisible = it

binding.swapViewIcon.isVisible = it

}

bemusementpark · 2024-07-10T05:22:01Z

app/src/main/java/org/thoughtcrime/securesms/webrtc/CallManager.kt

 private var peerConnectionFactory: PeerConnectionFactory? = null

+ // false when the user's video is in the floating render and true when it's in fullscreen
+ private var videoSwapped: Boolean = false


can we remove this and just let the State be the source of truth?

bemusementpark · 2024-07-10T05:23:01Z

app/src/main/java/org/thoughtcrime/securesms/webrtc/CallManager.kt

@@ -721,7 +790,7 @@ class CallManager(
 connection.setCommunicationMode()
 setAudioEnabled(true)
 dataChannel?.let { channel ->
- val toSend = if (!_videoEvents.value.isEnabled) VIDEO_DISABLED_JSON else VIDEO_ENABLED_JSON
+ val toSend = if (!_videoState.value.userVideoEnabled) VIDEO_DISABLED_JSON else VIDEO_ENABLED_JSON


d'you want to do this one? it's easier to read and reason about without the negation. It's easy to miss a !.

RyanRory and others added 21 commits February 16, 2023 16:03

WIP: swap video views

370cfb6

Merge branch 'dev' into swap-video-views

c474414

feat: swap video views

39b7980

minor fixes

cfb8e34

minor fix

baa4771

minor fix

05eee80

Merge branch 'dev' into swap-video-views

8678ac5

update libsession-util

0d386e7

Revert "update libsession-util"

ef1b671

This reverts commit 0d386e7.

reverse updating libsession-util

edcba41

update libsession-util

69f976f

Merge branch 'dev' into swap-video-views

8238503

Merge branch 'dev' into pr/1127

f80777d

Merge branch 'dev' into pr/1127

0bbb1c4

Tweaking colors to match designs

74b8a95

More theme fixes

72b9190

Showing floating video inset only when there is at least one video st…

f9e9bc8

…ream active, hiding it when both are inactive

Rotating controls on rotation

f0c7621

Clean up

eec45ae

ThomasSession changed the title ~~https:/oxen-io/session-android/pull/1127~~ [SES-1652] Swap video views in calls Jul 8, 2024

ThomasSession mentioned this pull request Jul 8, 2024

[SES-1652] Swap video views in calls #1127

Closed

simophin approved these changes Jul 8, 2024

View reviewed changes

bemusementpark self-requested a review July 8, 2024 04:54

bemusementpark suggested changes Jul 8, 2024

View reviewed changes

ThomasSession added 2 commits July 10, 2024 14:19

Review feedback

503f361

OrientationManager

a1c8974

The new OrientationManager encapsulate the orientation logic and sends out a mutable state flow

bemusementpark approved these changes Jul 10, 2024

View reviewed changes

PR feedback

ddd6396

ThomasSession merged commit b510b06 into dev Jul 10, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SES-1652] Swap video views in calls #1533

[SES-1652] Swap video views in calls #1533

ThomasSession commented Jul 8, 2024 •

edited

Loading

simophin left a comment

bemusementpark left a comment

bemusementpark Jul 8, 2024

bemusementpark Jul 8, 2024

bemusementpark Jul 8, 2024

bemusementpark Jul 10, 2024

ThomasSession commented Jul 8, 2024

bemusementpark commented Jul 8, 2024

bemusementpark left a comment

bemusementpark Jul 10, 2024

bemusementpark Jul 10, 2024

bemusementpark Jul 10, 2024

	val toSend = if (!_videoState.value.userVideoEnabled) VIDEO_DISABLED_JSON else VIDEO_ENABLED_JSON
	val toSend = if (_videoState.value.userVideoEnabled) VIDEO_ENABLED_JSON else VIDEO_DISABLED_JSON

[SES-1652] Swap video views in calls #1533

[SES-1652] Swap video views in calls #1533

Conversation

ThomasSession commented Jul 8, 2024 • edited Loading

simophin left a comment

Choose a reason for hiding this comment

bemusementpark left a comment

Choose a reason for hiding this comment

bemusementpark Jul 8, 2024

Choose a reason for hiding this comment

bemusementpark Jul 8, 2024

Choose a reason for hiding this comment

bemusementpark Jul 8, 2024

Choose a reason for hiding this comment

bemusementpark Jul 10, 2024

Choose a reason for hiding this comment

ThomasSession commented Jul 8, 2024

bemusementpark commented Jul 8, 2024

bemusementpark left a comment

Choose a reason for hiding this comment

bemusementpark Jul 10, 2024

Choose a reason for hiding this comment

bemusementpark Jul 10, 2024

Choose a reason for hiding this comment

bemusementpark Jul 10, 2024

Choose a reason for hiding this comment

ThomasSession commented Jul 8, 2024 •

edited

Loading