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

feat(calls): Allow to enforce a maximum call length #13456

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Oct 2, 2024

☑️ Resolves

🛠️ API Checklist

Tested with:

  • Android: ☑️
  • iOS: ☑️
  • Web: ☑️

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not possible
  • 📘 API documentation in docs/ has been updated or is not required
  • 🔖 Capability is added or not needed

@nickvergessen nickvergessen added 3. to review enhancement feature: settings ⚙️ Settings and config related issues feature: api 🛠️ OCS API for conversations, chats and participants feature: conversations 👥 labels Oct 2, 2024
@nickvergessen nickvergessen added this to the 🖤 Next Major (31) milestone Oct 2, 2024
@nickvergessen nickvergessen self-assigned this Oct 2, 2024
docs/settings.md Outdated Show resolved Hide resolved
docs/settings.md Outdated
@@ -82,6 +82,7 @@ Legend:
| `bridge_bot_password` | string | | No | | Automatically generated password of the matterbridge bot user profile |
| `default_attachment_folder` | string | `/Talk` | No | | Specify default attachment folder location |
| `start_calls` | int | `0` | Yes | 🖌️ | Who can start a call, see [constants list](constants.md#start-call) |
| `max_call_duration` | int | `0` | No | | Maximum duration of a call in seconds, 0 for unlimited |
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be mentioned that federated rooms are not supported?

Copy link
Member Author

Choose a reason for hiding this comment

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

They are on the host? But yes we can add it.

@ShGKme
Copy link
Contributor

ShGKme commented Oct 4, 2024

This value is not available in capabilities => clients and users don't know about the limit. The call just suddenly stops.

Shall we add it to capabilities to be able to inform users about it?

continue;
}

$this->participantService->endCallForEveryone($room, null);
Copy link
Contributor

Choose a reason for hiding this comment

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

To think about it in the future: this would be nice to be able to set the reason. So users know why the call has been ended.

@ShGKme
Copy link
Contributor

ShGKme commented Oct 4, 2024

Tested with 300 seconds. The call was ended after 407 seconds regardless of the second participant joining time. Both 1-1 and group.

Using the default cron.

@nickvergessen
Copy link
Member Author

This value is not available in capabilities => clients and users don't know about the limit. > The call just suddenly stops.

Shall we add it to capabilities to be able to inform users about it?

Sounds like a good UX follow up. But out of scope for the current requirements :)

Tested with 300 seconds. The call was ended after 407 seconds regardless of the second participant joining time. Both 1-1 and group.

Using the default cron.

Well if cron is not running there is little we can do. If you do a mobile app only call nothing is pinging the server, so we have to rely on the server cron. Planned requirement is 12h, so the ~5 min worst case overlay is "accepted risk".

Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

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

Works, but some things to concern:

  1. Users have no sign that the call time is limited
  2. Clients have no possibility to inform users about the limit
  3. A call ends suddenly like it was ended by a participant. User "Alice" may be confused why there is "Alice ended the call" when nobody ended the call
  4. Duration is in seconds, but with the default cron it might have 300 seconds error. Might be unclear for users

@nickvergessen nickvergessen merged commit 06b85d8 into main Oct 4, 2024
69 checks passed
@nickvergessen nickvergessen deleted the feat/13445/hard-end-call-limit branch October 4, 2024 14:10
@ShGKme
Copy link
Contributor

ShGKme commented Oct 4, 2024

Planned requirement is 12h

It's a bit confusing:

  • A user can set it with accuracy of 1 sec
  • The default error is 5 min (300 sec)
  • The expected typical value is around 12h (43_200 sec)

Do we need 1 second precision with ~300 sec error and thousands of seconds values? Maybe then make it in minutes and if we add a UI make it 5 min step?

Or at least note it in the docs.

@nickvergessen
Copy link
Member Author

Do we need 1 second precision with ~300 sec error and thousands of seconds values?

Makes CI testing easier if we add it.

… and if we add a UI …

We will not :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically end calls after defined duration
2 participants