Screen sharing over WebRTC #5911

onurays · 2022-05-03T12:16:31Z

Start a video call between 2 or more devices
Once the call is connected tap the menu icon (...)
You should see the "Start sharing screen" option, select it
A system permission dialog should appear, accept it
Your screen will be streamed to other devices (in ~5 seconds)
A sticky push notification should appear for Android 10 or later versions
"Start sharing screen" button should turn to "Stop sharing screen"

ericdecanini · 2022-05-03T12:19:19Z

vector/src/main/java/im/vector/app/features/call/VectorCallActivity.kt

+    private fun startScreenSharing(activityResult: ActivityResult) {
+        val videoCapturer = ScreenCapturerAndroid(activityResult.data, object : MediaProjection.Callback() {
+            override fun onStop() {
+                Timber.v("User revoked the screen capturing permission")


If we're keeping this in production I would make this Timber.i

Done. Yes, we have to implement this dummy callback.

ericdecanini · 2022-05-03T12:20:51Z

vector/src/main/java/im/vector/app/features/call/webrtc/WebRtcCall.kt

+        val factory = peerConnectionFactoryProvider.get() ?: return
+
+        this.videoCapturer = videoCapturer
+
+        val localMediaStream = factory.createLocalMediaStream(STREAM_ID)
+        val videoSource = factory.createVideoSource(videoCapturer.isScreencast)
+
+        // Start capturing screen
+        val surfaceTextureHelper = SurfaceTextureHelper.create("CaptureThread", rootEglBase!!.eglBaseContext)
+        videoCapturer.initialize(surfaceTextureHelper, context, videoSource.capturerObserver)
+        videoCapturer.startCapture(currentCaptureFormat.width, currentCaptureFormat.height, currentCaptureFormat.fps)
+
+        // Remove local camera previews
+        localSurfaceRenderers.forEach { it.get()?.let { localVideoTrack?.removeSink(it) } }
+
+        // Show screen preview locally
+        localVideoTrack = factory.createVideoTrack(VIDEO_TRACK_ID, videoSource).apply { setEnabled(true) }
+        localMediaStream?.addTrack(localVideoTrack)
+        localSurfaceRenderers.forEach { it.get()?.let { localVideoTrack?.addSink(it) } }
+
+        // Remove camera stream
+        peerConnection?.removeTrack(videoSender)
+
+        screenSender = peerConnection?.addTrack(localVideoTrack, listOf(STREAM_ID))


Each of these "regions" (as separated by the comments) can be made more readable by each being their own private function. This method can then read like a story without any comments

Done. This class needs refactoring.

github-actions · 2022-05-03T12:51:13Z

Unit Test Results

122 files ±0 122 suites ±0 2m 9s ⏱️ +3s
205 tests ±0 205 ✔️ ±0 0 💤 ±0 0 ❌ ±0
690 runs ±0 690 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit bb862cc. ± Comparison against base commit 0325754.

♻️ This comment has been updated with latest results.

mnaturel

When testing on a real device (Nokia 7.2, Android 11), it seems the sharing is not working. I click on share screen then I see a dialog saying I will share all the content of my screen. And then I see a notification saying a screen sharing is running. But the sharing option stays with "Share my screen" and not "Stop sharing" like I would expect. And I don't see the share on Web. It seems like it does not transfer the video stream.

Also, could you please add a bit more of context and explanation in the PR description (for example how to test, to see what is expected).

mnaturel · 2022-05-04T09:02:16Z

vector/src/main/java/im/vector/app/features/call/VectorCallViewModel.kt

@@ -147,6 +147,7 @@ class VectorCallViewModel @AssistedInject constructor(
                    setState { copy(otherKnownCallInfo = null) }
                }
            }
+            _viewEvents.post(VectorCallViewEvents.StopScreenSharingService)


Here I think the set of the isSharingScreen to false in the state is missing. Maybe we should reuse an mutualize what is done in handleToggleScreenSharing() when screen is shared:

if (isSharingScreen) { call?.stopSharingScreen() setState { copy(isSharingScreen = false) } _viewEvents.post(VectorCallViewEvents.StopScreenSharingService) }

What do you think?

Done, although we finish the call activity as soon as the call ended.

mnaturel · 2022-05-04T09:08:57Z

vector/src/main/java/im/vector/app/features/call/webrtc/ScreenCaptureServiceConnection.kt

+    private var callback: Callback? = null
+
+    fun bind(callback: Callback) {
+        this.callback = callback


I think it would be better to reset the callback to null when unbound to the service in onServiceDisconnected for example.

Right, done.

mnaturel · 2022-05-04T09:24:30Z

vector/src/main/java/im/vector/app/features/call/webrtc/WebRtcCall.kt

@@ -770,12 +775,49 @@ class WebRtcCall(
        return currentCaptureFormat
    }

-    fun startSharingScreen() {
-        // TODO. Will be handled within the next PR.
+    fun startSharingScreen(videoCapturer: VideoCapturer) {


Have you tried to extract the screen sharing capabilities into a dedicated component to avoid adding new responsabilities to WebRtcCall?

I don't think it is easily possible. This class has many local fields. We will start implementing Element Call soon, so we probably won't invest in this class more.

onurays · 2022-05-04T12:47:04Z

When testing on a real device (Nokia 7.2, Android 11), it seems the sharing is not working. I click on share screen then I see a dialog saying I will share all the content of my screen. And then I see a notification saying a screen sharing is running. But the sharing option stays with "Share my screen" and not "Stop sharing" like I would expect. And I don't see the share on Web. It seems like it does not transfer the video stream.

Also, could you please add a bit more of context and explanation in the PR description (for example how to test, to see what is expected).

Thank you for testing by turning off the camera :) I will fix this.

mnaturel · 2022-05-09T08:39:43Z

I tested again with physical device (Android 11, Nokia 7.2), here is what I discovered:

in audio call, when I approach my fingers from the front camera or status bar, the screen becomes black (I guess it behaves like this is several devices since you are supposed to talk to the phone). It may be annoying when screen sharing is running, don't know if we can do something about it.
in audio call, after stopping a screen sharing, the last image stays in the call preview in the web. I don't know if the problem comes from our side. It does not occur in a video call (even if we disable the video after the video call has started)

onurays · 2022-05-09T10:26:49Z

I tested again with physical device (Android 11, Nokia 7.2), here is what I discovered:

in audio call, when I approach my fingers from the front camera or status bar, the screen becomes black (I guess it behaves like this is several devices since you are supposed to talk to the phone). It may be annoying when screen sharing is running, don't know if we can do something about it.

in audio call, after stopping a screen sharing, the last image stays in the call preview in the web. I don't know if the problem comes from our side. It does not occur in a video call (even if we disable the video after the video call has started)

Proximity sensor issue is a good catch and fixed. Second one seems the responsibility of web, because we stop sending streams.

mnaturel · 2022-05-09T12:23:13Z

Thanks for fixing the proximity sensor issue, it is okay now. I discovered a new thing:

in a video call, I start sharing screen. When I disabled the video it disable the screen sharing as well. I think the user expects to only turn off the camera and not both camera and screen sharing.

github-actions · 2022-05-10T15:03:29Z

Matrix SDK

Integration Tests Results:

[org.matrix.android.sdk.session]
= passed=15 failures=5 errors=0 skipped=3
[org.matrix.android.sdk.account]
= passed=3 failures=0 errors=0 skipped=2
[org.matrix.android.sdk.internal]
= passed=110 failures=0 errors=0 skipped=38
[org.matrix.android.sdk.ordering]
= passed=16 failures=0 errors=0 skipped=0
[org.matrix.android.sdk.PermalinkParserTest]
= passed=2 failures=0 errors=0 skipped=0

Onuray Sahin added 3 commits April 22, 2022 15:50

Try to start streaming screen capture.

7939eca

Remove current video capturer and then share screen.

fb7533b

Changelog added.

dd5d263

onurays requested review from a team, ericdecanini and mnaturel and removed request for a team May 3, 2022 12:17

ericdecanini reviewed May 3, 2022

View reviewed changes

Code review fixes.

166be43

onurays requested a review from ericdecanini May 3, 2022 14:18

mnaturel reviewed May 4, 2022

View reviewed changes

Code review fixes.

b358863

onurays requested a review from mnaturel May 4, 2022 12:30

Onuray Sahin added 4 commits May 5, 2022 12:57

Fix stop sharing button state.

ba4413e

Don't enable video after stopping screen sharing for audio calls.

754208e

Update video mute status after stopping screen sharing.

b486559

Bind to screen sharing service after app killed and relaunched.

cf3d145

Stop proximity sensor while sharing screen.

9a1dbb2

onurays added 2 commits May 9, 2022 17:26

Disable video toggle button during screen sharing.

3a02e84

Lower alpha of video button while screen sharing.

bb862cc

mnaturel approved these changes May 10, 2022

View reviewed changes

onurays removed the request for review from ericdecanini May 10, 2022 09:50

onurays merged commit 185cd31 into develop May 10, 2022

onurays deleted the feature/ons/voip_screen_sharing branch May 10, 2022 10:06

This was referenced Jun 21, 2022

Screen Share #5602

Open

Add the ability to hide (video) call buttons #2432

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Screen sharing over WebRTC #5911

Screen sharing over WebRTC #5911

onurays commented May 3, 2022 •

edited

ericdecanini May 3, 2022

onurays May 3, 2022

ericdecanini May 3, 2022

onurays May 3, 2022

github-actions bot commented May 3, 2022 •

edited

mnaturel left a comment

mnaturel May 4, 2022

onurays May 4, 2022

mnaturel May 4, 2022

onurays May 4, 2022

mnaturel May 4, 2022

onurays May 4, 2022

onurays commented May 4, 2022

mnaturel commented May 9, 2022

onurays commented May 9, 2022

mnaturel commented May 9, 2022

github-actions bot commented May 10, 2022

Screen sharing over WebRTC #5911

Screen sharing over WebRTC #5911

Conversation

onurays commented May 3, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 3, 2022 • edited

Unit Test Results

mnaturel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onurays commented May 4, 2022

mnaturel commented May 9, 2022

onurays commented May 9, 2022

mnaturel commented May 9, 2022

github-actions bot commented May 10, 2022

Matrix SDK

Integration Tests Results:

onurays commented May 3, 2022 •

edited

github-actions bot commented May 3, 2022 •

edited