feat(ffmpeg): add ffmpeg manager, bundle/download ffmpeg in CI and (#11)

* feat(ffmpeg): add ffmpeg manager, bundle/download ffmpeg in CI and * fix(download-engine): let yt-dlp merge format to avoid codec conflicts and use actual format for filenames * chore(ci): replace monolithic job with platform-specific reusable jobs * chore(ci): run build matrix for windows, macos, linux in one to * feat(ci): remove platform matrix and platform gating to simplify build * chore(ci): pin github-translate-action to specific commit to ensure
2025-11-02 17:42:33 +08:00
parent 75c8b19f31
commit 40113a2b53
15 changed files with 810 additions and 146 deletions
--- a/src/main/download-engine/args-builder.ts
+++ b/src/main/download-engine/args-builder.ts
@@ -14,7 +14,8 @@ export const resolveVideoFormatSelector = (options: DownloadOptions): string =>
      return 'bestvideo+none'
    }
    if (!audioFormat || audioFormat === 'best') {
-      return 'best'
+      // Use bestvideo+bestaudio to ensure video and audio are merged into a single file
+      return 'bestvideo+bestaudio'
    }
    return `bestvideo+${audioFormat}`
  }
@@ -53,7 +54,11 @@ export const buildDownloadArgs = (

  // Format selection
  if (options.type === 'video') {
-    args.push('-f', resolveVideoFormatSelector(options))
+    const formatSelector = resolveVideoFormatSelector(options)
+    args.push('-f', formatSelector)
+    // Let yt-dlp automatically choose the best merge format (mkv/webm/mp4)
+    // based on codec compatibility. Forcing MP4 can cause failures
+    // when codecs are incompatible (e.g., VP9+Opus requires mkv/webm)
  } else if (options.type === 'audio') {
    args.push('-f', resolveAudioFormatSelector(options))
  } else if (options.type === 'extract') {
--- a/src/main/index.ts
+++ b/src/main/index.ts
@@ -7,6 +7,7 @@ import appIcon from '../../build/icon.png?asset'
 import { configureLogger } from './config/logger-config'
 import { services } from './ipc'
 import { downloadEngine } from './lib/download-engine'
+import { ffmpegManager } from './lib/ffmpeg-manager'
 import { ytdlpManager } from './lib/ytdlp-manager'
 import { settingsManager } from './settings'
 import { createTray, destroyTray } from './tray'
@@ -179,6 +180,15 @@ app.whenReady().then(async () => {
  // IPC services are automatically registered by electron-ipc-decorator when imported
  log.info('IPC services available:', Object.keys(services))

+  // Initialize ffmpeg
+  try {
+    log.info('Initializing ffmpeg...')
+    await ffmpegManager.initialize()
+    log.info('ffmpeg initialized successfully')
+  } catch (error) {
+    log.error('Failed to initialize ffmpeg:', error)
+  }
+
  // Initialize yt-dlp
  try {
    log.info('Initializing yt-dlp...')
--- a/src/main/lib/download-engine.ts
+++ b/src/main/lib/download-engine.ts
@@ -12,7 +12,7 @@ import type {
  VideoFormat,
  VideoInfo
 } from '../../shared/types'
-import { buildDownloadArgs } from '../download-engine/args-builder'
+import { buildDownloadArgs, resolveVideoFormatSelector } from '../download-engine/args-builder'
 import {
  findFormatByIdCandidates,
  parseSizeToBytes,
@@ -21,6 +21,7 @@ import {
 import { settingsManager } from '../settings'
 import { scopedLoggers } from '../utils/logger'
 import { DownloadQueue } from './download-queue'
+import { ffmpegManager } from './ffmpeg-manager'
 import { historyManager } from './history-manager'
 import { ytdlpManager } from './ytdlp-manager'

@@ -496,6 +497,46 @@ class DownloadEngine extends EventEmitter {

    const args = buildDownloadArgs(options, downloadPath, settings)

+    // Check if format selector contains '+' which means video and audio will be merged
+    const formatSelector =
+      options.type === 'video' ? resolveVideoFormatSelector(options) : undefined
+    const willMerge = formatSelector?.includes('+') ?? false
+
+    const urlArg = args.pop()
+    if (!urlArg) {
+      const missingUrlError = new Error('Download arguments missing URL.')
+      scopedLoggers.download.error('Missing URL argument for download ID:', id)
+      this.updateDownloadInfo(id, {
+        status: 'error',
+        completedAt: Date.now(),
+        error: missingUrlError.message
+      })
+      this.queue.downloadCompleted(id)
+      this.emit('download-error', id, missingUrlError)
+      this.addToHistory(id, options, 'error', missingUrlError.message)
+      return
+    }
+
+    let ffmpegPath: string
+    try {
+      ffmpegPath = ffmpegManager.getPath()
+    } catch (error) {
+      const ffmpegError = error instanceof Error ? error : new Error(String(error))
+      scopedLoggers.download.error('Failed to resolve ffmpeg for download ID:', id, ffmpegError)
+      this.updateDownloadInfo(id, {
+        status: 'error',
+        completedAt: Date.now(),
+        error: ffmpegError.message
+      })
+      this.queue.downloadCompleted(id)
+      this.emit('download-error', id, ffmpegError)
+      this.addToHistory(id, options, 'error', ffmpegError.message)
+      return
+    }
+
+    args.push('--ffmpeg-location', ffmpegPath)
+    args.push(urlArg)
+
    const controller = new AbortController()
    const ytdlpProcess = ytdlp.exec(args, {
      signal: controller.signal
@@ -593,23 +634,81 @@ class DownloadEngine extends EventEmitter {
        // Generate file path using downloadPath + title + ext
        const title = videoInfo?.title || 'Unknown'
        const sanitizedTitle = title.replace(/[<>:"/\\|?*]/g, '_').substring(0, 50)
-        const extension =
-          options.type === 'audio' ? options.extractFormat || 'mp3' : actualFormat || 'mp4'
+
+        // Determine file extension based on download type and format
+        // yt-dlp automatically chooses the best merge format (mkv/webm/mp4)
+        // based on codec compatibility, so we should use actualFormat when available
+        let extension: string
+        if (options.type === 'audio') {
+          extension = options.extractFormat || 'mp3'
+        } else if (willMerge) {
+          // For merged files, yt-dlp auto-selects format (mkv/webm/mp4)
+          // Use actualFormat if available, otherwise default to mkv (most compatible)
+          extension = actualFormat || 'mkv'
+        } else {
+          extension = actualFormat || 'mp4'
+        }
+
        const fileName = `${sanitizedTitle}.${extension}`
        const finalOutputPath = path.join(downloadPath, fileName)

-        scopedLoggers.download.info('Generated file path for ID:', id, 'Path:', finalOutputPath)
+        scopedLoggers.download.info(
+          'Generated file path for ID:',
+          id,
+          'Path:',
+          finalOutputPath,
+          'Will merge:',
+          willMerge
+        )

        let fileSize: number | undefined
+        let actualFilePath = finalOutputPath
        try {
          const fs = await import('node:fs/promises')
+          // Try to find the actual file - yt-dlp may generate files with slightly different names
          const stats = await fs.stat(finalOutputPath)
          fileSize = stats.size
+          actualFilePath = finalOutputPath
        } catch (error) {
-          if (latestKnownSizeBytes !== undefined) {
-            fileSize = latestKnownSizeBytes
-          } else {
-            scopedLoggers.download.warn('Failed to get file size for ID:', id, error)
+          // If the expected file doesn't exist, try to find it by scanning the directory
+          try {
+            const fs = await import('node:fs/promises')
+            const files = await fs.readdir(downloadPath)
+            // Look for files matching the title pattern with the correct extension
+            const matchingFiles = files.filter((file) => {
+              const baseName = file.replace(/\.[^.]+$/, '')
+              const fileExt = file.split('.').pop()?.toLowerCase()
+              return (
+                (baseName === sanitizedTitle || baseName.startsWith(sanitizedTitle)) &&
+                fileExt === extension.toLowerCase()
+              )
+            })
+
+            if (matchingFiles.length > 0) {
+              // Use the most recently modified file if multiple matches
+              const fileStats = await Promise.all(
+                matchingFiles.map(async (file) => {
+                  const filePath = path.join(downloadPath, file)
+                  const stats = await fs.stat(filePath)
+                  return { file, path: filePath, mtime: stats.mtime, size: stats.size }
+                })
+              )
+              const mostRecent = fileStats.sort((a, b) => b.mtime.getTime() - a.mtime.getTime())[0]
+              actualFilePath = mostRecent.path
+              fileSize = mostRecent.size
+              scopedLoggers.download.info('Found actual file:', actualFilePath, 'Size:', fileSize)
+            } else if (latestKnownSizeBytes !== undefined) {
+              fileSize = latestKnownSizeBytes
+              scopedLoggers.download.warn('File not found, using estimated size:', fileSize)
+            } else {
+              scopedLoggers.download.warn('Failed to find file for ID:', id, error)
+            }
+          } catch (scanError) {
+            if (latestKnownSizeBytes !== undefined) {
+              fileSize = latestKnownSizeBytes
+            } else {
+              scopedLoggers.download.warn('Failed to get file size for ID:', id, scanError)
+            }
          }
        }

@@ -621,7 +720,7 @@ class DownloadEngine extends EventEmitter {
          status: 'completed',
          completedAt: Date.now(),
          fileSize,
-          format: actualFormat || undefined,
+          format: willMerge ? 'mp4' : actualFormat || undefined,
          quality: actualQuality || undefined,
          codec: actualCodec || undefined
        })
--- a/src/main/lib/ffmpeg-manager.ts
+++ b/src/main/lib/ffmpeg-manager.ts
@@ -0,0 +1,101 @@
+import { execSync } from 'node:child_process'
+import fs from 'node:fs'
+import os from 'node:os'
+import path from 'node:path'
+
+class FfmpegManager {
+  private ffmpegPath: string | null = null
+
+  async initialize(): Promise<void> {
+    this.ffmpegPath = await this.findFfmpegBinary()
+    console.log('ffmpeg initialized at:', this.ffmpegPath)
+  }
+
+  getPath(): string {
+    if (!this.ffmpegPath) {
+      throw new Error('ffmpeg not initialized. Call initialize() first.')
+    }
+    return this.ffmpegPath
+  }
+
+  private getResourcesPath(): string {
+    if (process.env.NODE_ENV === 'development') {
+      return path.join(process.cwd(), 'resources')
+    }
+    return path.join(process.resourcesPath, 'app.asar.unpacked', 'resources')
+  }
+
+  private async findFfmpegBinary(): Promise<string> {
+    const platform = os.platform()
+    const resourceCandidates: string[] = []
+
+    if (process.env.FFMPEG_PATH && fs.existsSync(process.env.FFMPEG_PATH)) {
+      console.log('Using ffmpeg from FFMPEG_PATH:', process.env.FFMPEG_PATH)
+      return process.env.FFMPEG_PATH
+    }
+
+    if (platform === 'win32') {
+      resourceCandidates.push('ffmpeg.exe')
+    } else if (platform === 'darwin') {
+      resourceCandidates.push('ffmpeg_macos', 'ffmpeg')
+    } else {
+      resourceCandidates.push('ffmpeg_linux', 'ffmpeg')
+    }
+
+    const resourcesPath = this.getResourcesPath()
+    for (const candidate of resourceCandidates) {
+      const fullPath = path.join(resourcesPath, candidate)
+      if (fs.existsSync(fullPath)) {
+        if (platform !== 'win32') {
+          try {
+            fs.chmodSync(fullPath, 0o755)
+          } catch (error) {
+            console.warn('Failed to set executable permission on ffmpeg binary:', error)
+          }
+        }
+        console.log('Using bundled ffmpeg:', fullPath)
+        return fullPath
+      }
+    }
+
+    if (platform === 'darwin') {
+      const commonPaths = ['/opt/homebrew/bin/ffmpeg', '/usr/local/bin/ffmpeg']
+      for (const candidate of commonPaths) {
+        if (fs.existsSync(candidate)) {
+          console.log('Using system ffmpeg:', candidate)
+          return candidate
+        }
+      }
+    }
+
+    if (platform === 'linux' || platform === 'freebsd') {
+      try {
+        const systemPath = execSync('which ffmpeg').toString().trim()
+        if (systemPath && fs.existsSync(systemPath)) {
+          console.log('Using system ffmpeg:', systemPath)
+          return systemPath
+        }
+      } catch (_error) {
+        // Ignore error and continue
+      }
+    }
+
+    if (platform === 'win32') {
+      try {
+        const output = execSync('where ffmpeg').toString().split(/\r?\n/)[0]
+        if (output && fs.existsSync(output)) {
+          console.log('Using system ffmpeg:', output)
+          return output
+        }
+      } catch (_error) {
+        // Ignore error and continue
+      }
+    }
+
+    throw new Error(
+      'ffmpeg not found. Bundle it under resources/ (asarUnpack) or set the FFMPEG_PATH environment variable.'
+    )
+  }
+}
+
+export const ffmpegManager = new FfmpegManager()
--- a/src/renderer/src/pages/Home.tsx
+++ b/src/renderer/src/pages/Home.tsx
@@ -76,18 +76,20 @@ const getQualityPreset = (settings: AppSettings): OneClickQualityPreset =>

 const buildAudioSelectors = (preset: OneClickQualityPreset): string[] => {
  if (preset === 'worst') {
-    return ['worstaudio', 'worst']
+    return ['worstaudio']
  }

  const abrLimit = qualityPresetToAudioAbr[preset]
-  return dedupe([abrLimit ? `bestaudio[abr<=${abrLimit}]` : undefined, 'bestaudio', 'best'])
+  // Remove 'best' fallback to ensure merging - only use 'bestaudio' variants
+  return dedupe([abrLimit ? `bestaudio[abr<=${abrLimit}]` : undefined, 'bestaudio'])
 }

 const buildVideoFormatPreference = (settings: AppSettings): string => {
  const preset = getQualityPreset(settings)

  if (preset === 'worst') {
-    return 'worstvideo+worstaudio/worst'
+    // Use worstvideo+worstaudio as fallback instead of 'worst' to ensure merging
+    return 'worstvideo+worstaudio'
  }

  const maxHeight = qualityPresetToVideoHeight[preset]
@@ -110,7 +112,8 @@ const buildVideoFormatPreference = (settings: AppSettings): string => {
      combinations.push(video)
    }
  } else {
-    combinations.push('best')
+    // Use bestvideo+bestaudio as fallback instead of 'best' to ensure merging
+    combinations.push('bestvideo+bestaudio')
  }

  return dedupe(combinations).join('/')