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

Federated call ends after some time without a local participant #12917

Closed
ShGKme opened this issue Aug 7, 2024 · 9 comments · Fixed by strukturag/nextcloud-spreed-signaling#808
Assignees
Labels
1. to develop bug feature: api 🛠️ OCS API for conversations, chats and participants feature: call 📹 Voice and video calls feature: federation 🌐 feature: signaling 📶 Internal and external signaling backends
Milestone

Comments

@ShGKme
Copy link
Contributor

ShGKme commented Aug 7, 2024

How to use GitHub

  • Please use the 👍 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Steps to reproduce

  1. Start a call in a federated conversation
  2. Wait until everyone has joined

Actual behaviour

Federated call silently ends after some time. Looks like 30sec after the last person joined the call

  • Call timer stops after some time ()
  • Some people receive a "You've missed a call" notification during the call.
  • "Join call" is replaced with "Start call", callStartTime: 0, hasCall: false

Talk app

Talk app version: 20.0.0-beta.3 on c.nc.c

@ShGKme ShGKme added this to the 💙 Next Beta (30) milestone Aug 7, 2024
@github-project-automation github-project-automation bot moved this to 🧭 Planning evaluation (don't pick) in 💬 Talk team Aug 7, 2024
@ShGKme ShGKme moved this from 🧭 Planning evaluation (don't pick) to 📄 To do (~10 entries) in 💬 Talk team Aug 7, 2024
@danxuliu
Copy link
Member

The problem is that the ping is not updated for participants in the federated conversation* by the signaling server. Moreover, note that it would not be enough to make the signaling server ping the federated conversation for the federated users, as then the remote participants would still not have a ping, and the code would still misbehave (for example, to know if there are other participants in the call).

*Note that the ping is set when joining the federated conversation, although not when the participant joins the host conversation. However, it is nevertheless set when the signaling server joins the host conversation, and from that moment on the signaling server will update the host conversation, although not the federated conversation.

On a quick test I have not been able to reproduce the notification about the ended call, though; it would be worth finding the right steps to reproduce it.

Below are all the usages (unless I missed something 🤷) of the last ping. @nickvergessen Ideas? :-)

Uses of last_ping:

  • Commands/Monitor commands, to get the number of active participants; results will be wrong anyway in federated conversations, as it will not take into account the remote participants. Maybe ignore federated conversations, or print a warning?
  • Service/ParticipantService:
    • cleanGuestParticipants
    • getParticipantsForAllSessions, if maxAge is provided
    • getParticipantsInCall, if maxAge is provided
    • getParticipantUserIdsForCallNotifications
    • hasActiveSessionsInCall
    • getParticipantWithActiveSession

Uses of *LastPing:

  • Chat/Notifier::shouldParticipantBeNotified, to know if a user is active in the conversation, so the notification should be skipped
  • Controller/CallController::getPeersForCall, not relevant, as it is currently not available for federated conversations, and once it is it will just be proxied from the host.
  • Controller/ChatController::receiveMessages, to update the last ping when receiving messages in mobile apps, as they do not connect to the signaling unless in a call (which may not be true any longer either)
  • Controller/RoomController::formatParticipantList to clean user and guest sessions, clean inactive users, clean guests, and return the last ping in the formatted participant data
  • Controller/RoomController::joinRoom, for conflicts when joining a room again and to initially set the ping when joining the room
  • Controller/SignalingController::backendRoom to update the ping when the signaling server joins the host room and when it updates the ping in the host room; internal signaling server also updates the ping when pulling messages, although that is not currently relevant.
  • Service/RoomFormatter::formatRoomV4, used to format room data in controller responses
  • Signaling/BackendNotifier, not relevant, as it is used to provide the lastPing in the participants->update signaling messages, but this should have no effect in federated conversations, as when the clients join the room in the signaling server the signaling server in turn joins the room in the host Nextcloud server. Therefore, even if the federated Nextcloud server will notify the federated signaling server when the participants change the signaling server will not notify any client, as none of them will be in the federated room from the point of view of the signaling server and, therefore, they will not be listening for room events.

Uses of ParticipantService::cleanGuestParticipants

  • RoomController::formatParticipantList, not relevant, as guests are not allowed in federated conversations (they are on the host conversation, but not on the federated conversation itself)

Uses of ParticipantService::getParticipantsForAllSessions (with maxAge)

  • SignalingController:getUsersInRoom, not relevant, as it is used to get the active users in internal signaling messages; although currently federated calls are not implemented with internal signaling once/if it is implemented this should not be a problem, as either the signaling messages are relayed from the host, so this would not be used, or the signaling messages are got from the federated server, which would implicitly set the ping.

Uses of ParticipantService::getParticipantsInCall (with maxAge)

  • CallController::getPeersForCall, not relevant, as it is currently not available for federated conversations, and once it is it will just be proxied from the host.

Uses of ParticipantService::getParticipantUserIdsForCallNotifications

Uses of ParticipantService::hasActiveSessionsInCall

  • Chat/SystemMessage/Listener::sendSystemMessageAboutBeginOfCall, not relevant, as system messages are not sent in federated conversations
  • Controller/CallController::leaveCall and ::leaveFederatedCall, not relevant, as leaveFederatedCall is not called in federated conversations, and leaveCall returns before checking active sessions in federated conversations
  • Listener/RestrictStartingCalls, not relevant, as it is used to prevent starting a call if the participant does not have permissions and there is no ongoing call based on whether there are active participants in a call or not, but the host server will prevent anyway the federated user to start the call in that case.
  • Notification/Notifier::parseInvitation, to show Join call or View chat in the notification, depending on whether there is an active call or not, when invited to a 1-1 conversation
  • Service/ParticipantService::resetCallStateWhenNeeded, to reset call state when removing a participant, attendee... and there are no other participants in the call

Uses of ParticipantService::getParticipantWithActiveSession

  • Federation/CloudFederationProviderTalk::messagePosted, not relevant, as it is used to know if the user accepted the invite already or not, so the active session is not relevant, only if there is an attendee

@nickvergessen
Copy link
Member

Note that the ping is set when joining the federated conversation, although not when the participant joins the host conversation.

Just for the record, the main difference here is that in case 1 we return the room object on the response. We do not do that on the federated join, so there is no direct need, but we should do it, to avoid the session being purged right away if timing happens to be exact

@nickvergessen
Copy link
Member

As per chat:

  1. I think we should ask the HPB to ping both the Host Server and the Fed Server. The Host Server needs the info to track the general call state. The Fed Server needs the info to deal with notifications. Currently this is Host Server only
  2. Some places (parse invitation, etc) should be changed to be based on the rooms in_call flag instead.

@nickvergessen nickvergessen added feature: api 🛠️ OCS API for conversations, chats and participants feature: signaling 📶 Internal and external signaling backends feature: call 📹 Voice and video calls feature: federation 🌐 labels Aug 26, 2024
@nickvergessen
Copy link
Member

This still reproduces and is the most critical thing atm I would say:
grafik

Especially confusing is that the people are still "in the call", but people joining later can not connect to them.

@nickvergessen nickvergessen changed the title Federated call silently ends after some time Federated call silently ends after some time without a local participant Aug 27, 2024
@nickvergessen nickvergessen changed the title Federated call silently ends after some time without a local participant Federated call ends after some time without a local participant Aug 27, 2024
@nickvergessen
Copy link
Member

Currently this is Host Server only

This is "Fed Server" only atm. Not sure if my initial notes where wrong, or if that is because I use the same HPB for both servers

@fancycode
Copy link
Member

fancycode commented Aug 27, 2024

This is "Fed Server" only atm. Not sure if my initial notes where wrong, or if that is because I use the same HPB for both servers

I can reproduce that the pings are only sent on the Host Server (i.e. the server where the meeting is taking place) and only by the Host signaling server associated with that NC instance.

This is expected as the Host signaling server sends the pings from the room the sessions are connected to - which is the Host server.

@nickvergessen
Copy link
Member

Chatted a bit with Jojo and at least with the latest code base the call is no longer terminating.
However sessions last_ping is only updated on the Host Server for all participants, so the Fed Server can no longer distinguish active/passive sessions for notification handling

@fancycode
Copy link
Member

Should be fixed with strukturag/nextcloud-spreed-signaling#808

@nickvergessen
Copy link
Member

Tested and works

@github-project-automation github-project-automation bot moved this from 📄 To do (~10 entries) to ☑️ Done in 💬 Talk team Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop bug feature: api 🛠️ OCS API for conversations, chats and participants feature: call 📹 Voice and video calls feature: federation 🌐 feature: signaling 📶 Internal and external signaling backends
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants