Skip to content

Conversation

@danxuliu
Copy link
Member

I think that I found a way to do the mapping without having to touch the external signaling server code :-)

When a participant joins a call the Nextcloud server notifies the signaling server, and the signaling server in turn notifies all the clients. The message sent by the Nextcloud server uses the Nextcloud session id, but the signaling server replaces it with the signaling session id. However, it also passes the rest of properties to the clients as provided by the Nextcloud server. Therefore now the Nextcloud server includes a nextcloudSessionId property, so the message received by the clients contains both the signaling session id and the Nextcloud session id and thus they can perform the mapping between the two.

From what I understand any participant joining a call will notify the Nextcloud server, either directly like the WebUI or mobile apps, or indirectly like SIP clients through the signaling server, so this approach should work in all cases. However note that I have NOT tested with SIP clients; if someone could test or at least confirm that it should work like that it would be great :-)

Besides that Nextcloud session id was removed again from the raised hand message as it was a temporary fix until the mapping was ready (and the issue with the raised hand being kept after a participant left and joined again is also fixed).

@danxuliu danxuliu added this to the 💚 Next Alpha (21) milestone Jan 14, 2021
@nickvergessen nickvergessen added the feature: signaling 📶 Internal and external signaling backends label Jan 14, 2021
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

👯 🎉

@nickvergessen
Copy link
Member

nickvergessen commented Jan 14, 2021

Fix up time

When the internal signaling server is used the Nextcloud session id and
the signaling session id are the same. However, if the external
signaling server is used they are different.

Signaling messages for participants in calls (like joining the call,
muting and so on) are identified only by their signaling session id, so
when the external signaling server is used it was not possible to
associate participants in calls with the participant information
provided by the Nextcloud server.

However, when a participant joins a call her client notifies the
Nextcloud server (either directly or, in the case of SIP clients,
through the signaling server). The Nextcloud server then notifies the
signaling server which, in turn, notifies all the clients. When
Nextcloud notifies the signaling server the participant is identified
with the Nextcloud session id, and the signaling server then replaces
that with the signaling session id before relaying the message to the
clients.

Due to this when a participant joins (or leaves) a call now the
Nextcloud session id is sent too in an extra property. This property is
not adjusted by the external signaling server, so the clients receive
both the signaling session id and the Nextcloud session id and thus are
able to map them.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
As there is now a mapping between the Nextcloud session id and the
signaling server session id it is no longer needed to send the Nextcloud
session id in the raised hand message to identify the participant.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
@danxuliu danxuliu force-pushed the add-mapping-between-nextcloud-session-id-and-signaling-server-session-id branch from f298e4a to fd6ab91 Compare January 14, 2021 12:48
@nickvergessen nickvergessen merged commit 1be3cba into master Jan 14, 2021
@nickvergessen nickvergessen deleted the add-mapping-between-nextcloud-session-id-and-signaling-server-session-id branch January 14, 2021 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release bug feature: signaling 📶 Internal and external signaling backends

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants