[FFmpeg-devel] "OPW Qualification Task: Enable daemon mode for FFserver"

Nicolas George george at nsup.org
Sun Nov 9 11:10:33 CET 2014


Le septidi 17 brumaire, an CCXXIII, Binathi Bingi a écrit :
> From f06f2b656feb9b01c42533bcdf51fc5190ca6f91 Mon Sep 17 00:00:00 2001
> From: Binathi <binti179 at gmail.com>
> Date: Tue, 4 Nov 2014 21:42:07 +0530
> Subject: [PATCH] Restore Daemon mode in FFserver
> 
> Signed-off-by: Binathi Bingi <binti179 at gmail.com>
> ---
>  doc/ffserver.conf |  5 +++++
>  doc/ffserver.texi |  7 ++++---
>  ffserver.c        | 41 +++++++++++++++++++++++++++++++++++++++--
>  ffserver_config.c |  6 ++++--
>  ffserver_config.h |  1 +
>  5 files changed, 53 insertions(+), 7 deletions(-)
> 
> diff --git a/doc/ffserver.conf b/doc/ffserver.conf
> index b756961..0b63555 100644
> --- a/doc/ffserver.conf
> +++ b/doc/ffserver.conf
> @@ -25,6 +25,11 @@ MaxBandwidth 1000
>  # '-' is the standard output.
>  CustomLog -
>  
> +# Suppress NoDaemon and enable Daemon, if you want to launch ffserver in daemon mode.
> +# If no option is specified, default option is NoDaemon.
> +#NoDaemon
> +#Daemon
> +
>  ##################################################################
>  # Definition of the live feeds. Each live feed contains one video
>  # and/or audio sequence coming from an ffmpeg encoder or another
> diff --git a/doc/ffserver.texi b/doc/ffserver.texi
> index 77273d2..5d5fc0f 100644
> --- a/doc/ffserver.texi
> +++ b/doc/ffserver.texi
> @@ -405,9 +405,10 @@ In case the commandline option @option{-d} is specified this option is
>  ignored, and the log is written to standard output.
>  
>  @item NoDaemon
> -Set no-daemon mode. This option is currently ignored since now
> - at command{ffserver} will always work in no-daemon mode, and is
> -deprecated.
> +Set no-daemon mode. This is the default.
> +
> + at item Daemon
> +Set daemon mode. The default is NoDaemon
>  @end table
>  
>  @section Feed section
> diff --git a/ffserver.c b/ffserver.c
> index ea2a2ae..b623c0b 100644
> --- a/ffserver.c
> +++ b/ffserver.c
> @@ -3671,6 +3671,7 @@ static void handle_child_exit(int sig)
>  static void opt_debug(void)
>  {
>      config.debug = 1;
> +    config.ffserver_daemon = 0;
>      snprintf(config.logfilename, sizeof(config.logfilename), "-");
>  }
>  
> @@ -3694,7 +3695,7 @@ int main(int argc, char **argv)
>  {
>      struct sigaction sigact = { { 0 } };
>      int ret = 0;

> -
> +	

Stray change with a tab in it: this can not be applied.

>      config.filename = av_strdup("/etc/ffserver.conf");
>  
>      parse_loglevel(argc, argv, options);
> @@ -3728,7 +3729,7 @@ int main(int argc, char **argv)
>              logfile = stdout;
>          else
>              logfile = fopen(config.logfilename, "a");

> -        av_log_set_callback(http_av_log);
> +            av_log_set_callback(http_av_log);

Stray change with bizarre indentation. This line should probably stay as is.

>      }
>  
>      build_file_streams();
> @@ -3736,7 +3737,43 @@ int main(int argc, char **argv)
>      build_feed_streams();
>  
>      compute_bandwidth();
> +  
> +    if(config.ffserver_daemon) {
> +        pid_t ffserver_id = 0;
> +        pid_t sid = 0;
> +        int fd;
> +        ffserver_id = fork();
> +
> +        if (ffserver_id < 0) {
> +            ret = AVERROR(errno);
> +            av_log(NULL, AV_LOG_ERROR, "Impossible to start in daemon mode: %s\n", av_err2str(ret));
> +            exit(1);
> +        }
>  
> +        if (ffserver_id > 0)
> +            exit(0);
> +
> +        sid = setsid();
> +        if (sid < 0)
> +            exit(1);
> +

> +        if (strcmp(config.logfilename, "-") !=0) 
> +            close(0);

I am not sure what this is intended to do. It looks like a mix with other
methods of reopening the default streams. I suggest to avoid duplicating the
test and just drop this part, it is unnecessary with the dup2() method.

> +
> +        fd = open("/dev/null", O_RDWR);
> +

> +        if (fd >= 0){  

No need to test success if failure causes exit().

> +            dup2(fd, 0);  
> +            dup2(fd, 2); 
> +            if (strcmp(config.logfilename,"-") != 0)
> +                dup2 (fd, 1);  
> +        } 

I believe you forgot to close fd.

> +        if (fd < 0) {
> +            ret = AVERROR(errno);
> +            av_log(NULL, AV_LOG_ERROR, "Unable to repoen file descriptors: %s\n", av_err2str(ret));
> +            exit(1);
> +        }

The failure case should probably come immediately after the open(),
especially since it causes immediate exit.

> +    }     
>      /* signal init */
>      signal(SIGPIPE, SIG_IGN);
>  
> diff --git a/ffserver_config.c b/ffserver_config.c
> index e44cdf7..63cfd43 100644
> --- a/ffserver_config.c
> +++ b/ffserver_config.c
> @@ -358,8 +358,10 @@ static int ffserver_parse_config_global(FFServerConfig *config, const char *cmd,
>          ffserver_get_arg(arg, sizeof(arg), p);
>          if (resolve_host(&config->http_addr.sin_addr, arg) != 0)
>              ERROR("%s:%d: Invalid host/IP address: %s\n", arg);
> -    } else if (!av_strcasecmp(cmd, "NoDaemon")) {
> -        WARNING("NoDaemon option has no effect, you should remove it\n");
> +    } else if (!av_strcasecmp(cmd, "Daemon")){
> +        config->ffserver_daemon = 1;
> +    } else if (!av_strcasecmp(cmd, "NoDaemon")){
> +        config->ffserver_daemon = 0;
>      } else if (!av_strcasecmp(cmd, "RTSPPort")) {
>          ffserver_get_arg(arg, sizeof(arg), p);
>          val = atoi(arg);
> diff --git a/ffserver_config.h b/ffserver_config.h
> index 36d61d0..e3957b1 100644
> --- a/ffserver_config.h
> +++ b/ffserver_config.h
> @@ -100,6 +100,7 @@ typedef struct FFServerConfig {
>      unsigned int nb_max_http_connections;
>      unsigned int nb_max_connections;
>      uint64_t max_bandwidth;
> +    int ffserver_daemon;
>      int debug;
>      char logfilename[1024];
>      struct sockaddr_in http_addr;

Also, I notice you have trailing spaces all over the place; like tab, they
are forbidden in the official repository. I do not know what editor you use;
with vim, I can search for " \+$" with the 'hlsearch' option to see them.


In short, I suggest:

- remove the first "-" test and the close();
- move the <0 test just after the open and remove the >=0 test;
- close fd before the end;
- clean up tabs, trailing spaces and the indent change.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141109/6341f307/attachment.asc>


More information about the ffmpeg-devel mailing list