Skip to content

[Frontend] Remove librosa from audio dependency#37058

Merged
Isotr0py merged 27 commits intovllm-project:mainfrom
Isotr0py:remove-librosa
Mar 21, 2026
Merged

[Frontend] Remove librosa from audio dependency#37058
Isotr0py merged 27 commits intovllm-project:mainfrom
Isotr0py:remove-librosa

Conversation

@Isotr0py
Copy link
Copy Markdown
Member

@Isotr0py Isotr0py commented Mar 14, 2026

Purpose

Related issues:

Test Plan

pytest -s -v tests/multimodal/test_audio.py
pytest -s -v tests/multimodal/meida/test_audio.py

Test Result

All tests should pass


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

@mergify mergify Bot added the multi-modality Related to multi-modality (#4194) label Mar 14, 2026
Copy link
Copy Markdown
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 replaces the librosa dependency with pyav for audio processing, which is a good move to streamline dependencies. The changes are mostly correct and cover various parts of the codebase, including models, utilities, and tests. However, I've identified two critical issues. First, a new test for pyav resampling is incorrectly implemented and tests the wrong function. Second, the new load_audio_pyav function has a bug where it returns the original sample rate instead of the target sample rate after resampling, which could lead to significant issues in downstream processing. I've provided detailed comments and suggestions to address these critical problems.

Comment thread tests/multimodal/test_audio.py
Comment thread vllm/multimodal/media/audio.py
@mergify mergify Bot added the ci/build label Mar 14, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Mar 14, 2026

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @Isotr0py.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the performance Performance-related issues label Mar 16, 2026
@Isotr0py Isotr0py marked this pull request as ready for review March 17, 2026 08:03
Comment thread tests/multimodal/test_audio.py Outdated
@DarkLight1337 DarkLight1337 added the rocm Related to AMD ROCm label Mar 17, 2026
@github-project-automation github-project-automation Bot moved this to Todo in AMD Mar 17, 2026
@DarkLight1337 DarkLight1337 added the ready ONLY add when PR is ready to merge/full CI is needed label Mar 17, 2026
Copy link
Copy Markdown
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.

See if the CI environments can install av correctly. We should also check the CPU smoke test on main after merging

Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Mar 19, 2026

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @Isotr0py.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

1 similar comment
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Mar 19, 2026

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @Isotr0py.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Mar 19, 2026
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
@mergify mergify Bot removed the needs-rebase label Mar 19, 2026
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
@Isotr0py Isotr0py merged commit c7f98b4 into vllm-project:main Mar 21, 2026
134 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in AMD Mar 21, 2026
@Isotr0py Isotr0py deleted the remove-librosa branch March 21, 2026 03:36
zhewenl pushed a commit to zhewenl/vllm that referenced this pull request Mar 22, 2026
SouthWest7 pushed a commit to SouthWest7/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
khairulkabir1661 pushed a commit to khairulkabir1661/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
nithinvc pushed a commit to nithinvc/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>

Signed-off-by: Nithin Chalapathi <nithin.ch10@gmail.com>
JiantaoXu pushed a commit to JiantaoXu/vllm that referenced this pull request Mar 28, 2026
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
mtparet pushed a commit to blackfuel-ai/vllm that referenced this pull request Apr 9, 2026
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/build frontend multi-modality Related to multi-modality (#4194) performance Performance-related issues ready ONLY add when PR is ready to merge/full CI is needed rocm Related to AMD ROCm

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants