Disconnect received videos when not needed #6862

danxuliu · 2022-02-03T22:52:11Z

Fixes #5388
Requires strukturag/nextcloud-spreed-signaling#191

If a received video is explicitly disabled by the user, or if the remote participant is not visible, the received video is not needed. Until now the video was simply not rendered, but with this pull request the received video is disconnected/blocked, so it is not even transmitted nor decoded.

Note that, for now, this works only when the HPB is used. When the HPB is not used the renegotiation would need to be implemented by all clients, so something for the future.

If the external signaling server does not support the new update-sdp feature the video will not be disconnected when not needed either. If update-sdp is not supported then requesting a new offer would close the previous connection and create a new one, which would cause the audio to be interrupted during the connection change.

Pending:

Disconnect also not visible videos (not only explicitly blocked videos)
Verify that overlapping disconnections and reconnections work as expected (for example, quickly blocking and unblocking again before the previous reconnection finished) - The RTCPeerConnection will trigger a new negotiationneeded event if the expected transceiver direction does not match once the connection is established
Adjust to possible changes in Add support for request offers to update subscriber connections strukturag/nextcloud-spreed-signaling#191
Add details to the external signaling documentation about offer updates - The documentation was moved to the nextcloud-spreed-signaling repository, so it should be done there
Disconnect received video if the sender disabled it? Probably not worth the trouble, as the received video will be a 1FPS black stream, so it should not require much resources, and it would be needed only for the visible participants, as for the hidden ones it will be already blocked. Nevertheless, it could be added in a follow up if needed.
Make this work also without HPB - Requires renegotiations in all the clients, it will be done in a follow up
Show loading spinner or something like that while enabling video again? Or just the avatar? Right now the last video frame received before the video was disconnected may be shown even a few seconds after the renegotiation has finished.
- The framesDecoded property of the inbound-rtp stat could be monitored to know when the video has started to play again. However, in older browser versions that property was not available, and stats for received connections were not updated at a regular rate, so this should be carefully tested. Therefore it would be better to do it in a follow up pull request.
- If instead of a WebRTC renegotiation the received video is disabled using the Janus specific approach the video seems to start playing again much faster, so it may not be needed to show a spinner. However, it would be still needed once the videos are disconnected also without HPB.

How to test (scenario 1)

