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

[stable23] Fix reconnections on single media permission changes #7092

Merged
merged 12 commits into from Apr 4, 2022

Conversation

backportbot-nextcloud[bot]
Copy link

backport of #7034

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The constraints are used in the caller code for things like setting the
current call flags. The constraints were modified by the
MediaDevicesSource and the MediaDevicesManager depending on the
available devices, but they did not reflect things like a node further
in the pipeline failing to set the track (for example, if the
VirtualBackground failed hard) and causing the final track to be null.
Basing the constraints on the tracks instead should address those
(corner) cases while behaving the same in the general cases.

Besides that, this will make possible to remove the constraints from
"MediaDevicesSource.start()" in a following commit.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The constraints given to "MediaDevicesSource.start()" were used to limit
the media to be got. However, this only applied in the initial start; if
a new device from a media kind initially disallowed was selected later
the node got a track from it. Now "setAudioAllowed(bool)" and
"setVideoAllowed(bool)" are introduced to restrict the media to be got
whenever the node is active.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
If media is allowed the track will be started (if a device is
available), and if media is disallowed the current track of its kind
will be stopped.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
These methods will be used to allow or disallow media based on the
permission changes.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Until now the audio and video publishing permissions were handled as a
whole; either the local participant was able to publish both audio and
video or not. However, the permissions API is actually finer grained,
and it is possible to have one without the other. Now the allowed state
of local media is updated based on the publishing permissions, which
automatically starts and stops the tracks as needed. Nevertheless, in
some cases forced reconnections need to be explicitly triggered to
remove tracks from the senders, as well as to start and stop the local
media.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
If publishing media is disallowed now the media is also disabled to
ensure that, if the media is then allowed again, it will not be
unexpectedly enabled.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When the local participant joins a call in most cases the call flags
already reflect the current media being published. However, if the
active media changed since the request to join the call was sent (for
example, if a track started during a forced reconnection, which could
happen if audio was disallowed and video allowed at the same time) the
call flags would need to be updated.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When tracks are replaced the forced reconnections were based on whether
the call was started with local media active or not. However, in some
cases a forced reconnection was needed even if the call was started with
local media active. For example, if after joining a call the user
removed the audio and video devices and then added them again, as if
another participant joined the call in the meantime that participant
would not try to establish a connection after the devices were selected
again, as currently the clients only establish a connection if another
participant has media when the list of participants in the call change.

Besides that, when the flags are updated now the flags are based only in
the current tracks rather than on the previous flags, as if the flags
were updated again before the previous request finished the second
update would be based on the original flags.

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

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested (with HPB) and works 👍

@nickvergessen nickvergessen merged commit 1359d70 into stable23 Apr 4, 2022
14 checks passed
@nickvergessen nickvergessen deleted the backport/7034/stable23 branch April 4, 2022 13:20
@danxuliu
Copy link
Member

danxuliu commented Apr 4, 2022

Tested also without HPB and works 👍 The tests were done by cherry-picking the fixes in #7080 to make forced reconnections more reliable.

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

Successfully merging this pull request may close these issues.

None yet

2 participants