diff mbox

[FFmpeg-devel,1/2] ffmpeg: don't reconfigure terminal if we're not taking input from stdin

Message ID 20160910094656.59553-1-rodger.combs@gmail.com
State Superseded
Headers show

Commit Message

Rodger Combs Sept. 10, 2016, 9:46 a.m. UTC
---
 ffmpeg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Nicolas George Sept. 10, 2016, 10:11 a.m. UTC | #1
Le quintidi 25 fructidor, an CCXXIV, Rodger Combs a écrit :
> ---
>  ffmpeg.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/ffmpeg.c b/ffmpeg.c
> index d858407..08a7a3d 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,13 +4328,13 @@ int main(int argc, char **argv)
>  
>      show_banner(argc, argv, options);
>  

> -    term_init();
> -
>      /* parse options and open all input/output files */
>      ret = ffmpeg_parse_options(argc, argv);
>      if (ret < 0)
>          exit_program(1);
>  
> +    term_init();
> +

I am really sorry: in my last mail, I made a mistake. For my excuses, the
function names are misleading: term_init() does not only init the term, it
also sets the signal handlers; and ffmpeg_parse_options() does not only
parse the options, it also open the files.

That means that, unlike what I wrote, moving term_init() below
ffmpeg_parse_options() will change the behaviour: opening the files will
happen without signal handlers.

Still, initing the term after really parsing the options instead of
out-of-order parsing is better. In particular, regular parsing will set
stdin_interaction to 0 when stdin is used as input, this would not be taken
into account with out-of-order parsing.

I think the best solution would be to split signal initing from term_init()
into a separate signals_init(): then keep the call to signals_init() at the
current place of term_init() and the actual call to term_init() where you
moved it.

>      if (nb_output_files <= 0 && nb_input_files == 0) {
>          show_usage();
>          av_log(NULL, AV_LOG_WARNING, "Use -h to get full help or, even better, run 'man %s'\n", program_name);

Sorry for the wasted time.

Regards,
diff mbox

Patch

diff --git a/ffmpeg.c b/ffmpeg.c
index d858407..08a7a3d 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,13 +4328,13 @@  int main(int argc, char **argv)
 
     show_banner(argc, argv, options);
 
-    term_init();
-
     /* parse options and open all input/output files */
     ret = ffmpeg_parse_options(argc, argv);
     if (ret < 0)
         exit_program(1);
 
+    term_init();
+
     if (nb_output_files <= 0 && nb_input_files == 0) {
         show_usage();
         av_log(NULL, AV_LOG_WARNING, "Use -h to get full help or, even better, run 'man %s'\n", program_name);