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

Add support for federated calls #4120

Merged
merged 11 commits into from
Sep 13, 2024
Merged

Conversation

danxuliu
Copy link
Member

This pull request adds support for federated calls when using the external signaling server (note that support for the external signaling server in a federated conversation was not added to the chat view, though, only to the call view for now).

To do that now the federation properties from the signaling settings are given to the external signaling server when joining a room (as described in nextcloud/spreed#12604), and the avatar of participants in call view now take into account the actorType and actorId fields in the participants->update signaling message to get a federated avatar if needed (as described in nextcloud/spreed#12863).

Both changes should be backwards compatible with older Talk versions; if an older Talk version is used the federation properties will not be included in the signaling settings and neither will be actorType and actorId in the participants->update signaling messages, so everything should work just as before.

TODO

  • Make it work... Right now participants in a federated conversation are not properly shown in the call view, although the federated user from Android is shown for other participants
  • Probably add a little hack to prevent the call view from closing if there is no call, as due to a current bug in Talk calls are ended in the federated conversation after the initial ping expires
  • Adjust code that disables call buttons in a federated conversation
  • Cleanup
  • Test with older versions (it should work just as before... but better be sure :-) )

Follow-ups

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed
  • 🔖 Capability is checked or not needed
  • 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • 📅 Milestone is set
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

@danxuliu danxuliu added enhancement New feature or request 2. developing Work in progress feature: ☎️ call feature: 🌐 federation Support for federated conversations labels Aug 28, 2024
@danxuliu danxuliu added this to the 20.0.0 milestone Aug 28, 2024
@nickvergessen nickvergessen requested a review from mahibi August 28, 2024 06:48
Starting with Talk 20 the signaling settings include a "federation"
property that provide the values needed to join a federated room in the
external signaling settings. The "federation" property is specific to
each conversation, and it will be returned although empty for
non-federated conversations.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This is necessary to get the specific federation properties for the
room.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The "federation" values are used by the external signaling server to
establish a connection with the remote signaling server in a federated
room.

For now this is applied only in calls; when the room is joined in the
chat view again after a call it will still join it in the old way,
without federation properties, which will cause the connection with the
remote signaling server to be closed.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Starting with Talk 20 the signaling messages that provide updates to the
participants ("participants->update" in the external signaling server,
"usersInRoom" in the internal signaling server) now include "actorType"
and "actorId" properties for each participant.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
…signaling

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
…signaling

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
…signaling

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Otherwise the WebSocket configuration and session clashes with the one
from the CallActivity, which already uses the federated settings.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@mahibi mahibi force-pushed the add-support-for-federated-calls branch from 0985052 to 4cea7b2 Compare September 13, 2024 14:45
@mahibi mahibi modified the milestones: 20.0.0, 20.1.0 Sep 13, 2024
@mahibi
Copy link
Collaborator

mahibi commented Sep 13, 2024

/backport to stable-20.0

@mahibi mahibi marked this pull request as ready for review September 13, 2024 14:49
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
@mahibi mahibi merged commit 580dd58 into master Sep 13, 2024
11 of 17 checks passed
@mahibi mahibi deleted the add-support-for-federated-calls branch September 13, 2024 15:01
Copy link
Contributor

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/4120-talk.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud Talk app.

Copy link
Contributor

Codacy

Lint

TypemasterPR
Warnings8694
Errors131131

SpotBugs

CategoryBaseNew
Bad practice66
Correctness1111
Dodgy code7879
Internationalization33
Malicious code vulnerability33
Performance66
Security11
Total108109

Lint increased!

SpotBugs increased!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress enhancement New feature or request feature: ☎️ call feature: 🌐 federation Support for federated conversations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants