fix(build): Fix FFmpeg cross-platform build on macOS for Windows targets

[mkdir700]

Summary

• Fix FFmpeg cross-platform build issue when building Windows packages on macOS
• Add proper environment variable support for target platform detection
• Ensure release builds include correct FFmpeg binaries for all platforms

Changes Made

🔧 Build System Fixes

• Add cross-env dependency: Support cross-platform environment variables
• Update Windows build scripts: Set BUILD_TARGET_PLATFORM=win32 and BUILD_TARGET_ARCH
• Modify FFmpeg download script: Prioritize environment variables over command arguments
• Fix vite plugin: Avoid duplicate downloads when FFmpeg already exists

🚀 Release Process Improvements

• Update all release commands: Download all platform FFmpeg binaries before building
• Add dedicated build:release command: Separate build process for releases
• Prevent duplicate downloads: Skip prebuild FFmpeg download in release mode

Problem Solved

Previously when building Windows packages on macOS:

• ❌ prebuild stage downloaded macOS FFmpeg (darwin-arm64)
• ❌ All Windows packages contained wrong FFmpeg binaries
• ❌ Windows users couldn't use FFmpeg functionality

Now:

• ✅ Build scripts detect target platform from environment variables
• ✅ Correct FFmpeg binaries downloaded for each target (win32-x64, win32-arm64)
• ✅ Release process downloads all platforms: win32, darwin, linux (x64 + arm64)

Test Results

Verified FFmpeg binaries are correct:

• win32-x64/ffmpeg.exe: PE32+ executable (console) x86-64, for MS Windows ✅
• win32-arm64/ffmpeg.exe: PE32+ executable (console) Aarch64, for MS Windows ✅
• darwin-*/ffmpeg: Mach-O 64-bit executable ✅

Test Plan

• Test individual platform builds: pnpm build:win:x64, pnpm build:win:arm64
• Test environment variable detection in download script
• Test release script downloads all platforms: npm run ffmpeg:download-all
• Verify FFmpeg binary architecture with file command
• Full release build test (requires manual verification)

Summary by CodeRabbit

• Bug Fixes

  • Correctly bundles the appropriate FFmpeg binary per OS/architecture.
  • Skips redundant FFmpeg downloads when already present, reducing build time and noise.
  • Adds clearer, platform-specific download logging.

• Chores

  • Streamlined release/build flow to pre-download FFmpeg before packaging.
  • Added cross-platform environment variable handling for Windows x64/ARM64 builds.
  • Introduced a dedicated release build script for consistent packaging.

• Documentation

  • Updated CLI help to describe environment variables for targeting FFmpeg downloads.

[coderabbitai]

Warning

Rate limit exceeded

@mkdir700 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 53 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between a814535 and 5e60953.

📒 Files selected for processing (1)

• electron.vite.config.ts (2 hunks)

Walkthrough

Adds environment-aware FFmpeg target detection and caching to the Electron Vite config, integrates env-driven fast-path into the FFmpeg download script, and revises package.json scripts to set targets via cross-env and pre-download FFmpeg during release builds.

Changes

Cohort / File(s)	Summary
Build config: FFmpeg target detection & caching electron.vite.config.ts	Computes platform/arch from BUILD_TARGET_PLATFORM/BUILD_TARGET_ARCH (fallback to process values), builds per-target ffmpegPath, skips download if present, and logs target-specific messages; retains existing download via TSX spawn within try/catch.
Build scripts & release workflow package.json	Adds build:release and prebuild:release; updates Windows build scripts to set BUILD_TARGET_PLATFORM/ARCH via cross-env; release scripts now pre-run ffmpeg:download-all then build:release; adds cross-env to devDependencies.
Downloader CLI: env fast-path scripts/download-ffmpeg.ts	If BUILD_TARGET_PLATFORM is set, immediately downloads FFmpeg for that platform/arch (arch defaults to process.arch), bypassing command parsing; otherwise preserves existing commands and updates help text to document env vars.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as npm/yarn script
  participant EVite as electron.vite.config
  participant FS as fs
  participant TSX as tsx downloader

  Dev->>EVite: Start build (env: BUILD_TARGET_PLATFORM/ARCH?)
  EVite->>EVite: Resolve target platform/arch
  EVite->>FS: Check ffmpegPath exists?
  alt Exists
    EVite-->>Dev: Log "FFmpeg already present" and continue
  else Missing
    EVite-->>Dev: Log "Downloading FFmpeg for {platform}-{arch}..."
    Dev->>TSX: Spawn download-ffmpeg.ts
    TSX-->>Dev: exit (success/error)
  end

