On Wed, Jan 6, 2016 at 9:00 PM, Ganesh Ajjanagadde
<gajjanaga...@gmail.com> wrote:
> In the spirit of commit a956840cbc. Simple method to reproduce:
> pass -vstats_file /dev/full to ffmpeg.
>
> All raw fclose usages in ffmpeg.c taken care of here.
>
> Signed-off-by: Ganesh Ajjanagadde <gajjanaga...@gmail.com>
> ---
> ffmpeg.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/ffmpeg.c b/ffmpeg.c
> index 0c83165..858e6d7 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -554,8 +554,12 @@ static void ffmpeg_cleanup(int ret)
> av_freep(&input_streams[i]);
> }
>
> - if (vstats_file)
> - fclose(vstats_file);
> + if (vstats_file) {
> + if ((ret = fclose(vstats_file)) < 0)
> + av_log(NULL, AV_LOG_ERROR,
> + "Error closing vstats file, loss of information possible:
> %s\n",
> + av_err2str(ret));
> + }
> av_freep(&vstats_filename);
>
> av_freep(&input_streams);
> @@ -4200,7 +4204,10 @@ static int transcode(void)
> ost = output_streams[i];
> if (ost) {
> if (ost->logfile) {
> - fclose(ost->logfile);
> + if ((ret = fclose(ost->logfile)) < 0)
> + av_log(NULL, AV_LOG_ERROR,
> + "Error closing logfile, loss of information
> possible: %s\n",
> + av_err2str(ret));
> ost->logfile = NULL;
> }
> av_freep(&ost->forced_kf_pts);
> --
> 2.6.4
>
Should have mentioned this earlier, but this fclose related business
was due to listening to:
https://www.irill.org/events/ghm-gnu-hackers-meeting/videos/jim-meyering-goodbye-world-the-perils-of-relying-on-output-streams-in-c,
slides:
https://www.gnu.org/ghm/2011/paris/slides/jim-meyering-goodbye-world.pdf.
For a more sustainable solution, we should create an API in e.g
libavutil for this IMHO, since fclose is called not just in ffmpeg but
also in libavfilter. There may be 2 ways of doing this:
1. Developing it organically in FFmpeg.
2. Using gnulib and associated close_stream/other API's (e.g
https://github.com/coreutils/gnulib/blob/master/lib/close-stream.c) as
suggested in the talk as a basis. There are some problems:
a) Licensing - it is GPL. There is some mention regarding relicensing
for gnulib, https://www.gnu.org/software/gnulib/manual/html_node/Copyright.html,
so it may be ok to relicense under lgpl. However, I am not a lawyer.
b) Care may need to be taken to make sure it is portable enough, and
refactoring may be necessary.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
