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

Fix: overwrite leftover ffmpeg temp file if a prior conversion failed #2431

Merged
merged 3 commits into from
Oct 2, 2024

Conversation

gilbertgong
Copy link
Contributor

@gilbertgong gilbertgong commented Sep 25, 2024

In our long running whisper.cpp instance, we ran into an issue where ffmpeg failed to convert a single file, and because it failed - it left the resultant target temp file. Subsequent requests failed to process because ffmpeg declined to overwrite the existing target file:

ffmpeg version 4.4.2-0ubuntu0.22.04.1 Copyright (c) 2000-2021 the FFmpeg developers
  built with gcc 11 (Ubuntu 11.2.0-19ubuntu1)
  configuration: --prefix=/usr --extra-version=0ubuntu0.22.04.1 --toolchain=hardened --libdir=/usr/lib/x86_64-linux-gnu --incdir=/usr/include/x86_64-linux-gnu --arch=amd64 --enable-gpl --disable-stripping --enable-gnutls --enable-ladspa --enable-libaom --enable-libass --enable-libbluray --enable-libbs2b --enable-libcaca --enable-libcdio --enable-libcodec2 --enable-libdav1d --enable-libflite --enable-libfontconfig --enable-libfreetype --enable-libfribidi --enable-libgme --enable-libgsm --enable-libjack --enable-libmp3lame --enable-libmysofa --enable-libopenjpeg --enable-libopenmpt --enable-libopus --enable-libpulse --enable-librabbitmq --enable-librubberband --enable-libshine --enable-libsnappy --enable-libsoxr --enable-libspeex --enable-libsrt --enable-libssh --enable-libtheora --enable-libtwolame --enable-libvidstab --enable-libvorbis --enable-libvpx --enable-libwebp --enable-libx265 --enable-libxml2 --enable-libxvid --enable-libzimg --enable-libzmq --enable-libzvbi --enable-lv2 --enable-omx --enable-openal --enable-opencl --enable-opengl --enable-sdl2 --enable-pocketsphinx --enable-librsvg --enable-libmfx --enable-libdc1394 --enable-libdrm --enable-libiec61883 --enable-chromaprint --enable-frei0r --enable-libx264 --enable-shared
  libavutil      56. 70.100 / 56. 70.100
  libavcodec     58.134.100 / 58.134.100
  libavformat    58. 76.100 / 58. 76.100
  libavdevice    58. 13.100 / 58. 13.100
  libavfilter     7.110.100 /  7.110.100
  libswscale      5.  9.100 /  5.  9.100
  libswresample   3.  9.100 /  3.  9.100
  libpostproc    55.  9.100 / 55.  9.100
[wav @ 0x5950add2a6c0] Estimating duration from bitrate, this may be inaccurate
Guessed Channel Layout for Input Stream #0.0 : mono
Input #0, wav, from 'whisper_server_temp_file.wav':
  Duration: 00:00:03.30, bitrate: 768 kb/s
  Stream #0:0: Audio: pcm_s16le ([1][0][0][0] / 0x0001), 48000 Hz, mono, s16, 768 kb/s
File 'whisper_server_temp_file.wav_temp.wav' already exists. Overwrite? [y/N] Not overwriting - exiting

Deleting the file manually allows it to again proceed. This patch preemptively removes the file just in case it exists to fix this potential issue. adds the -y flag to ffmpeg instructing it to overwrite if necessary.

@jensdraht1999
Copy link

Yeah seems to be good idea, but you could after "-i" pipe give a "-y" pipe (which means always say yes).

So this here:
cmd_stream << "ffmpeg -i "" << temp_filename << "" -ar 16000 -ac 1 -c:a pcm_s16le "" << converted_filename_temp << "" 2>&1";

would become this:
cmd_stream << "ffmpeg -i "" << temp_filename << "" -y -ar 16000 -ac 1 -c:a pcm_s16le "" << converted_filename_temp << "" 2>&1";

@gilbertgong
Copy link
Contributor Author

Yeah seems to be good idea, but you could after "-i" pipe give a "-y" pipe (which means always say yes).

So this here: cmd_stream << "ffmpeg -i "" << temp_filename << "" -ar 16000 -ac 1 -c:a pcm_s16le "" << converted_filename_temp << "" 2>&1";

would become this: cmd_stream << "ffmpeg -i "" << temp_filename << "" -y -ar 16000 -ac 1 -c:a pcm_s16le "" << converted_filename_temp << "" 2>&1";

Sounds good, I've made this change.

@gilbertgong gilbertgong changed the title Remove possible leftover ffmpeg temp file from a previous failed conversion Fix: overwrite leftover ffmpeg temp file if a prior conversion failed Sep 27, 2024
@ggerganov ggerganov merged commit ede1718 into ggerganov:master Oct 2, 2024
lyapple2008 pushed a commit to lyapple2008/whisper.cpp.mars that referenced this pull request Nov 2, 2024
* Remove possible leftover ffmpeg temp file from a previous failed conversion

* Revert "Remove possible leftover ffmpeg temp file from a previous failed conversion"

This reverts commit 0079740.

* Flag to force ffmpeg to overwrite output file if it exists
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.

3 participants