fix: dispatch on type instead of length in ReferenceLoader.load_audio() - #1318
Open
yuvrajnode wants to merge 1 commit into
Open
fix: dispatch on type instead of length in ReferenceLoader.load_audio()#1318yuvrajnode wants to merge 1 commit into
yuvrajnode wants to merge 1 commit into
Conversation
load_audio() accepts `bytes | str` and used `len(reference_audio) > 255` as a proxy for "this is raw audio rather than a path". That heuristic is wrong in three of the four possible cases: - bytes shorter than 256 (a truncated or empty upload) fall through to `Path(bytes)`, raising `TypeError: argument should be a str object or an os.PathLike object returning str, not <class 'bytes'>` - a str path that does not exist reaches `io.BytesIO(str)`, raising `TypeError: a bytes-like object is required, not 'str'` - a valid str path longer than 255 characters is misread as audio data and also reaches `io.BytesIO(str)`, raising the same TypeError Only the "long bytes" case, which is the common one for real audio, worked as intended. Both production callers reach this via VQManager.encode_reference() with bytes (audio_to_bytes() for id-based references, ServeReferenceAudio.audio for API requests), so a small or malformed upload surfaces an opaque TypeError instead of a decode error. Dispatch on isinstance(..., bytes) and let torchaudio.load() report missing or undecodable files, which it already does with a clear message.
Author
|
Hi @tongtong-lu, @leng-yue, @Stardust-minus, @Whale-Dolphin Could you please take a look when you have a moment? Let me know if you need any additional context or if any changes are required. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ReferenceLoader.load_audio()acceptsbytes | strand useslen(reference_audio) > 255as a proxy for "this is raw audio rather than a path". That heuristic is wrong in three of the four possible cases.Current behavior
TypeError: argument should be a str object or an os.PathLike object returning str, not <class 'bytes'>TypeError: a bytes-like object is required, not 'str'TypeErrorOnly the common "long bytes" case behaves as intended. Short byte strings fall through to
Path(bytes), and anystrthat fails the.exists()check reachesio.BytesIO(str)— neither accepts the type it is handed.Impact
Both production callers reach this through
VQManager.encode_reference()with bytes —audio_to_bytes()for id-based references, andServeReferenceAudio.audiofor API requests. So a small or malformed upload surfaces an opaqueTypeErroraboutos.PathLikerather than a meaningful decode error.Fix
Dispatch on the actual type and let
torchaudio.load()report missing or undecodable files — it already does so with a clear message. This fixes all three broken cases and leaves the working one unchanged.The repo has no test suite, so I kept this to the behavioral fix rather than introducing pytest as a new dependency.