Skip to content

[Bugfix][Frontend] support webm with audioread fallback#18477

Closed
cpwan wants to merge 1 commit intovllm-project:mainfrom
cpwan:add-webm
Closed

[Bugfix][Frontend] support webm with audioread fallback#18477
cpwan wants to merge 1 commit intovllm-project:mainfrom
cpwan:add-webm

Conversation

@cpwan
Copy link
Copy Markdown

@cpwan cpwan commented May 21, 2025

FIX #18385

@mergify mergify bot added the frontend label May 21, 2025
@cpwan cpwan force-pushed the add-webm branch 2 times, most recently from c667ae0 to 8529fe5 Compare May 21, 2025 10:31
@github-actions
Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

@cpwan
Copy link
Copy Markdown
Author

cpwan commented May 21, 2025

Btw, i tried to make minimal change to the code, yet the code in the main branch falls short of the pre-commit check, such as the import sorting...

@mergify mergify bot added the ci/build label May 21, 2025
@DarkLight1337
Copy link
Copy Markdown
Member

@NickLucche does this look good to you?

@cpwan
Copy link
Copy Markdown
Author

cpwan commented May 21, 2025

Let me tidy up a bit the git history

Signed-off-by: cpwan <cpwan@connect.ust.hk>
Copy link
Copy Markdown
Collaborator

@NickLucche NickLucche 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 taking action on the issue.
To be frank I am not familiar with audioread, but I see librosa already uses it as a fallback so maybe this doesn't even go into the requirements lists.

librosa uses soundfile and audioread for reading audio. As of v0.7, librosa uses soundfile by default, and falls back on audioread only when dealing with codecs unsupported by soundfile. For a list of codecs supported by soundfile, see the libsndfile documentation.

https://github.com/librosa/librosa/blob/e403272fc984bc4aeb316e5f15899042224bb9fe/docs/ioformats.rst#read-specific-formats

Also I am totally uneducated on potential security concerns in opening up to a matrioska format like webm (cc @russellb ).
Can we at least test for the sake of completeness sending some video/image in webm?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this exception is way too generic

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should write to a bytesio in memory buffer not a temp file. This may, among other things, trigger permissions issues on deployments.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should probably log debug/warning this path

@NickLucche
Copy link
Copy Markdown
Collaborator

Also worth to look into why librosa isn't falling back to audioread as reported in the docs, is it an optional dep or..?

@github-actions
Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had any activity within 90 days. It will be automatically closed if no further activity occurs within 30 days. Leave a comment if you feel this pull request should remain open. Thank you!

@github-actions github-actions bot added the stale Over 90 days of inactivity label Aug 20, 2025
@mergify
Copy link
Copy Markdown

mergify bot commented Aug 20, 2025

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

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 Aug 20, 2025
@github-actions github-actions bot added unstale Recieved activity after being labelled stale and removed stale Over 90 days of inactivity labels Aug 21, 2025
@github-actions
Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had any activity within 90 days. It will be automatically closed if no further activity occurs within 30 days. Leave a comment if you feel this pull request should remain open. Thank you!

@github-actions github-actions bot added stale Over 90 days of inactivity and removed unstale Recieved activity after being labelled stale labels Nov 20, 2025
@mergify mergify bot removed the needs-rebase label Nov 20, 2025
@github-actions github-actions bot added unstale Recieved activity after being labelled stale and removed stale Over 90 days of inactivity labels Nov 22, 2025
@mergify
Copy link
Copy Markdown

mergify bot commented Nov 22, 2025

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

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 Nov 22, 2025
@github-actions
Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had any activity within 90 days. It will be automatically closed if no further activity occurs within 30 days. Leave a comment if you feel this pull request should remain open. Thank you!

@github-actions github-actions bot added the stale Over 90 days of inactivity label Feb 21, 2026
@github-actions github-actions bot removed the unstale Recieved activity after being labelled stale label Feb 21, 2026
@hmellor hmellor closed this Mar 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Audio transcription does not support webm

4 participants