Loading

sequenceDiagram
  autonumber
  participant CLI as scripts/download-ffmpeg.ts
  participant Env as process.env
  participant DL as downloader.downloadFFmpeg

  CLI->>Env: Read BUILD_TARGET_PLATFORM/ARCH
  alt Env target provided
    CLI-->>CLI: Determine arch (env or process.arch)
    CLI->>DL: Download for {platform}-{arch}
    DL-->>CLI: Result
  else No env target
    CLI-->>CLI: Parse command (all|current|clean|platform)
    CLI->>DL: Execute based on command
    DL-->>CLI: Result
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat(ffmpeg): integrate bundled FFmpeg with automatic fallback mechanism #112 — Also modifies FFmpeg download/build integration, overlapping on electron.vite.config.ts plugin logic, downloader behavior, and package.json script orchestration.

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly and accurately summarizes the primary change — fixing FFmpeg cross-platform builds when producing Windows targets on macOS by introducing environment-variable target detection and build/script adjustments; it directly reflects the PR objectives and modified files (vite config, download script, package.json). The phrasing is concise, specific, and avoids noisy or vague terms. It will be clear to reviewers scanning history what the main intent of the changeset is.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Poem

I thump the keys, a burrowed build,
FFmpeg fetched, the logs are stilled.
Platforms pegged, arches aligned,
Caches checked—no bytes resigned.
cross-env whispers, scripts take flight,
Release moon rises—ship tonight! 🐇✨

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-win-build-ffmpeg

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

scripts/download-ffmpeg.ts (1)

111-166: Client timeout isn’t applied; redirect handling incomplete and can leak sockets.

• Passing { timeout: 30000 } to https.get won’t trigger request 'timeout' unless request.setTimeout is used. Your request.on('timeout') never fires.
• Only 301/302 are followed; 303/307/308 are common for GitHub/CDN redirects.
• On redirect, the previous response isn’t drained/destroyed before recursing.

Fix:

