[FFmpeg-devel] [PATCH] ffmpeg: modify tty state when stderr is redirected

[Ganesh Ajjanagadde]

On Thu, Jul 30, 2015 at 5:49 PM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Thu, Jul 30, 2015 at 02:43:01PM +0200, Michael Niedermayer wrote:
>> On Wed, Jul 29, 2015 at 05:28:16PM -0400, Ganesh Ajjanagadde wrote:
>> > On Wed, Jul 29, 2015 at 3:27 PM, Michael Niedermayer
>> > <michael at niedermayer.cc> wrote:
>> > > On Wed, Jul 29, 2015 at 02:43:52PM -0400, Ganesh Ajjanagadde wrote:
>> > >> On Mon, Jul 27, 2015 at 9:56 AM, Ganesh Ajjanagadde
>> > >> <gajjanagadde at gmail.com> wrote:
>> > >> > This fixes Ticket2964
>> > >> >
>> > >> > Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> > >> > ---
>> > >> >  ffmpeg.c | 2 +-
>> > >> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > >> >
>> > >> > diff --git a/ffmpeg.c b/ffmpeg.c
>> > >> > index 751c7d3..98f812e 100644
>> > >> > --- a/ffmpeg.c
>> > >> > +++ b/ffmpeg.c
>> > >> > @@ -372,7 +372,7 @@ void term_init(void)
>> > >> >          struct termios tty;
>> > >> >          int istty = 1;
>> > >> >  #if HAVE_ISATTY
>> > >> > -        istty = isatty(0) && isatty(2);
>> > >> > +        istty = isatty(0);
>> > >> >  #endif
>> > >> >          if (istty && tcgetattr (0, &tty) == 0) {
>> > >> >              oldtty = tty;
>> > >>
>> > >> ping
>> > >
>> > > i dont mind applying this but i dont remember why it was there
>> > > so this might break somethig and someone might then have to revert
>> >
>> > See the long discussion I had (with my initial patch series) for full details.
>> > A short summary is as follows:
>> > in order to accept "q" and other stuff, ffmpeg has to change the terminal mode.
>> > Once terminal mode is changed, on event of "hard" signal like SIGSEGV,
>> > it is not the responsibility of ffmpeg to clean up and restore the
>> > terminal state
>> > that now appears as messed up.
>> > I had a patch to do this, but this requires registering signal handler
>> > for such signals,
>> > and others had valid objections since the core dump is no longer clean.
>> > Thus, terminal restoration should be handled by the shell.
>> > Fortunately, zsh has such functionality (thanks Nicolas for pointing
>> > this out!) via "ttyctl -f"
>> > to "freeze" terminal, i.e prevent any process from damaging the
>> > terminal state on exit.
>> > In bash it is harder to do this; AFAIK requires manual intervention.
>> >
>> > Unless fate tests redirect 2(stderr) and do not redirect 0(stdin),
>> > functionality is identical.
>> > Even otherwise, by above argument, I think this is the right thing to do.
>>
>> patch applied
>>
>> note, if something breaks, ill revert this one, but hopefully it
>> will work fine
>
> any failure in fate trashes the terminal, thus reverted
>
> To reproduce, add a abort() into wav_read_header()
> run make fate-acodec-adpcm-ima_wav

Yes, but the trashing cleanup is not ffmpeg's job; the shell should do it.

>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Many that live deserve death. And some that die deserve life. Can you give
> it to them? Then do not be too eager to deal out death in judgement. For
> even the very wise cannot see all ends. -- Gandalf
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>