Skip to content

feat: improved video export function #13

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Raz-js
Copy link

@Raz-js Raz-js commented Mar 11, 2025

Used @KarimBenn22's PR as a foundation for this PR

What I changed:

  • Settings for higher quality recording output
  • The button now resets after the video is exported

Note: I'm also planning on attempting to find a method to allow the recorder to identify iterations and start and stop based on them.

Copy link

vercel bot commented Mar 11, 2025

@Raz-js is attempting to deploy a commit to the Paper Design Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

@Grenish Grenish left a comment

Choose a reason for hiding this comment

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

The code is looking good but there are still some minor bugs that might affect the app video export feature

Copy link

Choose a reason for hiding this comment

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

Not all browsers support vp9. Consider fallback

mimeType: MediaRecorder.isTypeSupported('video/webm;codecs=vp9') 
  ? 'video/webm;codecs=vp9' 
  : 'video/webm';

Also for the following code,

useEffect(() => {
    return () => {
        if (recordingRef.current) {
            recordingRef.current.stop();
        }
    };
}, []);

If the component unmounts mid-recording, this is helpful, but consider stopping the stopwatch too.

useEffect(() => {
    return () => {
        recordingRef.current?.stop();
        stopStopWatch();
    };
}, []);

Copy link

Choose a reason for hiding this comment

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

The code is pretty good and solid but there is one major bug that you might have not noticed. In the following code,

if (startTimeRef.current !== null) {
    const elapsedMS = Date.now() - startTimeRef.current;
    setTimeElapsed(Math.floor(elapsedMS / 1000));
}
setTimeElapsed((prev) => prev + 1);

You're updating timeElapsed twice:

  1. Once using actual time difference
  2. Once adding +1 manually

Below is the suggested fix for the code.

const updateTime = useCallback(() => {
    if (startTimeRef.current !== null) {
        const elapsedMS = Date.now() - startTimeRef.current;
        setTimeElapsed(Math.floor(elapsedMS / 1000));
    }
    rafRef.current = requestAnimationFrame(updateTime);
}, []);

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