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

Proper handling of peers that join and leave the conference #130

Open
daniel-abramov opened this issue Feb 11, 2023 · 1 comment
Open

Proper handling of peers that join and leave the conference #130

daniel-abramov opened this issue Feb 11, 2023 · 1 comment
Labels
T-Enhancement New feature or request T-Task X-Refactor Refactoring task

Comments

@daniel-abramov
Copy link
Contributor

Currently, there is only a single shared channel that all peers use. This channel is created by the conference and passed to each peer. Each peer gets a wrapped copy of the channel which adds the identifiers (the sender of the message) to each message. Then peers write to the channel and the conference listens on the channel to get peer updates.

This means that there is a custom-wrapped version of the channel that is actually never closed (it could get closed once the last peer ends, but by the receiver, not by the sender which is a bit odd and not particularly idiomatic in Go).

A much more idiomatic and elegant approach would be each peer creating its own channel to report the updates and returning the listening part to the caller. That way, when the peer feels like it's done and the channel is not required, the peer could close the channel. The caller will listen to the channel to get peer updates and get notified when the channel is closed. This means that the conference would either listen not on a single peer channel, but on a slice of channels (the size of each changes dynamically) and stop the conference once the last channel is closed. This would require the usage of reflect.Select() and dynamically managing the size of the channel, but would result in a more predictable and idiomatic API that is also easier to reason about.

@daniel-abramov daniel-abramov added T-Enhancement New feature or request T-Task X-Refactor Refactoring task and removed T-Task T-Enhancement New feature or request labels Feb 11, 2023
@daniel-abramov daniel-abramov changed the title Idiomatic handling of the channels from peers to the conference Proper handling of peers that join and leave the conference Mar 6, 2023
@daniel-abramov daniel-abramov added the T-Enhancement New feature or request label Mar 6, 2023
@daniel-abramov
Copy link
Contributor Author

In addition, I suggest adding unique identifiers to each peer on the SFU side when reporting messages from the peers to the conference. I.e. the conference may have a counter and increase it by one each time someone joins the conference. Each new participant will be given an internal ID that the SFU assigns to it, then we could log only the user_id and the internal_id to separate participants (and report them to the telemetry when we have one).

The reason this is required is because of the following scenario that I was able to reproduce once:

  1. User X joined the call.
  2. User X re-joins the call. When the call is re-joined, the user_id and device_id are the same, while the call_id is different, this is problematic since it means that for some time the participant map will have 2 participants: the "old" participant X and the "new" participant X, because although they share the same identifiers, their call_id is different (and the call_id is part of the ParticipantID structure), which makes us think that it's a different instance of the participant (whereas in reality, it's the same participant).
  3. Our logs only contain user_id and device_id at the moment, which makes it confusing for the log reader to distinguish the communication between these 2 instances of the same participant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Enhancement New feature or request T-Task X-Refactor Refactoring task
Projects
None yet
Development

No branches or pull requests

1 participant