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 specific UI for recording calls #8725

Merged
merged 3 commits into from Feb 17, 2023

Conversation

danxuliu
Copy link
Member

@danxuliu danxuliu commented Feb 9, 2023

Follow up to #8708

The recording UI can be tested without actually recording the call in the following way:

  • Start a call in a public conversation
  • In a private window, open {url of the call}/recording

Note that there will be no sound, as the call is joined without any user interaction and therefore sounds are blocked. This is correctly handled when accessing the endpoint from the recording server.

🖼️ Screenshots

🏚️ Before 🏡 After
Recording-UI-Before Recording-UI-After

🚧 TODO

  • Cleanup experimental code
  • Add proper validation in the page endpoint - Validation against random and checksum like in OCS requests from the signaling and recording servers can not be added (or, at least, not easily), as the headers sent by the browser when loading the URL can not be modified. Nevertheless, if I am not mistaken it should be enough to have it public with a brute force protection for the room token, as the page itself does not reveal any information (other than the room token), and any further action require the participant to be properly authenticated to access protected conversations.
  • Fix margins of the call container

@danxuliu danxuliu added this to the 💚 Next Major (26) milestone Feb 9, 2023
@danxuliu danxuliu mentioned this pull request Feb 9, 2023
16 tasks
@danxuliu danxuliu force-pushed the add-specific-ui-for-recording-calls branch from 7434178 to ebe6585 Compare February 13, 2023 04:41
@nickvergessen
Copy link
Member

Rebase for clarification of the state?

@danxuliu danxuliu force-pushed the add-specific-ui-for-recording-calls branch from ebe6585 to 8194631 Compare February 14, 2023 19:38
Instead of joining the call in the main Talk page now a specific page to
be used only by the recording server was added.

This will make possible to use a specific UI tailored for call
recording, but also to use an internal client of the external signaling
server rather than a normal participant.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When the call view is shown in recording mode the local participant is
not shown, and there is no visible button to show the grid stripe
either.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
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.

DIdn't test, but screenshot looks good.

* @param string $token
* @return TemplateResponse|NotFoundResponse
*/
public function recording(string $token = ''): Response {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public function recording(string $token = ''): Response {
public function recording(string $token): Response {

Doesn't need to be optional, since it's part of the URL already

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen merged commit 7e0012a into master Feb 17, 2023
@nickvergessen nickvergessen deleted the add-specific-ui-for-recording-calls branch February 17, 2023 06:24
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.

None yet

2 participants