Skip to content

fix: isolate and clean up resampled audio files - #557

Open
abaichuan1017 wants to merge 3 commits into
collabora:mainfrom
abaichuan1017:fix/temporary-resampling-files
Open

abaichuan1017 wants to merge 3 commits into
collabora:mainfrom
abaichuan1017:fix/temporary-resampling-files

Conversation

@abaichuan1017

Copy link
Copy Markdown

File transcription currently writes <stem>_resampled.wav into the working directory, overwriting an existing file or another conversion with the same name. It also leaves the converted file behind after playback.

Create a unique temporary WAV, close both PyAV containers on every path, remove failed conversions, and let the file-transcription client remove its owned temporary file after playback, including errors. Document the cleanup responsibility for direct resample callers.

Validation: 71 client/utility tests passed, including native PyAV conversion at 16 kHz, preserving an existing output, distinct conversion paths, conversion-failure cleanup, and cleanup after successful/failed playback. Three regression cases fail on the original implementation. git diff --check passed. No microphone, model, or server integration was needed for these file-lifecycle checks.

@boxerab boxerab left a comment

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.

thanks, this looks good. one gap: load_audio in whisper_live/transcriber/tensorrt_utils.py also calls resample and never removes the file. before this change it overwrote the same <stem>_resampled.wav each time, now every call leaves a new file in the temp dir. could you wrap the read in try/finally and os.remove(resampled_file) there too, like examples/manual_audio_chunking.py does?

optional: the completed flag could be except BaseException: unlink, raise instead.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants