Handle invalid audio waveform values in SaveVideo without failing prompt#13193
Handle invalid audio waveform values in SaveVideo without failing prompt#13193iqdoctor wants to merge 2 commits intoComfy-Org:masterfrom
Conversation
iqdoctor
requested review from
Kosinkadink,
comfyanonymous and
guill
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 462592a359
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except Exception as exc: | ||
| logger.warning( | ||
| "Failed to encode audio track, saving video-only output: %s", exc | ||
| ) |
There was a problem hiding this comment.
The broad except Exception here will also catch output.mux(...) failures (for example, filesystem write errors such as a full disk), then log a warning and return as if save succeeded. That changes prior behavior from explicit failure to silent partial/corrupt output, which can hide real save errors in production workflows. Consider limiting the fallback to known audio-content errors (e.g., frame creation/encode argument errors) and letting I/O/mux failures propagate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 628af86: narrowed fallback to audio-data/encode exceptions only (av.error.ArgumentError, ValueError, TypeError), moved mux outside fallback so filesystem/I/O write failures propagate as hard failures. Also switched fallback log to explicit ERROR message for operator logs.
📝 WalkthroughWalkthroughAdded module-level logging and a helper 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
Thanks, valid point. You are right that the previous broad I pushed a follow-up commit to narrow fallback handling:
Commit: Operational context from production ( |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@comfy_api/latest/_input_impl/video_types.py`:
- Around line 467-486: The except clause is catching a non-existent
av.error.ArgumentError which will raise AttributeError at runtime; update the
exception handling around the audio encoding block (the try that calls
waveform.float().cpu().contiguous().numpy(), av.AudioFrame.from_ndarray,
audio_stream.encode and then mux_packets) to remove av.error.ArgumentError and
only catch the valid exceptions (ValueError, TypeError) so the logger.error
branch still runs for encoding/validation failures without referencing a
nonexistent symbol.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ab6b83c0-caee-4a13-8c53-fa879b3156f8
📒 Files selected for processing (1)
comfy_api/latest/_input_impl/video_types.py
| encoded_audio_packets = None | ||
| flush_audio_packets = None | ||
| try: | ||
| audio_np = waveform.float().cpu().contiguous().numpy() | ||
| if not np.isfinite(audio_np).all(): | ||
| audio_np = np.nan_to_num(audio_np, nan=0.0, posinf=0.0, neginf=0.0) | ||
|
|
||
| frame = av.AudioFrame.from_ndarray(audio_np, format='fltp', layout=layout) | ||
| frame.sample_rate = audio_sample_rate | ||
| frame.pts = 0 | ||
| encoded_audio_packets = audio_stream.encode(frame) | ||
| flush_audio_packets = audio_stream.encode(None) | ||
| except (av.error.ArgumentError, ValueError, TypeError) as exc: | ||
| logger.error( | ||
| "Audio encode failed due to invalid audio data; skipping audio track and saving video-only output: %s", | ||
| exc, | ||
| ) | ||
| else: | ||
| mux_packets(output, encoded_audio_packets) | ||
| mux_packets(output, flush_audio_packets) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PyAV av.error.ArgumentError exception class
💡 Result:
There is no av.error.ArgumentError exception class in PyAV. Official PyAV documentation across versions (9.0.2, 14.3.0, 15.0.0, 17.0.0) explicitly states that FFmpeg argument errors (EINVAL) are mapped to av.error.ValueError, which inherits from both ValueError and FFmpegError. This allows catching via either except av.FFmpegError or except ValueError. No search results, GitHub issues, or source code snippets mention or define av.error.ArgumentError. Common "Invalid argument" errors (Errno 22, EINVAL) raise ValueError, OSError, or specific subclasses like InvalidDataError depending on context. PyAV error handling translates FFmpeg errors to Python builtin exceptions where possible, shadowing the hierarchy under av.FFmpegError. A complete list of specific exceptions includes BSFNotFoundError, BugError, BufferTooSmallError, etc., but not ArgumentError.
Citations:
- 1: https://pyav.basswood-io.com/docs/14.3/api/error.html
- 2: https://pyav.org/docs/stable/api/error.html
- 3: https://pyav.basswood-io.com/docs/15.0/api/error.html
- 4: Make err_check raise OSError when appropriate? PyAV-Org/PyAV#563
- 5: https://pyav.basswood-io.com/
Remove non-existent av.error.ArgumentError from exception handling.
The except clause attempts to catch av.error.ArgumentError, which does not exist in PyAV. Official PyAV documentation confirms that FFmpeg argument errors (EINVAL) are mapped to av.error.ValueError, not a separate ArgumentError class. This will cause an AttributeError at runtime.
Replace:
except (av.error.ArgumentError, ValueError, TypeError) as exc:with:
except (ValueError, TypeError) as exc:The ValueError already covers FFmpeg argument validation errors via PyAV's error mapping.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@comfy_api/latest/_input_impl/video_types.py` around lines 467 - 486, The
except clause is catching a non-existent av.error.ArgumentError which will raise
AttributeError at runtime; update the exception handling around the audio
encoding block (the try that calls waveform.float().cpu().contiguous().numpy(),
av.AudioFrame.from_ndarray, audio_stream.encode and then mux_packets) to remove
av.error.ArgumentError and only catch the valid exceptions (ValueError,
TypeError) so the logger.error branch still runs for encoding/validation
failures without referencing a nonexistent symbol.
1 participant
Fixes #13192
What
Harden
VideoFromComponents.save_to()audio path to prevent full prompt failure when audio tensor contains malformed values.Changes in
comfy_api/latest/_input_impl/video_types.py:torch.nan_to_num(...).clamp(-1.0, 1.0)audio_npwithnp.isfiniteandnp.nan_to_numencodetry/exceptand continue with video-only output on failureWhy
Current behavior can throw:
av.error.ArgumentError: Invalid argument: 'avcodec_send_frame()' returned 22which aborts the whole prompt during save. This is especially visible in long video+audio workflows.
Validation
python3 -m py_compile comfy_api/latest/_input_impl/video_types.py225c52f6:VideoFromComponents(...).save_to('/tmp/comfy_audio_nan_inf_test.mp4')succeeded