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

stream video to backend #260

Merged
merged 12 commits into from
Apr 22, 2022
Merged

stream video to backend #260

merged 12 commits into from
Apr 22, 2022

Conversation

elianiva
Copy link
Member

@elianiva elianiva commented Apr 17, 2022

nothing works yet

Here are some brief notes explaning how it works:

Frontend:

  • Requests permission when a user enters the /coding-test page.

    I have a question regarding this step, though. What if the user rejects the
    permission request? Do we ask them again? or do we simply don't allow them
    to go to the /coding-test page until they allow it?

  • Starts the video stream without audio in 360p and record it using MediaRecorder.

    We can't do compression so we're forced to use a low resolution (I think 360p is sensible)
    MediaRecorder will splice the video to 1 second / chunk. We can increase this if we want.
    I chose 1s because it's very small so it can be sent directly to the server.

  • Push the chunk into a queue inside a web worker.

    Just in case the internet isn't fast enough (which is very unlikely, it's like ~20-40kb per chunk,
    but who knows). We'll store the chunks inside a queue and post them sequentially to backend.

Backend:

  • Added Minio client as a singleton to ServiceCollection.

  • Made a VideoController to handle post request from client. Currently the path is POST /video.

  • The controller is fat since I'm not sure how to refactor the uploader logic into its own service.

  • It has the same authentication rules as ResumeExam in the event hub.

  • It will create a minio bucket if it doesn't exist.

  • Upload the file to Minio bucket using stream.

    We don't want to wait the backend to download the file and then upload it to Minio.
    That's too slow and inefficient.

  • The file is stored using time-started_time-ended.webm format.

    The reason why it's like this is because the file was chunked and
    the browser's MediaRecorder API only attach Matroska header to the first chunk.

    Possible solution is we can pluck the header and shove it to the next chunks, but I think
    it's better for a separate PR since we'll only need this for video compression.

    I've explained this here and here.
    I also attached some comments in the code.
    We can then join the video chunks based on their timestamps using an automated worker
    or just do it manually using ffmpeg. Whichever works better.

@elianiva elianiva added the scope: frontend Regarding the frontend side label Apr 17, 2022
@elianiva elianiva self-assigned this Apr 17, 2022
@elianiva elianiva mentioned this pull request Apr 18, 2022
49 tasks
@elianiva elianiva added the needs review This PR needs some user reviews label Apr 21, 2022
@elianiva elianiva changed the title [WIP] stream video to backend stream video to backend Apr 21, 2022
@elianiva elianiva requested review from aldy505 and ronnygunawan and removed request for aldy505 April 21, 2022 01:31
@aldy505
Copy link
Member

aldy505 commented Apr 21, 2022

I have a question regarding this step, though. What if the user rejects the permission request? Do we ask them again? or do we simply don't allow them to go to the /coding-test page until they allow it?

yes they can't continue. but don't repeatedly ask the permission.

@aldy505
Copy link
Member

aldy505 commented Apr 21, 2022

I haven't had the time to review the code. I just read the description for now.

Copy link
Contributor

@ronnygunawan ronnygunawan left a comment

Choose a reason for hiding this comment

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

Tech debt: fat controller

@ronnygunawan ronnygunawan merged commit ad16c5d into master Apr 22, 2022
@elianiva elianiva deleted the feat/video-stream branch April 24, 2022 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review This PR needs some user reviews scope: frontend Regarding the frontend side
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants