[FFmpeg-devel] [PATCH] Check for the isatty function

Martin Storsjö martin
Thu Jul 23 22:45:43 CEST 2009


On Thu, 23 Jul 2009, Ramiro Polla wrote:

> 2009/7/21 M?ns Rullg?rd <mans at mansr.com>:
> > Martin Storsj? <martin at martin.st> writes:
> >> On Tue, 14 Jul 2009, Ramiro Polla wrote:
> >>> 2009/7/14 M?ns Rullg?rd <mans at mansr.com>:
> >>> > Martin Storsj? <martin at martin.st> writes:
> >>> >
> >>> >> diff --git a/ffmpeg.c b/ffmpeg.c
> >>> >> index ca083de..15b1dcd 100644
> >>> >> --- a/ffmpeg.c
> >>> >> +++ b/ffmpeg.c
> >>> >> @@ -3978,8 +3978,10 @@ int main(int argc, char **argv)
> >>> >> ? ? ?avdevice_register_all();
> >>> >> ? ? ?av_register_all();
> >>> >>
> >>> >> +#if HAVE_ISATTY
> >>> >> ? ? ?if(isatty(STDIN_FILENO))
> >>> >> ? ? ? ? ?url_set_interrupt_cb(decode_interrupt_cb);
> >>> >> +#endif
> >>> >>
> >>> >> ? ? ?for(i=0; i<CODEC_TYPE_NB; i++){
> >>> >> ? ? ? ? ?avcodec_opts[i]= avcodec_alloc_context2(i);
> >>> >
> >>> > Fine by me.
> >>> >
> >>> > That said, why do we do this only if stdin is a terminal?
> >>>
> >>> I don't know, Michael added that. If we set it unconditionally, won't
> >>> this interfere with pipe inputs?
> >>
> >> Yes, it would probably interfere with pipes.
> >>
> >> Another solution would be:
> >>
> >> #ifdef HAVE_ISATTY
> >> ? ? if (isatty(STDIN_FILENO))
> >> #endif
> >> ? ? ? ? url_set_interrupt_cb(decode_interrupt_cb);
> >>
> >> That is, if isatty isn't available, one can perhaps assume that stdin
> >> isn't a pipe.
> >>
> >> OTOH, the initial version is safer in this aspect. If I understood issue66
> >> correctly, this is only needed to be able to abort by pressing 'q' while
> >> opening the file, so skipping if it if unsure (as in the initial patch)
> >> should be quite safe.
> >
> > I didn't mean to imply that the patch was wrong. ?I just wasn't sure
> > why the original code was that way in the first place.
> 
> I would apply it, but I don't know the consequences it might have on
> various systems. If there are no more comments I'll apply it over the
> weekend...

Assuming that the configure check works as it should, this doesn't have 
any consequences at all for the systems that ffmpeg currently works on.

// Martin



More information about the ffmpeg-devel mailing list