Setup the HPB (with Add support for request offers to update subscriber connections strukturag/nextcloud-spreed-signaling#191)
Start a call with audio and video
In a private window, join the call without audio nor video
Open the WebRTC stats in another tab; in Firefox about:webrtc, in Chromium chrome://webrtc-internals
Open the video stats for the receiver connection; in Firefox the stats are shown at the end of the details of a connection, in Chromium they are shown under RTCMediaStreamTrack_receiver_2 (track) (or something similar)
Disable the received video

Result with this pull request

The video is no longer received according to the stats; if the video is enabled again the video will be received again

Result without this pull request

The video is still received according to the stats

How to test (scenario 2)

Setup the HPB (with Add support for request offers to update subscriber connections strukturag/nextcloud-spreed-signaling#191)
Start a call with audio and video
In a private window, join the call without audio nor video
In another browser, join the call without audio nor video
Open the WebRTC stats in another tab; in Firefox about:webrtc, in Chromium chrome://webrtc-internals
Open the video stats for the receiver connection; in Firefox the stats are shown at the end of the details of a connection, in Chromium they are shown under RTCMediaStreamTrack_receiver_2 (track) (or something similar)
Follow the participant from the private window (so the participant from the original window is hidden)
Wait 5 seconds (as the video is not immediately blocked when hidden)

Result with this pull request

The video is no longer received according to the stats; if the video is shown again the video will be received again

Result without this pull request

The video is still received according to the stats

src/__mocks__/attachmediastream.js

If the HPB is used it is enough to reconnect only with the receiver peer that needs a negotiation rather than forcing a full reconnection. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Before a negotiation needed event in a receiver Peer caused a forced reconnection, so the whole call was reset. Now only the affected Peer object is reset, so the simulcast levels previously set need to be set again on the new Peer object. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

When an offer is requested to the HPB the old connection is stopped and a new one is created. If the HPB has support for updating the subscribers this is now used instead when negotiation is needed, as this prevents the existing connection (and thus the media) to be interrupted during the renegotiation. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Remote video can be blocked and allowed again by adjusting the direction of the video transceiver of RTCPeerConnections, which triggers a "negotiationneeded" event. When a negotiation is needed a new exchange of offers and answers needs to be made between the peers. Renegotiations need to be supported by both peers, so currently this is available only with the HPB, and only on versions that support the "update-sdp" feature. When the HPB is being used "negotiationneeded" event causes a new offer to be requested, which will be an update if the HPB supports it. Updates rather than new connections are needed to prevent an audio interruption while the connection changes. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

If the connection transitions to "disconnected" (or was disconnected before starting the renegotiation) the UI will wrongly show the message about reconnecting due to connection problems. I have not found an easy way to solve this, as it would require to keep track of whether a renegotiation started due to handling the "negotiationneeded" event or something else (and, in the future, once renegotiations are implemented also without HPB, it should be necessary to know too if the offer was sent by the remote peer for other reasons). Due to this, for now a slightly misleading message will be shown if the connection is updated while "disconnected". Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

A remote video is not needed when it was explicitly disabled by the local user (independently of whether it is enabled by the remote participant) or when it is not visible. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Now the remote videos will be blocked when not needed, that is, when explicitly disabled by the local user or when not visible. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

The "talk:video:toggled" event is triggered by VideoBottomBar and only handled in CallView to enable or disable the remote video. As the handling just calls a method in the RemoteVideoBlocker this can be directly done in the VideoBottomBar instead. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

nickvergessen · 2022-04-21T06:54:04Z

In a private window, join the call without audio nor video
The video is no longer received according to the stats; if the video is enabled again the video will be received again

So it worked for me using a private window, but a normal window did not work 😕 . Is that a bug with about:webrtc?

nickvergessen

Other than that it seems to work.
PR state is still developing?

danxuliu · 2022-04-21T10:19:51Z

In a private window, join the call without audio nor video
The video is no longer received according to the stats; if the video is enabled again the video will be received again

So it worked for me using a private window, but a normal window did not work confused . Is that a bug with about:webrtc?

No idea :-/ I have just tried and it worked fine for me when blocking the video in a normal window and when blocking the video in a private window 🤷

Other than that it seems to work.
PR state is still developing?

The pull request is ready, but it may require changes if something is changed in strukturag/nextcloud-spreed-signaling#191 (that is why I did not set it to To review).

danxuliu · 2022-04-28T07:58:26Z

strukturag/nextcloud-spreed-signaling#191 was merged, so ready for review. The pending items can be done in follow ups.

danxuliu · 2022-04-29T09:50:10Z

/backport to stable24

danxuliu added 2. developing feature: WebRTC 🚡 WebRTC connection between browsers and/or mobile clients feature: signaling 📶 Internal and external signaling backends feature: frontend 🖌️ "Web UI" client feature: call 📹 Voice and video calls labels Feb 3, 2022

danxuliu added this to the 💛 Next Major (24) milestone Feb 3, 2022

nickvergessen reviewed Feb 9, 2022

View reviewed changes

src/__mocks__/attachmediastream.js Show resolved Hide resolved

nickvergessen modified the milestones: v14.0.0-beta.1, 💛 Next RC (24) Apr 8, 2022

danxuliu force-pushed the disconnect-received-videos-when-not-needed branch from 6ade95c to da0c6d2 Compare April 13, 2022 12:13

nickvergessen modified the milestones: v14.0.0-rc.1, 💛 Next RC (24), 💚 Next Major (25) Apr 13, 2022

danxuliu force-pushed the disconnect-received-videos-when-not-needed branch from a35401b to 9c17bf5 Compare April 20, 2022 14:01

danxuliu added 10 commits April 20, 2022 16:02

Do not force reconnect when negotiation is needed for receiver peer

4b3e824

If the HPB is used it is enough to reconnect only with the receiver peer that needs a negotiation rather than forcing a full reconnection. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Add tests for UI feedback on connection status of remote participants

5cb20b9

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Refactor "isNotConnected()" to "isConnected()"

bbb1783

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Use RemoteVideoBlocker in call view

114b83d

Now the remote videos will be blocked when not needed, that is, when explicitly disabled by the local user or when not visible. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

danxuliu force-pushed the disconnect-received-videos-when-not-needed branch from 9c17bf5 to 4e4ff9b Compare April 20, 2022 14:03

nickvergessen approved these changes Apr 21, 2022

View reviewed changes

danxuliu removed the 2. developing label Apr 28, 2022

danxuliu added the 3. to review label Apr 28, 2022

nickvergessen merged commit ca478a5 into master Apr 28, 2022

nickvergessen deleted the disconnect-received-videos-when-not-needed branch April 28, 2022 08:20

danxuliu mentioned this pull request Apr 29, 2022

Disconnect received videos by toggling the video directly in Janus #7237

Merged

1 task

backportbot-nextcloud bot added the backport-request label Apr 29, 2022

backportbot-nextcloud bot mentioned this pull request Apr 29, 2022

[stable24] Disconnect received videos when not needed #7247

Merged

backportbot-nextcloud bot removed the backport-request label Apr 29, 2022

This was referenced Apr 30, 2022

Fix endless offer requests with the SIP bridge #7257

Merged

Fix trying to use "remoteVideoBlocker" with local screens #7317

Merged

danxuliu mentioned this pull request Aug 31, 2022

Stop receiving video when not visible (user opt out or other page) #6698

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Disconnect received videos when not needed #6862

Disconnect received videos when not needed #6862

Uh oh!

danxuliu commented Feb 3, 2022 •

edited

Loading

Uh oh!

Uh oh!

nickvergessen commented Apr 21, 2022

Uh oh!

nickvergessen left a comment

Uh oh!

danxuliu commented Apr 21, 2022

Uh oh!

danxuliu commented Apr 28, 2022

Uh oh!

danxuliu commented Apr 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Disconnect received videos when not needed #6862

Disconnect received videos when not needed #6862

Uh oh!

Conversation

danxuliu commented Feb 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to test (scenario 1)

Result with this pull request

Result without this pull request

How to test (scenario 2)

Result with this pull request

Result without this pull request

Uh oh!

Uh oh!

nickvergessen commented Apr 21, 2022

Uh oh!

nickvergessen left a comment

Choose a reason for hiding this comment

Uh oh!

danxuliu commented Apr 21, 2022

Uh oh!

danxuliu commented Apr 28, 2022

Uh oh!

danxuliu commented Apr 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

danxuliu commented Feb 3, 2022 •

edited

Loading