On Thu, Jul 30, 2015 at 10:58:57PM -0400, Ganesh Ajjanagadde wrote:
> On Thu, Jul 30, 2015 at 9:53 PM, Michael Niedermayer
> <mich...@niedermayer.cc> wrote:
> > On Thu, Jul 30, 2015 at 05:57:54PM -0400, Ganesh Ajjanagadde wrote:
> >> On Thu, Jul 30, 2015 at 5:49 PM, Michael Niedermayer
> >> <mich...@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
> >> >> > <mich...@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
> >> >> > >> <gajjanaga...@gmail.com> wrote:
> >> >> > >> > This fixes Ticket2964
> >> >> > >> >
> >> >> > >> > Signed-off-by: Ganesh Ajjanagadde <gajjanaga...@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.
> >
> > no disagreement here but as the shell does not do that (well at least
> > not bash here)
> > it causes moderate inconvenience to the developers
> > in everyday work if the terminal needs to be reset after a fate
> > failure
>
> Ok, in that case I will reopen the ticket.
> Note that this rules out straightforward fixes.
> The only idea I have left is to create wrapper functions around ffmpeg
> invocations (e.g in fate) like:
>
> function ffmpeg_wrapper()
> {
> STTYOPTS="$(stty --save)"
> ffmpeg "$@"
> stty "${STTYOPTS}"
> }.
>
> What do people think of this?i dont think this solves the problem, not everything calls ffmpeg through such wrapper ATM ffmpeg in practice doesnt leave the terminal in a messed up state from te users point of view when it crashes. And there should be no regression here. I dont think it would be an improvment if #2964 is fixed and then instead have ffmpeg randomly mess up the terminal instead 2964: (aka the user can quit a process by pressing 'q' instead of crtl-c when its stderr is oddly redirected and stderr actually is what the whole system uses to communicate so the user cannot even see the message about pressing 'q') I think #2964 is less important than a regression with messing up the terminal. So to fix #2964 a solution would need to be found which does not cause the terminal to be messed up on crashes in real world cases. I dont know how to do that. And i dont know enough about this subject to say if its practically possible or not. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I do not agree with what you have to say, but I'll defend to the death your right to say it. -- Voltaire
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
