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

[Michael Niedermayer]

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

[...]

-- 
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150730/6b8a8131/attachment.sig>