On Thu, Jan 7, 2016 at 4:34 PM, Michael Niedermayer <mich...@niedermayer.cc> wrote: > On Thu, Jan 07, 2016 at 04:01:14PM -0800, Ganesh Ajjanagadde wrote: >> On Thu, Jan 7, 2016 at 3:57 PM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote: >> > On Thu, Jan 7, 2016 at 2:18 PM, Michael Niedermayer >> > <mich...@niedermayer.cc> wrote: >> >> On Thu, Jan 07, 2016 at 11:16:27PM +0100, Michael Niedermayer wrote: >> >>> On Thu, Jan 07, 2016 at 10:00:47AM -0800, Ganesh Ajjanagadde wrote: >> >>> > On Thu, Jan 7, 2016 at 9:27 AM, Michael Niedermayer >> >>> > <mich...@niedermayer.cc> wrote: >> >>> > > On Wed, Jan 06, 2016 at 09:00:46PM -0800, Ganesh Ajjanagadde 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(-) >> >>> > > >> >>> > > LGTM >> >>> > > >> >>> > > thanks >> >>> > >> >>> > So there is actually a problem with the diagnostic obtained, a more >> >>> > accurate diagnostic is via errno, say strerror(errno) instead of >> >>> > av_err2str(ret). >> >>> > Comparison: >> >>> > Error closing vstats file, loss of information possible: Operation not >> >>> > permitted >> >>> > vs >> >>> > Error closing vstats file, loss of information possible: No space left >> >>> > on device >> >>> > for the provided /dev/full example. >> >>> > >> >>> > So there are a number of possiblities: >> >>> > 1. Have 2 separate av_log lines, one for each of these. >> >>> > 2. A single av_log line, using strerror(errno). >> >>> > 3. Leave as is. >> >>> > >> >>> > I prefer 2. Let me know your preference, and I will push later. >> >>> >> >>> yes agree, 2. >> >> >> >> probably should use av_err2str() instead of strerror() though >> > >> > I thought strerror was C89? Your idea unfortunately causes problems >> > (suspect it is because appropriate error tag is missing): >> > Error closing vstats file, loss of information possible: Error number >> > 28 occurred. >> >> Did some digging, using strerror should be fine, see 984-989 of >> cmdutils.c for very similar usage. > > probably but why does av_err2str not work ? did you use > AVERROR(errno) ?
Oops, passed it directly without going through AVERROR. Fixed, pushed. Thanks for your patience and review. > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > He who knows, does not speak. He who speaks, does not know. -- Lao Tsu > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
