fix(windows): auto-locate ffmpeg when not in PATH - #1188
Conversation
Add _find_ffmpeg() helper that searches common Windows installation locations (WinGet, Chocolatey, Scoop, manual installs) before falling back to a FileNotFoundError with an actionable message. This fixes MP3 export failing with "ffmpeg executable not found" for users who installed ffmpeg via WinGet or other package managers that don't always inject the binary into the PATH of child processes spawned by uv. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds Windows-compatible FFmpeg discovery to the audio utilities module by introducing a helper function that locates FFmpeg via PATH, Windows package managers (WinGet, Chocolatey, Scoop), and common installation directories. The MP3 export function now uses this discovered path and provides improved error messages if FFmpeg is not found. ChangesFFmpeg Discovery and Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
acestep/audio_utils.py (1)
200-223:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
FileNotFoundErrorfromtorchaudio.save()will be misreported as "ffmpeg not found".
torchaudio.save()(line 201) runs inside the sametryblock that catchesFileNotFoundErrorand re-raises it as"ffmpeg executable not found". If the soundfile backend, a missing codec, or a temp-directory eviction raisesFileNotFoundError, the real cause is silently replaced with a misleading ffmpeg message, making the failure undiagnosable in the field.Move
_find_ffmpeg()before thetryblock so the ffmpegFileNotFoundErroris propagated cleanly andtorchaudio.save()errors surface with their own messages.🐛 Proposed fix
+ ffmpeg_exe = _find_ffmpeg() # fail fast before writing the temp WAV with tempfile.NamedTemporaryFile(suffix=".wav", delete=False) as temp_wav: temp_wav_path = Path(temp_wav.name) try: torchaudio.save( str(temp_wav_path), tensor_to_save, int(target_sample_rate), channels_first=True, backend='soundfile', ) - ffmpeg_exe = _find_ffmpeg() cmd = [ ffmpeg_exe, '-y', '-hide_banner', '-loglevel', 'error',🤖 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 200 - 223, Move the ffmpeg lookup out of the broad try so a FileNotFoundError raised by torchaudio.save() isn't misattributed to ffmpeg; call _find_ffmpeg() (assign to ffmpeg_exe) before entering the try block, then use a narrower try/except around subprocess.run (or catch FileNotFoundError only for the ffmpeg invocation) to re-raise the helpful "ffmpeg executable not found" RuntimeError; update the block containing torchaudio.save, subprocess.run, and logger.debug accordingly so torchaudio.save errors surface with their original messages and only subprocess.run's FileNotFoundError is converted.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@acestep/audio_utils.py`:
- Around line 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.
---
Outside diff comments:
In `@acestep/audio_utils.py`:
- Around line 200-223: Move the ffmpeg lookup out of the broad try so a
FileNotFoundError raised by torchaudio.save() isn't misattributed to ffmpeg;
call _find_ffmpeg() (assign to ffmpeg_exe) before entering the try block, then
use a narrower try/except around subprocess.run (or catch FileNotFoundError only
for the ffmpeg invocation) to re-raise the helpful "ffmpeg executable not found"
RuntimeError; update the block containing torchaudio.save, subprocess.run, and
logger.debug accordingly so torchaudio.save errors surface with their original
messages and only subprocess.run's FileNotFoundError is converted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 653a58ec-48dc-43e0-86cd-5db92680d88d
📒 Files selected for processing (1)
acestep/audio_utils.py
| def _find_ffmpeg() -> str: | ||
| """Locate the ffmpeg executable, checking common installation paths on Windows if not in PATH.""" |
There was a problem hiding this comment.
🛠️ 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.
| 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.
1 participant
Problem
On Windows, when ACE-Step is launched via uv run (e.g. from start_gradio_ui.bat), the child Python process sometimes does not inherit the full user PATH. This causes MP3 export to fail with:
This happens even when ffmpeg is installed (via WinGet, Chocolatey, Scoop, or manually) but the binary isn't visible to the subprocess.
Solution
Add a _find_ffmpeg() helper in �udio_utils.py that:
The MP3 save code now calls _find_ffmpeg() instead of hardcoding 'ffmpeg'.
Testing
Reproduced on Windows 11 with ffmpeg installed via WinGet (winget install Gyan.FFmpeg) and ACE-Step launched through start_gradio_ui.bat. MP3 export now succeeds.
Summary by CodeRabbit