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

Editor: Supports video rendering termination #28475

Merged

Conversation

ycw
Copy link
Contributor

@ycw ycw commented May 23, 2024

fix: #28454

With this PR, users can now terminate video rendering by closing the popup window

Details:

  • Checks popup window .closed before each FS (writefile) APIs, short-circuit if value is true (suggested by @aardgoose), in this case no further processing.

  • Calls ffmpeg.exit() on popup window unload, in order to terminate ongoing .run() (tested by @Mugen87), in this case no further processing.

    • NOTE: Calling ffmpeg.exit() won't stop queued FS (writeFile) calls, instead this is guarded by checking popup window .closed)

    • NOTE: There will be some errors logged in the console after calling ffmpeg.exit(), these errors can't be caught in sidebar.project.video.js because they were raised from a worker spawned from ffmpeg.js:

      screenshot

      uncatchable errors when ffmpeg exit called

  • Unlinks out.mp4 from ffmpeg.FS when teardown

  • Ensures that player.dispose() will be called no matter ffmpeg.exit()-ed or not

  • Handles setProgress with 2 special cases:

    1. ratio sometimes be Infinity when multi videos are being rendered at the same time
    2. ffmpeg.wasm/issues/178 (*I can't reproduce this though)

@mrdoob
Copy link
Owner

mrdoob commented May 24, 2024

Before is twas possible to render multiple videos at the same time.
Now that only one video is created I guess it's no longer possible?

@mrdoob mrdoob added this to the r165 milestone May 24, 2024
@ycw
Copy link
Contributor Author

ycw commented May 24, 2024

render multiple videos at the same time.

It's still support because a new ffmpeg context is created per 'RENDER' click.

BTW, progress callback ratio can sometimes be NaN, or Infinity when rendering multi video at the same time, this special case wasn't handled by current editor. This PR handled this case now.

@ycw ycw force-pushed the editor-terminates-video-rendering-when-popup-closed branch from 499f494 to d115584 Compare May 24, 2024 11:19
@mrdoob mrdoob merged commit 2ffccca into mrdoob:dev May 27, 2024
11 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.

Editor keeps rendering video on background even if users had stopped it
2 participants