Skip to content

fix(windows): auto-locate ffmpeg when not in PATH #1188

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.

Sign up for GitHub

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
jeremy10170 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(windows): auto-locate ffmpeg when not in PATH #1188

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 45 additions & 2 deletions acestep/audio_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import os
import subprocess
import hashlib
import shutil
import sys
import tempfile
from pathlib import Path
from typing import Union, Optional, List, Tuple
Expand All @@ -22,6 +24,43 @@
from loguru import logger


def _find_ffmpeg() -> str:
"""Locate the ffmpeg executable, checking common installation paths on Windows if not in PATH."""
Comment on lines +27 to +28

@coderabbitai coderabbitai Bot May 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Docstring is missing Returns and Raises sections.

Per coding guidelines, docstrings must include purpose, key inputs/outputs, and raised exceptions when relevant. _find_ffmpeg unconditionally raises FileNotFoundError when the executable is absent, which callers rely on.

✏️ Proposed docstring update 
-    """Locate the ffmpeg executable, checking common installation paths on Windows if not in PATH."""
+    """Locate the ffmpeg executable, checking common installation paths on Windows if not in PATH.
+
+    Returns:
+        Absolute path to the ffmpeg executable as a string.
+
+    Raises:
+        FileNotFoundError: If ffmpeg cannot be found via PATH or any known install location.
+    """

As per coding guidelines, "Docstrings are mandatory for all new or modified Python modules, classes, and functions. Docstrings must be concise and include purpose plus key inputs/outputs and raised exceptions when relevant."

📝 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
def _find_ffmpeg() -> str:
"""Locate the ffmpeg executable, checking common installation paths on Windows if not in PATH."""
def _find_ffmpeg() -> str:
"""Locate the ffmpeg executable, checking common installation paths on Windows if not in PATH.
Returns:
Absolute path to the ffmpeg executable as a string.
Raises:
FileNotFoundError: If ffmpeg cannot be found via PATH or any known install location.
"""
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@acestep/audio_utils.py` around lines 27 - 28, Update the _find_ffmpeg
function docstring to include explicit Returns and Raises sections: state that
it returns a str path to the ffmpeg executable when found and that it raises
FileNotFoundError if ffmpeg cannot be located; keep the existing short
description about checking PATH and common Windows install locations and mention
the exception is raised unconditionally when not found so callers can rely on
it.

found = shutil.which("ffmpeg")
if found:
return found

if sys.platform == "win32":
candidates: list[Path] = []

# WinGet packages (any Gyan.FFmpeg version)
winget_base = Path(os.environ.get("LOCALAPPDATA", "")) / "Microsoft" / "WinGet" / "Packages"
if winget_base.is_dir():
candidates.extend(p / "bin" for p in winget_base.glob("Gyan.FFmpeg*") if (p / "bin").is_dir())
# Also handle nested versioned sub-dirs (e.g. Gyan.FFmpeg_.../ffmpeg-X.Y-full_build/bin)
for pkg in winget_base.glob("Gyan.FFmpeg*"):
candidates.extend(p / "bin" for p in pkg.glob("ffmpeg-*") if (p / "bin").is_dir())

# Chocolatey
candidates.append(Path(os.environ.get("ChocolateyInstall", r"C:\ProgramData\chocolatey")) / "bin")

# Scoop
candidates.append(Path(os.environ.get("USERPROFILE", "")) / "scoop" / "shims")

# Common manual installs
for root in (Path("C:/ffmpeg"), Path("C:/tools/ffmpeg")):
candidates.append(root / "bin")
candidates.append(root)

for candidate in candidates:
exe = candidate / "ffmpeg.exe"
if exe.is_file():
logger.debug(f"[_find_ffmpeg] Found ffmpeg at {exe} (not in PATH)")
return str(exe)

raise FileNotFoundError("ffmpeg not found")


def apply_fade(
audio_data: Union[torch.Tensor, np.ndarray],
fade_in_samples: int = 0,
Expand Down Expand Up @@ -166,8 +205,9 @@ def _save_mp3(
channels_first=True,
backend='soundfile',
)
ffmpeg_exe = _find_ffmpeg()
cmd = [
'ffmpeg', '-y', '-hide_banner', '-loglevel', 'error',
ffmpeg_exe, '-y', '-hide_banner', '-loglevel', 'error',
'-i', str(temp_wav_path),
'-codec:a', 'libmp3lame',
'-ar', str(int(target_sample_rate)),
Expand All @@ -177,7 +217,10 @@ def _save_mp3(
subprocess.run(cmd, check=True, capture_output=True, timeout=120)
logger.debug(f"[AudioSaver] Saved audio to {output_path} (mp3, {target_sample_rate}Hz, {bitrate})")
except FileNotFoundError as e:
raise RuntimeError("ffmpeg executable not found. Install ffmpeg or add it to PATH to export MP3 files.") from e
raise RuntimeError(
"ffmpeg executable not found. Install ffmpeg and add it to PATH to export MP3 files. "
"See https://ffmpeg.org/download.html"
) from e
except subprocess.TimeoutExpired as e:
raise RuntimeError("ffmpeg MP3 export timed out after 120 seconds.") from e
except subprocess.CalledProcessError as e:
Expand Down