Skip to content

Conversation

@hjyssg
Copy link
Owner

@hjyssg hjyssg commented Sep 29, 2025

Summary

  • backend: initialize the video thumbnail service with a bundled Windows ffmpeg binary when available and fall back gracefully when missing
  • backend: wait for ffmpeg availability before spawning conversions and reuse the resolved binary for GIF generation
  • docs: document the resource folder where Windows users should place ffmpeg.exe

Testing

  • npm test (backend) (fails: path-util assertions expect Windows-style paths when running on Linux)

https://chatgpt.com/codex/tasks/task_e_68da63a33cf883258517611154e16fce

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

Comment on lines +51 to +58
function init() {
if (global.isWindows) {
const bundledPath = resolveBundledFfmpeg();
if (bundledPath) {
setFfmpegBinary(bundledPath);
} else {
logger.error('[video-thumbnail] Windows environment detected but no ffmpeg.exe bundled');
global._has_ffmpeg_ = false;

Choose a reason for hiding this comment

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

[P1] Fallback skips system ffmpeg on Windows

When init() runs on Windows and the bundled resource/ffmpeg/ffmpeg.exe is missing, the code only logs an error and sets _has_ffmpeg_ to false. It never attempts to use a system-wide ffmpeg from the PATH. As a result, video GIF generation is permanently disabled on Windows machines unless users copy an executable into the resource folder, even though the commit description claims the service should “fall back gracefully when missing”. Consider invoking setFfmpegBinary('ffmpeg') after the bundled lookup fails so installations with a pre-installed ffmpeg keep working.

Useful? React with 👍 / 👎.

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.

2 participants