Skip to content

Fix resampling of zero-length audio - #17056

Open
chelsealong wants to merge 1 commit into
Comfy-Org:masterfrom
chelsealong:fix/codex-comfyui-17055
Open

chelsealong wants to merge 1 commit into
Comfy-Org:masterfrom
chelsealong:fix/codex-comfyui-17055

Conversation

@chelsealong

Copy link
Copy Markdown
Contributor

Summary

Fixes #17055. Return an empty waveform before reshaping it during unequal-rate resampling. Existing argument and resampling-method validation still runs.

Tests

  • Before fix: focused regression test failed for both 44100→48000 and 48000→44100 Hz.
  • After fix: focused regression test passed (2 passed).
  • Ruff check passed for both changed files.

The change and tests were prepared with Codex assistance.

@coderabbitai

coderabbitai Bot commented Oct 15, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: Comfy-Org/ComfyUI/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 1983cfe8-1c3e-4395-b9f2-e368349fe951
📥 Commits

Reviewing files that changed from the base of the PR and between ba99487 and 07677a9.

📒 Files selected for processing: 2
  • comfy/audio.py
  • tests-unit/comfy_test/audio_test.py
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

⏰ Context from checks skipped due to timeout: 8
  • GitHub Check: Run Pylint
  • GitHub Check: test (windows-2022)
  • GitHub Check: test (macos-latest)
  • GitHub Check: test (windows-latest)
  • GitHub Check: test (macos-latest)
  • GitHub Check: test (ubuntu-latest)
  • GitHub Check: test
  • GitHub Check: test (ubuntu-latest)
🧰 Additional context used
📚 Code guidelines: 1
AGENTS.md — configured
📓 Path-based instructions: 3
Core ML/diffusion engine.

⚙️ CodeRabbit configuration file

Files:

  • comfy/audio.py
IMPORTANT: Only comment on issues directly introduced by this PR's code changes.

⚙️ CodeRabbit configuration file

Files:

  • comfy/audio.py
  • tests-unit/comfy_test/audio_test.py
Source excerpt: Keep changes small and direct.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • comfy/audio.py
  • tests-unit/comfy_test/audio_test.py
🔀 Multi-repo context Comfy-Org/ComfyUI_frontend

Linked repositories findings

Comfy-Org/ComfyUI_frontend

  • The frontend exposes AudioConcat, AudioMerge, EmptyAudio, and JoinAudioChannels as existing node contracts; this PR changes no node names, inputs, or outputs. [::Comfy-Org/ComfyUI_frontend::]
    • src/locales/en/nodeDefs.json:322,413,5100,9530
  • Browser synthesis tests use EmptyAudio as the standard AUDIO producer, confirming zero-duration audio is a supported frontend workflow. [::Comfy-Org/ComfyUI_frontend::]
    • browser_tests/fixtures/customNode/autoRun.ts:48

Comfy-Org/comfy-kitchen

  • No audio resampling, waveform, or affected-node implementation references were found, so this fix does not appear to require a comfy-kitchen coordination or ownership change.
🔇 Additional comments: 2
comfy/audio.py: 1

64-65: LGTM!

tests-unit/comfy_test/audio_test.py: 1

1-15: LGTM!

📝 Walkthrough

When the waveform’s last dimension is empty, resample now returns the input unchanged before constructing the resampling kernel. A parametrized test checks both 44.1 kHz-to-48 kHz and reverse conversions, including preservation of shape, dtype, and device.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 07677

Empty waveforms retain their dimensions and tensor properties during unequal-rate resampling, with input validation preserved. No actionable merge risk is evident.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: fixing resampling for zero-length audio.
Description check Passed The description directly explains the zero-length waveform fix, retained validation, linked issue, regression tests, and lint results.
Linked Issues check Passed Issue [#17055] requires unequal-rate resampling to preserve an empty waveform's time dimension, leading dimensions, dtype, and device, while retaining validation. comfy/audio.py now returns `wavefor…
Out of Scope Changes check Passed The pull request changes only resample and adds a focused regression test for issue [#17055]. Both changes directly support the linked issue. No unrelated production behavior or unrelated files are …
  • Fix all pre-merge checks with AI
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Zero-length audio fails resampling when sample rates differ

1 participant