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

Reconnect on all ICE failures when using the MCU #1881

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@danxuliu
Copy link
Member

commented Jun 3, 2019

Until now, when using the MCU, reconnections only happened when the connection failed for the peer that sent the offer, that is, for the own peer object that sends the media to the MCU; if the connection failed on any of the other peers (those that receive the media from the MCU) there was no reconnection.

In some cases it could happen that one of the connections to the MCU to receive the media from another participant failed while the other connections to receive the media as well as the connection to send it were kept connected. In that case the participant would still be in the call, but she would not be able to hear or see the other participant. To prevent this now reconnections are forced on any ICE failures when using the MCU.

Note that this does not lead to any glare issues (two peers sending an offer to each other), as when the MCU is used peers only send their own offer to the MCU and receive offers for the rest of the peers from the MCU.

All of the above applies only when the MCU is used; when the MCU is not used the unconditional reconnection can not be applied (due to the glare issues), so reconnections when the MCU is not used still need to be fixed.

danxuliu added some commits May 24, 2019

Move forced reconnection functions above "videos" object
This is just groundwork for the following commit.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Reconnect on all ICE failures when using the MCU
Until now, when using the MCU, reconnections only happened when the
connection failed for the peer that sent the offer, that is, for the own
peer object that sends the media to the MCU; if the connection failed on
any of the other peers (those that receive the media from the MCU) there
was no reconnection.

In some cases it could happen that one of the connections to the MCU to
receive the media from another participant failed while the other
connections to receive the media as well as the connection to send it
were kept connected. In that case the participant would still be in the
call, but she would not be able to hear or see the other participant. To
prevent this now reconnections are forced on any ICE failures when using
the MCU.

Note that this does not lead to any glare issues (two peers sending an
offer to each other), as when the MCU is used peers only send their own
offer to the MCU and receive offers for the rest of the peers from the
MCU.

All of the above applies only when the MCU is used; when the MCU is not
used the unconditional reconnection can not be applied (due to the glare
issues), so reconnections when the MCU is not used still need to be
fixed.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@@ -581,6 +637,8 @@ var spreedPeerConnectionTable = [];

videoView.setConnectionStatus(OCA.Talk.Views.VideoView.ConnectionStatus.FAILED_NO_RESTART);
}
} else {
handleIceFailedWithMcu();

This comment has been minimized.

Copy link
@fancycode

fancycode Jun 4, 2019

Member

You are forcing a reconnect if either the own stream or one of the peers fails. Did you test what happens when you are in a call with multiple peers and your connection to the MCU gets interrupted? In that scenario, you will receive a failed event for all of the streams in a short period and will force multiple reconnects from what I understand.

It probably would be better to start a short timer on a failed event to be able to combine multiple events into a single reconnect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.