| Message ID | 20160908001800.91910-1-rodger.combs@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Sep 07, 2016 at 07:17:59PM -0500, Rodger Combs wrote: > --- > ffmpeg.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) ill leave these 2 patches for nicolas to review ... thx [...]
Le duodi 22 fructidor, an CCXXIV, Rodger Combs a écrit : > --- > ffmpeg.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/ffmpeg.c b/ffmpeg.c > index d858407..1d793fe 100644 > --- a/ffmpeg.c > +++ b/ffmpeg.c > @@ -366,7 +366,7 @@ static BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) > void term_init(void) > { > #if HAVE_TERMIOS_H > - if(!run_as_daemon){ > + if (!run_as_daemon && stdin_interaction) { > struct termios tty; > if (tcgetattr (0, &tty) == 0) { > oldtty = tty; > @@ -4328,6 +4328,10 @@ int main(int argc, char **argv) > > show_banner(argc, argv, options); > > + ret = locate_option(argc, argv, options, "stdin"); > + if (ret && !strcmp(argv[ret], "-nostdin")) > + stdin_interaction = 0; > + > term_init(); I think it would be more elegant to move the term_init() call a few lines down, after ffmpeg_parse_options(), rather than parse -nostdin out of order. Apart from that, both patches looks right to me. > > /* parse options and open all input/output files */ Regards,
> On Sep 8, 2016, at 07:49, Nicolas George <george@nsup.org> wrote: > > Le duodi 22 fructidor, an CCXXIV, Rodger Combs a écrit : >> --- >> ffmpeg.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/ffmpeg.c b/ffmpeg.c >> index d858407..1d793fe 100644 >> --- a/ffmpeg.c >> +++ b/ffmpeg.c >> @@ -366,7 +366,7 @@ static BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) >> void term_init(void) >> { >> #if HAVE_TERMIOS_H >> - if(!run_as_daemon){ >> + if (!run_as_daemon && stdin_interaction) { >> struct termios tty; >> if (tcgetattr (0, &tty) == 0) { >> oldtty = tty; >> @@ -4328,6 +4328,10 @@ int main(int argc, char **argv) >> >> show_banner(argc, argv, options); >> > >> + ret = locate_option(argc, argv, options, "stdin"); >> + if (ret && !strcmp(argv[ret], "-nostdin")) >> + stdin_interaction = 0; >> + >> term_init(); > > I think it would be more elegant to move the term_init() call a few lines > down, after ffmpeg_parse_options(), rather than parse -nostdin out of order. Agreed in principle; I did it this way because I'm not entirely sure if there would be negative consequences to parsing options (and thus opening I/O) before setting up signal handlers. If you think that's not an issue, I'll make the change you suggest. > > Apart from that, both patches looks right to me. > >> >> /* parse options and open all input/output files */ > > Regards, > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
Le tridi 23 fructidor, an CCXXIV, Rodger Combs a écrit : > Agreed in principle; I did it this way because I'm not entirely sure if > there would be negative consequences to parsing options (and thus opening > I/O) before setting up signal handlers. If you think that's not an issue, > I'll make the change you suggest. I really do not see anything that could be an issue there, the calls are very near to each other and options parsing does not use the terminal as input. Regards,
diff --git a/ffmpeg.c b/ffmpeg.c index d858407..1d793fe 100644 --- a/ffmpeg.c +++ b/ffmpeg.c @@ -366,7 +366,7 @@ static BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) void term_init(void) { #if HAVE_TERMIOS_H - if(!run_as_daemon){ + if (!run_as_daemon && stdin_interaction) { struct termios tty; if (tcgetattr (0, &tty) == 0) { oldtty = tty; @@ -4328,6 +4328,10 @@ int main(int argc, char **argv) show_banner(argc, argv, options); + ret = locate_option(argc, argv, options, "stdin"); + if (ret && !strcmp(argv[ret], "-nostdin")) + stdin_interaction = 0; + term_init(); /* parse options and open all input/output files */