Skip to content

Conversation

@debroy-rh
Copy link
Contributor

@debroy-rh debroy-rh commented Sep 19, 2025

Test Plan:

Unit tests for - vllm/multimodal/audio.py
The tests validate audio resampling (librosa, scipy) and ensure AudioResampler correctly delegates to the right backend, raises errors for invalid methods, and enforces target sample rates. They also verify AudioMediaIO can load audio from bytes, base64, and file paths, and encode audio to base64, with external libraries mocked (librosa, soundfile). Assertions check correct function calls, expected outputs, and error handling.

@mergify mergify bot added the multi-modality Related to multi-modality (#4194) label Sep 19, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces unit tests for audio processing functionalities in vllm/multimodal/audio.py. The tests are well-structured, covering resampling and media I/O operations with appropriate mocking. My review identifies a critical gap in the test coverage for resample_audio_scipy. The current tests only validate scenarios with integer resampling ratios, failing to detect a bug in the implementation that causes incorrect resampling for non-integer ratios. I've suggested an additional test case to expose and address this correctness issue.

Copy link
Member

@DarkLight1337 DarkLight1337 left a comment

Choose a reason for hiding this comment

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

Thanks for adding these tests!

Make sure you run pip-compile to update the requirements/test.txt file

@debroy-rh
Copy link
Contributor Author

debroy-rh commented Sep 20, 2025

spicy is already added to requirements/test.txt. so I removed it from requirements/test.in.

Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: Debolina Roy <debroy@redhat.com>
@DarkLight1337 DarkLight1337 merged commit 5aeb925 into vllm-project:main Sep 21, 2025
26 checks passed
kingsmad pushed a commit to kingsmad/vllm that referenced this pull request Sep 22, 2025
Signed-off-by: Debolina Roy <debroy@redhat.com>
@debroy-rh debroy-rh deleted the dr_test branch September 22, 2025 20:52
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
Signed-off-by: Debolina Roy <debroy@redhat.com>
charlifu pushed a commit to ROCm/vllm that referenced this pull request Sep 25, 2025
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: charlifu <charlifu@amd.com>
yewentao256 pushed a commit that referenced this pull request Oct 3, 2025
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: yewentao256 <zhyanwentao@126.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 10, 2025
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
choprahetarth pushed a commit to Tandemn-Labs/vllm that referenced this pull request Oct 11, 2025
Signed-off-by: Debolina Roy <debroy@redhat.com>
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
Signed-off-by: Debolina Roy <debroy@redhat.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
Signed-off-by: Debolina Roy <debroy@redhat.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

multi-modality Related to multi-modality (#4194)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants