Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force read-mode in av.open #566

Merged
merged 1 commit into from
Nov 27, 2023
Merged

Conversation

ClaytonJY
Copy link
Contributor

The av.open functions checks input metadata to determine the mode to open with ("r" or "w"). If an input to decode_audio is found to be in write-mode, it will fail. Forcing read mode solves this.

I encountered this in a FastAPI application, where I used the UploadFile type to accept a file in my API endpoint. This uses a SpooledTemporaryFile underneath. In some circumstances, such as with small input files, this spooled file will have mode "w+b" when we first attempt to read it, causing decode_audio to try opening with av.open in write mode, and failing. Forcing to read-mode fixes this.

It would not make sense for av.open in decode_audio to ever use write-mode, so this should not negatively impact any other uses.

@gonzaloplaza
Copy link

Hi there, we're experiencing the same issue. Please do you know if this kind of fixes can be released soon? Many thanks 🙏🏻

@ClaytonJY
Copy link
Contributor Author

@gonzaloplaza while I have some approvals it looks like we're waiting on an "approving review"; this is my first PR here so I don't know how many folks can provide that or what to expect on timeline 🤷

The `av.open` functions checks input metadata to determine the mode to open with ("r" or "w"). If an input to `decode_audio` is found to be in write-mode, without this change it can't be read. Forcing read mode solves this.
@nguyendc-systran nguyendc-systran merged commit 9641d5f into SYSTRAN:master Nov 27, 2023
6 checks passed
@ClaytonJY ClaytonJY deleted the patch-1 branch November 27, 2023 15:31
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.

5 participants