-        const request = https.get(
+        const request = https.get(
           currentUrl,
           {
             headers: {
               'User-Agent': 'EchoPlayer-FFmpeg-Downloader/1.0'
             },
-            timeout: 30000
           },
           (response) => {
-            // 处理重定向
-            if (response.statusCode === 301 || response.statusCode === 302) {
+            // 处理重定向 (3xx)
+            if (response.statusCode && response.statusCode >= 300 && response.statusCode < 400) {
               const redirectUrl = response.headers.location
               if (redirectUrl) {
-                console.log(`重定向到: ${redirectUrl}`)
+                logger.info('Redirecting download', { redirectUrl })
+                // 清理当前响应，避免资源泄漏
+                response.resume()
                 download(redirectUrl, redirectCount + 1)
                 return
               }
             }
@@
-        request.on('error', reject)
-        request.on('timeout', () => {
-          request.destroy()
-          reject(new Error('下载超时'))
-        })
+        request.on('error', reject)
+        request.setTimeout(30_000, () => {
+          request.destroy(new Error('下载超时'))
+        })

Additionally, prefer logger over console in this file (see prior comment).

electron.vite.config.ts (1)

39-65: Spawn approach works; consider importing the downloader directly to avoid a build-time tool dependency on tsx.

Spawning 'tsx' assumes it’s on PATH. Importing the class avoids an extra process and PATH assumptions.

-        await new Promise<void>((resolve, reject) => {
-          const downloadScript = spawn(
-            'tsx',
-            ['scripts/download-ffmpeg.ts', 'platform', targetPlatform, targetArch],
-            {
-              stdio: 'inherit'
-            }
-          )
-          downloadScript.on('close', (code) => {
-            if (code === 0) {
-              console.log('FFmpeg Downloaded successfully')
-              resolve()
-            } else {
-              reject(new Error(`FFmpeg Download failed with exit code: ${code}`))
-            }
-          })
-          downloadScript.on('error', (error) => {
-            reject(error)
-          })
-        })
+        try {
+          const { FFmpegDownloader } = await import('./scripts/download-ffmpeg')
+          const dl = new FFmpegDownloader()
+          await dl.downloadFFmpeg(targetPlatform, targetArch)
+          logger.info('FFmpeg downloaded successfully', { platform: targetPlatform, arch: targetArch })
+        } catch (err) {
+          throw err
+        }

If you keep spawn, also switch logs:

-              console.log('FFmpeg Downloaded successfully')
+              logger.info('FFmpeg downloaded successfully', { platform: targetPlatform, arch: targetArch })
-        console.warn('FFmpeg Download failed', error)
+        logger.warn('FFmpeg download failed', { error })

package.json (1)

30-33: Ensure prebuild hooks run consistently on macOS/Linux builds.

build:mac and build:linux bypass the shared "build" script (and therefore prebuild), relying solely on the plugin to fetch FFmpeg. For parity with Windows and to keep one path, reuse "build".

-    "build:mac": "electron-vite build && electron-builder --mac --publish never",
+    "build:mac": "npm run build && electron-builder --mac --publish never",
-    "build:linux": "electron-vite build && electron-builder --linux --publish never",
+    "build:linux": "npm run build && electron-builder --linux --publish never",

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ac701b5 and a814535.

⛔ Files ignored due to path filters (1)

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (3)

• electron.vite.config.ts (1 hunks)
• package.json (4 hunks)
• scripts/download-ffmpeg.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: 在组件/Hook 中避免硬编码尺寸和时长，优先使用 useTheme() 的 token（如 motionDurationMid、borderRadiusSM/MD 等）或集中定义的样式变量
定制 antd 组件样式时优先使用 styled-components（styled(Component) 包装），避免通过全局 SCSS 与全局 className 覆盖
项目使用 Zustand 结合 Immer 中间件与自定义中间件栈（持久化、DevTools、订阅选择器）进行状态管理

Files:

• electron.vite.config.ts
• scripts/download-ffmpeg.ts

**/*.{tsx,ts}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{tsx,ts}: Zustand 必须在组件/Hook 顶层使用 selector（useStore(selector)）；禁止在 useMemo/useEffect 内部调用 store Hook
避免使用返回对象的 useStore 选择器（如 useStore(s => ({ a: s.a, b: s.b }))），应使用单字段选择器或配合 shallow 比较器

Files:

• electron.vite.config.ts
• scripts/download-ffmpeg.ts

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: 统一使用 loggerService 记录日志，禁止使用 console
logger 使用示例：logger.error('Error in :', { error }); 第二个参数必须是对象字面量 {}

Files:

• electron.vite.config.ts
• scripts/download-ffmpeg.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: build-windows
• GitHub Check: test (ubuntu-latest, 20)
• GitHub Check: test (windows-latest, 20)
• GitHub Check: test (macos-latest, 20)

🔇 Additional comments (3)

scripts/download-ffmpeg.ts (1)

396-400: Sane fallback to 'current'. LGTM.

package.json (2)

75-76: prebuild/prebuild:release contract is coherent.

prebuild downloads for normal builds; release path pre-downloads all and skips redundant prebuild. Good.

---

133-133: Dev dep cross-env addition: good.

Required for cross-platform env vars; no issues.

[coderabbitai]

🛠️ Refactor suggestion

Use a logger instead of console; early-exit check is good.

Replace console with a build-time logger to meet logging rules. Keep the path computation unchanged.

-      if (fs.existsSync(ffmpegPath)) {
-        console.log(`FFmpeg already exists for ${targetPlatform}-${targetArch}`)
+      if (fs.existsSync(ffmpegPath)) {
+        logger.info('FFmpeg already exists', { platform: targetPlatform, arch: targetArch })
         return
       }

Add at top:

+import logger from './scripts/build-logger'

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// 根据构建目标决定下载哪个平台
	const targetPlatform = process.env.BUILD_TARGET_PLATFORM || process.platform
	const targetArch = process.env.BUILD_TARGET_ARCH || process.arch
	// 检查是否已存在，避免重复下载
	const ffmpegPath = path.resolve(
	'resources/ffmpeg',
	`${targetPlatform}-${targetArch}`,
	targetPlatform === 'win32' ? 'ffmpeg.exe' : 'ffmpeg'
	)
	if (fs.existsSync(ffmpegPath)) {
	console.log(`FFmpeg already exists for ${targetPlatform}-${targetArch}`)
	return
	}
	import logger from './scripts/build-logger'
	// 根据构建目标决定下载哪个平台
	const targetPlatform = process.env.BUILD_TARGET_PLATFORM || process.platform
	const targetArch = process.env.BUILD_TARGET_ARCH || process.arch
	// 检查是否已存在，避免重复下载
	const ffmpegPath = path.resolve(
	'resources/ffmpeg',
	`${targetPlatform}-${targetArch}`,
	targetPlatform === 'win32' ? 'ffmpeg.exe' : 'ffmpeg'
	)
	if (fs.existsSync(ffmpegPath)) {
	logger.info('FFmpeg already exists', { platform: targetPlatform, arch: targetArch })
	return
	}

🤖 Prompt for AI Agents

In electron.vite.config.ts around lines 21 to 35, replace the console.log call
with your project's build-time logger: add an import/require for the build
logger at the top of the file (for example import { buildLogger } from
'<project-build-logger-path>') and change console.log(`FFmpeg already exists for
${targetPlatform}-${targetArch}`) to buildLogger.info(`FFmpeg already exists for
${targetPlatform}-${targetArch}`); keep the ffmpegPath computation and the
fs.existsSync early-exit logic unchanged.

[coderabbitai]

🧹 Nitpick (assertive)

Prefer logger for status messages.

-      console.log(`Downloading FFmpeg for ${targetPlatform}-${targetArch}...`)
+      logger.info('Downloading FFmpeg', { platform: targetPlatform, arch: targetArch })

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	console.log(`Downloading FFmpeg for ${targetPlatform}-${targetArch}...`)
	logger.info('Downloading FFmpeg', { platform: targetPlatform, arch: targetArch })

🤖 Prompt for AI Agents

In electron.vite.config.ts around line 37, replace the console.log status
message with the project's structured logger; use the existing logger instance
(e.g., logger.info or logger.debug) or import the shared logger module and call
it with the same message `Downloading FFmpeg for
${targetPlatform}-${targetArch}...`; ensure you remove the console.log call and
use the logger so messages follow the application's logging configuration and
formatting.

[coderabbitai]

🛠️ Refactor suggestion

Set BUILD_TARGET_ARCH for build:win to avoid arch mismatches; summary claims this but script doesn’t.

Currently only build:win:x64 and build:win:arm64 set the arch. build:win sets only the platform, which can make the plugin download the host-arch binary on macOS (often arm64) while electron-builder defaults to x64 for Windows—leading to wrong FFmpeg in the package.

-    "build:win": "cross-env BUILD_TARGET_PLATFORM=win32 npm run build && electron-builder --win --publish never",
+    "build:win": "cross-env BUILD_TARGET_PLATFORM=win32 BUILD_TARGET_ARCH=x64 npm run build && electron-builder --win --publish never",

Alternatively, drop build:win and keep only explicit arch variants to be unambiguous.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	"build:win": "cross-env BUILD_TARGET_PLATFORM=win32 npm run build && electron-builder --win --publish never",
	"build:win:x64": "cross-env BUILD_TARGET_PLATFORM=win32 BUILD_TARGET_ARCH=x64 npm run build && electron-builder --win --x64 --publish never",
	"build:win:arm64": "cross-env BUILD_TARGET_PLATFORM=win32 BUILD_TARGET_ARCH=arm64 npm run build && electron-builder --win --arm64 --publish never",
	"build:win": "cross-env BUILD_TARGET_PLATFORM=win32 BUILD_TARGET_ARCH=x64 npm run build && electron-builder --win --publish never",
	"build:win:x64": "cross-env BUILD_TARGET_PLATFORM=win32 BUILD_TARGET_ARCH=x64 npm run build && electron-builder --win --x64 --publish never",
	"build:win:arm64": "cross-env BUILD_TARGET_PLATFORM=win32 BUILD_TARGET_ARCH=arm64 npm run build && electron-builder --win --arm64 --publish never",

🤖 Prompt for AI Agents

In package.json around lines 26 to 28, the "build:win" script sets
BUILD_TARGET_PLATFORM but not BUILD_TARGET_ARCH which causes arch mismatches
(host arch vs electron-builder default); either add a BUILD_TARGET_ARCH (e.g.,
x64 or arm64) to the "build:win" script so it explicitly targets a single
Windows architecture, or remove the generic "build:win" script and keep only the
explicit "build:win:x64" and "build:win:arm64" variants to avoid ambiguity.

[coderabbitai]

🛠️ Refactor suggestion

Env fast-path is correct; replace console with logger to comply with guidelines.

Per repo rules, do not use console in TS/JS; route logs through a logger. Also keep the message structure consistent (second arg object literal).

Apply within this hunk:

-      console.log(
-        `检测到构建目标平台: ${process.env.BUILD_TARGET_PLATFORM}-${process.env.BUILD_TARGET_ARCH || process.arch}`
-      )
+      logger.info('Detected build target platform', {
+        platform: process.env.BUILD_TARGET_PLATFORM,
+        arch: process.env.BUILD_TARGET_ARCH || process.arch
+      })

Support code (new utility used in build/config/scripts):

// scripts/build-logger.ts
export const logger = {
  info: (msg: string, meta: Record<string, unknown> = {}) => require('node:console').log(msg, meta),
  warn: (msg: string, meta: Record<string, unknown> = {}) => require('node:console').warn(msg, meta),
  error: (msg: string, meta: Record<string, unknown> = {}) => require('node:console').error(msg, meta)
}
export default logger

And add at file top:

import logger from './build-logger'

🤖 Prompt for AI Agents

In scripts/download-ffmpeg.ts around lines 384 to 395, replace the console.log
call in the BUILD_TARGET_PLATFORM fast-path with the project logger: add import
logger from './build-logger' at the top of the file (if not already present),
then change the console.log(...) to logger.info('检测到构建目标平台', { platform:
process.env.BUILD_TARGET_PLATFORM, arch: process.env.BUILD_TARGET_ARCH ||
process.arch }) and keep the subsequent await downloader.downloadFFmpeg(...)
call unchanged; ensure no other console.* calls remain in this hunk.

[coderabbitai]

🧹 Nitpick (assertive)

Help text: add note that env overrides CLI args.

Small clarity win: explicitly state that BUILD_TARGET_* takes precedence over command arguments when both are provided.

   BUILD_TARGET_PLATFORM - 构建目标平台 (win32, darwin, linux)
   BUILD_TARGET_ARCH     - 构建目标架构 (x64, arm64)
+  注: 若同时传入命令参数与上述环境变量，将优先使用环境变量

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	环境变量:
	BUILD_TARGET_PLATFORM - 构建目标平台 (win32, darwin, linux)
	BUILD_TARGET_ARCH - 构建目标架构 (x64, arm64)
	`)
	环境变量:
	BUILD_TARGET_PLATFORM - 构建目标平台 (win32, darwin, linux)
	BUILD_TARGET_ARCH - 构建目标架构 (x64, arm64)
	注: 若同时传入命令参数与上述环境变量，将优先使用环境变量
	`)

🤖 Prompt for AI Agents

In scripts/download-ffmpeg.ts around lines 436-440, the help text listing
BUILD_TARGET_* env vars should explicitly state that these environment variables
override any CLI arguments; update the help text by appending a short clarifying
sentence (in the same language/format as the surrounding text) indicating that
BUILD_TARGET_PLATFORM and BUILD_TARGET_ARCH take precedence over command-line
options when both are provided.