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: Communication Nice To Have #64

Merged
merged 13 commits into from
Nov 4, 2024
Merged

feat: Communication Nice To Have #64

merged 13 commits into from
Nov 4, 2024

Conversation

bensohh
Copy link

@bensohh bensohh commented Nov 3, 2024

Main Changes
Integrate a video & audio functionality to replace the existing chat template. This is integrated using PeerJS and their online default server, we set the user Call ID and matched user Call ID to the following:

  • username + "-" + collabId
  • matchedUsername + "-" + collabId
    (This is to make their ID unique and prevent random users using the same PeerJS server from joining. An alternative is that we can also host our own PeerJS server as a microservice?)

The video & audio functionality includes:

  • On/Off Webcam
  • On/Off Mic
  • Call MatchedUser (Either user can initiate the video call)

User/Activity Flow
When the user is redirected to the collaboration page, they will be prompted by the browser to allow access for webcam and microphone. They should be able to see themselves. Only after one of the user initiate call then will both user see one another. Each user has control over their own video & audio.

How to test?
If you only have a single webcam, you should see that both screens should show the same stream (aka see yourself)
If you happen to have an additional webcam, you can use two browsers, for example arc and chrome, set the default camera of each browser respectively and you should be able to see that the screen display different streams.

Additional Changes:

  • Change the default code editor to python
  • Comment out the isValidToken check within frontend's middleware.ts

Bug Fixes:

  • Bug: When a user close his/her browser or tab, the matched user is unaware and stays in the collaboration session
  • Fix: When matched user detects that he/she is the only active user in the session, setTimeout for a end session function is triggered (~15s). Within the 15s, if a user reconnects, we abort the end session.

@bensohh bensohh marked this pull request as ready for review November 3, 2024 06:10
@bensohh bensohh self-assigned this Nov 3, 2024
Copy link

@tituschewxj tituschewxj left a comment

Choose a reason for hiding this comment

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

Thanks for implementing the video and audio communication. Generally looks ok. I have left some suggestions here. I have approved the PR so you can merge when you are ready.

UI Issues

A small improvement may be to toggle the "CALL" button to a "END CALL" button once a call starts, and we may also want to toggle the color of this button for better visual impact.

Another improvement to the UI may be to round the corners of the video.

There is an UI issue of the container being too large, may want to fit the video on the screen by resizing the dimensions of the video?

image

Other Issues

I found a bug where the video call doesn't work on Firefox but works on chromium browsers, we may want to look into this or add a note that it doesn't work on Firefox.

@bensohh
Copy link
Author

bensohh commented Nov 4, 2024

Thanks for implementing the video and audio communication. Generally looks ok. I have left some suggestions here. I have approved the PR so you can merge when you are ready.

UI Issues

A small improvement may be to toggle the "CALL" button to a "END CALL" button once a call starts, and we may also want to toggle the color of this button for better visual impact.

Another improvement to the UI may be to round the corners of the video.

There is an UI issue of the container being too large, may want to fit the video on the screen by resizing the dimensions of the video?

image

Other Issues

I found a bug where the video call doesn't work on Firefox but works on chromium browsers, we may want to look into this or add a note that it doesn't work on Firefox.

@tituschewxj

  • I added the end call functionality to the video communication feature (which was previously not available/not implemented)
  • Amend the styling to fit screen with 16:9 aspect ratio, if the screen is super wide with narrow width, the same error might occur since we primarily focus on creating the app for 13 inch screen.
  • Fix error with echoing & null error for getSenders

@bensohh bensohh merged commit 521ac4a into staging Nov 4, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants