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

[Ramiro Polla]

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...