This PR resolves a critical runtime crash during the audio export stage on Linux systems, specifically occurring in Cloud environments like Lightning AI.#1089
Conversation
… ai. - Patched acestep/audio_utils.py to force FLAC as default export format. - Injected LD_LIBRARY_PATH to resolve FFmpeg library discovery issues. - Fixed symbol errors in PyTorch 2.10+ and updated INSTALL.md.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAudioSaver.save_audio now checks Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller (UI / Code)
participant Env as OS Env
participant AudioSaver as AudioSaver.save_audio
participant Torchaudio as torchaudio / backend
Caller->>AudioSaver: request save_audio(buffer, format, channels_first)
Env->>AudioSaver: ACESTEP_FORCE_FLAC_EXPORT present?
alt ACESTEP_FORCE_FLAC_EXPORT == "1"
AudioSaver->>AudioSaver: override format = "flac"
end
AudioSaver->>Torchaudio: save(buffer, format, channels_first)
Torchaudio-->>AudioSaver: success / error
AudioSaver-->>Caller: return result / raise
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (1)
acestep/audio_utils.py (1)
310-318: Dead code:elsebranch is unreachable.All valid format values are handled by the preceding branches:
"mp3"→ line 266"opus","aac"→ line 275"flac","wav","wav32"→ line 284Since the format validation on lines 223-225 already normalizes invalid formats to
self.default_format(which must be one of the valid formats), thiselsebranch can never execute.🧹 Remove unreachable code
torchaudio.save( str(output_path), audio_tensor, sample_rate, channels_first=True, backend='soundfile', ) - else: - # Other formats use default backend - torchaudio.save( - str(output_path), - audio_tensor, - sample_rate, - channels_first=channels_first, - backend="soundfile" # <--- ADD THIS LINE - ) -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/audio_utils.py` around lines 310 - 318, Remove the unreachable else branch that calls torchaudio.save(...) with backend="soundfile" in the audio saving logic: the format cases ("mp3", "opus"/"aac", "flac"/"wav"/"wav32") and the prior normalization to self.default_format already cover all valid formats, so delete the entire else block (the torchaudio.save call with backend="soundfile") from the function containing the audio save logic to eliminate dead code; if you intended a default backend, instead set it where formats are normalized or in the relevant format-specific branches (refer to the existing format-handling branches around the torchaudio.save calls).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@acestep/audio_utils.py`:
- Line 26: The module acquires an unused import `sys` which should be removed to
avoid lint warnings; delete the `import sys` statement from the top of
acestep/audio_utils.py unless you actually need `sys` for a platform check—if
you do need it, move the usage into the platform-checking codepath and reference
`sys` there instead of leaving the unused import.
- Around line 222-225: The code currently overwrites the caller-supplied format
by unconditionally setting format = "flac"; remove that hardcoded assignment so
the function respects the incoming format parameter, only falling back to
self.default_format when format is None or empty (e.g., format = format or
self.default_format). Keep the validation check and logger.warning for
unsupported values (use the same logger.warning call that references
self.default_format), and if you need an opt-in forced-FLAC behavior implement
it via an environment variable or constructor flag rather than overwriting the
parameter; check places that call this function (inference.py and
generation_progress.py) still pass their audio_format through.
- Around line 14-16: The module-level hardcoded LD_LIBRARY_PATH assignment
should be removed and replaced with a guarded, platform-aware initialization
that runs early (before any audio libs are imported) — implement a helper (e.g.,
init_ffmpeg_ld_library_path) and call it from your application entrypoint or
package __init__.py before importing torchaudio/soundfile; in that helper check
platform.machine()/sys.platform and append the appropriate path
("/usr/lib/x86_64-linux-gnu" or "/usr/lib/aarch64-linux-gnu") only if not
already present, and avoid touching LD_LIBRARY_PATH on non-Linux platforms so
you don’t pollute other OS environments or rely on import-time side effects.
In `@docs/en/INSTALL.md`:
- Around line 137-144: Remove the internal implementation notes currently
inserted into the user-facing INSTALL.md (the lines mentioning "In
acestep/audio_utils.py", os.environ["LD_LIBRARY_PATH"], save_audio,
torchaudio.save and backend="soundfile") and replace them with the intended bash
snippet only; also close or remove the empty ```bash code block so the Linux
launch scripts section flows into the Git installation note and keep the
IMPORTANT cloud-user note intact. Ensure no references to code symbols
(os.environ, save_audio, torchaudio.save) remain in the user guide—keep only
actionable install commands and notes.
---
Nitpick comments:
In `@acestep/audio_utils.py`:
- Around line 310-318: Remove the unreachable else branch that calls
torchaudio.save(...) with backend="soundfile" in the audio saving logic: the
format cases ("mp3", "opus"/"aac", "flac"/"wav"/"wav32") and the prior
normalization to self.default_format already cover all valid formats, so delete
the entire else block (the torchaudio.save call with backend="soundfile") from
the function containing the audio save logic to eliminate dead code; if you
intended a default backend, instead set it where formats are normalized or in
the relevant format-specific branches (refer to the existing format-handling
branches around the torchaudio.save calls).
🪄 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: c9564f65-145f-4abf-8b5f-c5f550e642ad
📒 Files selected for processing (2)
acestep/audio_utils.pydocs/en/INSTALL.md
|
Thanks for tackling this — the torchcodec/Lightning AI cloud-GPU pain is real and worth fixing. However, I went through the diff carefully and I think this PR can't be merged as-is. There are two functional blockers plus a broken docs block. I verified every finding against current 🔴 Blocker 1 —
|
- Restore original format selection logic and add optional FLAC override via ACESTEP_FORCE_FLAC_EXPORT env var - Remove ineffective backend change in unreachable branch - Remove module-level LD_LIBRARY_PATH modification - Update INSTALL.md with proper FFmpeg dependency setup and environment variable guidance - Fix broken markdown code blocks in documentation - Remove unused import This keeps user format choice intact while providing a safe workaround for torchcodec issues in cloud environments.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Line 52: Change the default for ACESTEP_FORCE_FLAC_EXPORT in the sample env so
forced FLAC is opt-in: update the ACESTEP_FORCE_FLAC_EXPORT entry to a disabled
state (e.g., set to 0 or comment out the line) instead of 1 and add a short
inline comment noting that enabling it is optional/experimental; ensure the
variable name ACESTEP_FORCE_FLAC_EXPORT is preserved so consumers can opt in if
needed.
In `@acestep/audio_utils.py`:
- Around line 220-222: The current force_flac branch sets format = "flac" but
leaves output_path's filename/extension unchanged, causing extension/content
mismatches; when ACESTEP_FORCE_FLAC_EXPORT is "1" (force_flac), update
output_path to match the forced format (e.g., use
pathlib.Path(output_path).with_suffix(f".{format}") or rebuild the filename from
format) and ensure any downstream suffix normalization logic (the block around
output_path and its suffix handling) uses the updated format so saved bytes and
file extension are consistent; reference the force_flac variable, format
assignment, and output_path handling in your change.
🪄 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: 1e533094-2683-4d7e-a791-0c91cf16c374
📒 Files selected for processing (3)
.env.exampleacestep/audio_utils.pydocs/en/INSTALL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/en/INSTALL.md
| force_flac = os.getenv("ACESTEP_FORCE_FLAC_EXPORT", "0") == "1" | ||
| if force_flac: | ||
| format = "flac" |
There was a problem hiding this comment.
Forced FLAC can produce extension/content mismatch (e.g., .mp3 file containing FLAC).
At Line 221, format is forced to "flac", but if output_path already has .mp3/.wav (Line 233), suffix normalization won’t rewrite it. With upstream callers prebuilding names from requested format, this can save FLAC bytes under non-FLAC extensions.
Proposed fix
force_flac = os.getenv("ACESTEP_FORCE_FLAC_EXPORT", "0") == "1"
if force_flac:
- format = "flac"
+ format = "flac"
# Ensure output path has correct extension
output_path = Path(output_path)
+ if force_flac and output_path.suffix.lower() != ".flac":
+ output_path = output_path.with_suffix(".flac")Also applies to: 228-240
🧰 Tools
🪛 Ruff (0.15.9)
[error] 222-222: Variable format is shadowing a Python builtin
(A001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acestep/audio_utils.py` around lines 220 - 222, The current force_flac branch
sets format = "flac" but leaves output_path's filename/extension unchanged,
causing extension/content mismatches; when ACESTEP_FORCE_FLAC_EXPORT is "1"
(force_flac), update output_path to match the forced format (e.g., use
pathlib.Path(output_path).with_suffix(f".{format}") or rebuild the filename from
format) and ensure any downstream suffix normalization logic (the block around
output_path and its suffix handling) uses the updated format so saved bytes and
file extension are consistent; reference the force_flac variable, format
assignment, and output_path handling in your change.
|
Hello, Changes made1. Restored format selection logic
2. Removed ineffective backend modification
3. Removed module-level LD_LIBRARY_PATH modification
4. Documentation fixes
5. Minor cleanup
VerificationTested on:
Results:
This keeps the fix aligned with the intended design:
Happy to make further adjustments if needed. |
|
Thanks @sobiswriter — the revised diff addresses the original blockers cleanly. The opt-in Two small things still need a touch-up before this can land: 1.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
acestep/audio_utils.py (1)
219-221:⚠️ Potential issue | 🔴 CriticalForce-FLAC can still write FLAC data under non-FLAC extensions.
When
ACESTEP_FORCE_FLAC_EXPORT=1(Line 219–221),formatbecomes FLAC, but existing known suffixes (Line 232) are preserved. A.mp3path can therefore contain FLAC bytes.💡 Proposed fix
force_flac = os.getenv("ACESTEP_FORCE_FLAC_EXPORT", "0") == "1" if force_flac: - format = "flac" + format = "flac" # Ensure output path has correct extension output_path = Path(output_path) # Determine extension based on format ext = ".wav" if format == "wav32" else f".{format}" + if force_flac and output_path.suffix.lower() != ext: + output_path = output_path.with_suffix(ext) if output_path.suffix.lower() not in [".flac", ".wav", ".mp3", ".opus", ".aac", ".m4a"]: output_path = output_path.with_suffix(ext)Also applies to: 227-239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@acestep/audio_utils.py` around lines 219 - 221, The force-FLAC option sets the encoding variable format = "flac" (ACESTEP_FORCE_FLAC_EXPORT) but does not change the output filename/suffix, which can lead to FLAC bytes being written to e.g. .mp3 files; update the export path logic (where known suffixes are handled and where format is used, e.g., the variable format and any code that builds output_path or preserves existing suffixes) so that when format == "flac" you also set the file extension to ".flac" (use Path.with_suffix or os.path.splitext to replace the suffix), and apply the same fix in the other block referenced (lines around 227–239) to ensure filename suffix always matches the chosen format.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@acestep/audio_utils.py`:
- Around line 219-221: The force-FLAC option sets the encoding variable format =
"flac" (ACESTEP_FORCE_FLAC_EXPORT) but does not change the output
filename/suffix, which can lead to FLAC bytes being written to e.g. .mp3 files;
update the export path logic (where known suffixes are handled and where format
is used, e.g., the variable format and any code that builds output_path or
preserves existing suffixes) so that when format == "flac" you also set the file
extension to ".flac" (use Path.with_suffix or os.path.splitext to replace the
suffix), and apply the same fix in the other block referenced (lines around
227–239) to ensure filename suffix always matches the chosen format.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ca7d8863-2cad-49af-8acc-89a3af70a4b4
📒 Files selected for processing (1)
acestep/audio_utils.py
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/en/INSTALL.md (1)
143-143: Optional: Remove blank line inside blockquote.Line 143 contains a blank line within the Markdown blockquote (
>with no content), triggering markdownlint rule MD028. While this doesn't affect rendering, removing it would clean up the structure.♻️ Proposed cleanup
> ```bash > export LD_LIBRARY_PATH=/usr/lib/x86_64-linux-gnu:$LD_LIBRARY_PATH > ``` -> - + > **Note:** Git must be installed via your system package manager (`sudo apt install git`, `sudo yum install git`, `sudo pacman -S git`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/INSTALL.md` at line 143, Remove the empty blockquote line that triggers MD028: locate the blockquote sequence that starts with lines containing leading '>' markers around the code block and the subsequent Note (the lines with "> ```", the lone "> " blank line, and "> **Note:** ..."), and delete the solitary "> " blank line so the code fence and the following note remain consecutive within the blockquote..env.example (1)
57-58: Optional: Remove extra blank lines.Two consecutive blank lines create unnecessary spacing. Consider reducing to a single blank line for cleaner formatting.
♻️ Proposed cleanup
# ACESTEP_FORCE_FLAC_EXPORT=0 - - + # ==================== Model Storage ====================🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 57 - 58, Remove the unnecessary consecutive blank lines in .env.example (the extra empty line sequence near the middle of the file) so that only a single blank line remains between sections; simply delete the extra blank line to clean up formatting and keep consistent spacing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.env.example:
- Around line 57-58: Remove the unnecessary consecutive blank lines in
.env.example (the extra empty line sequence near the middle of the file) so that
only a single blank line remains between sections; simply delete the extra blank
line to clean up formatting and keep consistent spacing.
In `@docs/en/INSTALL.md`:
- Line 143: Remove the empty blockquote line that triggers MD028: locate the
blockquote sequence that starts with lines containing leading '>' markers around
the code block and the subsequent Note (the lines with "> ```", the lone "> "
blank line, and "> **Note:** ..."), and delete the solitary "> " blank line so
the code fence and the following note remain consecutive within the blockquote.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c5ecc80-57ff-4945-bf35-e791583b0b70
📒 Files selected for processing (2)
.env.exampledocs/en/INSTALL.md
|
Thanks for the detailed follow-up @ChuxiJ — I’ve addressed the remaining points. Final updates1.
2. INSTALL.md fixes
3. General cleanup
VerificationTested again on:
Results:
This keeps the fix aligned with project expectations:
Thanks again for the guidance — happy to make any final tweaks if needed. Thanks a lot another review again as I said, I'm kinda new to all this and opensourcing n all but I still try to do my best. It will surely be a big deal for me if I could become a contirbuter here. Tnks u ppl.. ;) |
Removed extra blank lines and clarified Git installation note.
Removed unnecessary blank lines in the .env.example file.
|
Hello out there @ChuxiJ it's been long, I was hopping if u'd look at the changes and suggest anything.. If not, then accept the commit, It would really put me to ease as well. Thank u...... |
2 participants
##The Problem
In environments using PyTorch 2.10.0+cu128, the torchcodec library fails with a binary incompatibility error: undefined symbol: torch_dtype_float4_e2m1fn_x2. Additionally, many cloud instances lack the specific shared objects (like libavutil.so.60) required by the experimental MP3 export path, causing an OSError.
specific error:
raise RuntimeError(
RuntimeError: Could not load libtorchcodec. Likely causes:
1. FFmpeg is not properly installed in your environment. We support
versions 4, 5, 6, 7, and 8, and we attempt to load libtorchcodec
for each of those versions. Errors for versions not installed on
your system are expected; only the error for your installed FFmpeg
version is relevant. On Windows, ensure you've installed the
"full-shared" version which ships DLLs.
2. The PyTorch version (2.10.0+cu128) is not compatible with
this version of TorchCodec. Refer to the version compatibility
table:
https://github.com/pytorch/torchcodec?tab=readme-ov-file#installing-torchcodec.
3. Another runtime dependency; see exceptions below.
[start of libtorchcodec loading traceback]
##Changes
Backend Stabilization: Patched acestep/audio_utils.py to prioritize FLAC as the default export format. This bypasses the unstable torchcodec MP3 logic while preserving lossless audio quality.
Library Path Resolution: Added LD_LIBRARY_PATH initialization at the module level to help the system locate FFmpeg shared libraries in standard Linux paths.
Documentation: Updated INSTALL.md with the necessary apt-get command for Linux users to install missing system-level FFmpeg dependencies.
##Verification
Environment: Lightning AI (L4 GPU), Ubuntu 22.04.
Success: Audio generation now completes and serializes successfully without crashing at the final stage.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation