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

Binathi Bingi binti179 at gmail.com
Sat Nov 1 17:20:59 CET 2014


Hello

I tried to incorporate the changes suggested in above mail.
Now we have NoDaemon as by default as per the current standard.
NoDaemon and Daemon are now treated as two separate options.
Code is indented.
Reason for fork fail included.
Documentation has been changed.

>From e4b0cc451b7ffcf42f0a31b4ccd4d05ac69c1880 Mon Sep 17 00:00:00 2001
From: Binathi <binti179 at gmail.com>
Date: Fri, 31 Oct 2014 23:27:20 +0530
Subject: [PATCH] Enable Daemon mode for FFServer

Signed-off-by: Binathi <binti179 at gmail.com>

Improvements: Enable Daemon mode in FFServer

Signed-off-by: Binathi <binti179 at gmail.com>

Improvements: Enable Daemon mode in FFServer

Signed-off-by: Binathi <binti179 at gmail.com>
---
 doc/ffserver.conf |  5 +++++
 doc/ffserver.texi |  7 ++++---
 ffserver.c        | 33 +++++++++++++++++++++++++++++++--
 ffserver_config.c |  6 ++++--
 ffserver_config.h |  1 +
 5 files changed, 45 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..d7a6cbe 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..611d1c4 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 = 1;
     snprintf(config.logfilename, sizeof(config.logfilename), "-");
 }

@@ -3736,10 +3737,38 @@ int main(int argc, char **argv)
     build_feed_streams();

     compute_bandwidth();
-
+
+    if (config.ffserver_daemon) {
+        int ffserver_id = 0;
+        pid_t sid = 0;
+
+        ffserver_id = fork();
+
+        if (ffserver_id < 0) {
+            av_log(NULL, AV_LOG_ERROR, "Couldn't launch ffserver in daemon
mode. Fork Failure Info(): %s\n",av_err2str(AVERROR(errno)));
+            exit(1);
+        }
+
+        if (ffserver_id > 0) {
+            exit(0);
+        }
+
+        sid = setsid();
+        if (sid < 0) {
+            exit(1);
+        }
+
+        open ("/dev/null", O_RDWR);
+        dup(0);
+        dup(0);
+
+        if (strcmp(config.logfilename, "-") != 0) {
+            close(1);
+        }
+    }
     /* signal init */
     signal(SIGPIPE, SIG_IGN);
-
+
     if (http_server() < 0) {
         http_log("Could not start server\n");
         exit(1);
diff --git a/ffserver_config.c b/ffserver_config.c
index e44cdf7..a066e58 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;
-- 
1.9.1


Regards,

Binathi

On Sat, Nov 1, 2014 at 4:28 PM, Nicolas George <george at nsup.org> wrote:

> Le decadi 10 brumaire, an CCXXIII, Binathi Bingi a écrit :
> > Hello
> >
> > I tried to include the changes specified by Nicholas.
> > We can switch between both Daemon and NoDaemon mode, using the option in
> > ffserver.conf file.
> >
> > >From 018f8c1e1acf062a9e6a3ec94f671d574ec4b712 Mon Sep 17 00:00:00 2001
> > From: Binathi <binti179 at gmail.com>
> > Date: Fri, 31 Oct 2014 23:27:20 +0530
> > Subject: [PATCH] Enable Daemon mode for FFServer
> >
> > Signed-off-by: Binathi <binti179 at gmail.com>
> > ---
> >  doc/ffserver.conf |  4 ++++
> >  doc/ffserver.texi |  7 +++++--
> >  ffserver.c        | 31 +++++++++++++++++++++++++++++--
> >  ffserver_config.c |  7 +++++--
> >  ffserver_config.h |  1 +
> >  5 files changed, 44 insertions(+), 6 deletions(-)
> >
> > diff --git a/doc/ffserver.conf b/doc/ffserver.conf
> > index b756961..eac088b 100644
> > --- a/doc/ffserver.conf
> > +++ b/doc/ffserver.conf
> > @@ -25,6 +25,10 @@ MaxBandwidth 1000
> >  # '-' is the standard output.
> >  CustomLog -
> >
>
> > +# Suppress Daemon if you don't want to launch ffserver in daemon mode.
> > +#NoDaemon
> > +Daemon
>
> Needs to be consistent with the default, see below.
>
> > +
> >  ##################################################################
> >  # 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..15dc4b3 100644
> > --- a/doc/ffserver.texi
> > +++ b/doc/ffserver.texi
> > @@ -406,8 +406,11 @@ 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.
> > + at command{ffserver} will always work in daemon mode.
> > +
> > +@ Daemon
> > +Set daemon mode.
> > + at command{ffserver} will always work in daemon mode. To enable no-daemon
> > mode, suppress this and enable NoDaemon.
>
> The wording is misleading: ffserver will NOT "always" work in daemon mode:
> the options are precisely there to choose if it does. The usual wording for
> that kind of option look usually like that:
>
> Option_foo: make the application behave foo; this is the default.
>
> Option_bar: make the application behave bar; the default is foo.
>
> >  @end table
> >
> >  @section Feed section
> > diff --git a/ffserver.c b/ffserver.c
> > index ea2a2ae..d6eb0b4 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), "-");
> >  }
> >
> > @@ -3736,10 +3737,36 @@ int main(int argc, char **argv)
> >      build_feed_streams();
> >
> >      compute_bandwidth();
> > -
> > +
>
> > +    if (config.ffserver_daemon) {
>
> Do not forget to indent.
>
> > +    int ffserver_id = 0;
> > +    pid_t sid = 0;
> > +
> > +    ffserver_id = fork();
> > +
> > +    if (ffserver_id < 0) {
>
> > +        av_log(NULL, AV_LOG_WARNING, "Fork failed!Couldn't launch
> ffserver
> > in daemon mode.\n");
> > +        exit(1);
>
> Since there is exit(1), then this is an ERROR, not a WARNING.
>
> And please translate the reason for the fork fail:
> av_err2str(AVERROR(errno)).
>
> > +    }
> > +
> > +    if (ffserver_id > 0) {
> > +        exit(0);
> > +    }
> > +
> > +    sid = setsid();
> > +    if (sid < 0) {
> > +        exit(1);
> > +    }
> > +
>
> > +    open ("/dev/null", O_RDWR);
> > +
> > +    if (strcmp(config.logfilename, "-") != 0) {
> > +            close(1);
> > +        }
>
> This looked wrong before and still looks wrong now. Opening a file (or
> device) and ignoring the returned file descriptor is almost never correct.
> Closing stdout and not re-opening it afterwards can hardly be right either.
>
> > +    }
> >      /* signal init */
> >      signal(SIGPIPE, SIG_IGN);
> > -
> > +
> >      if (http_server() < 0) {
> >          http_log("Could not start server\n");
> >          exit(1);
> > diff --git a/ffserver_config.c b/ffserver_config.c
> > index e44cdf7..f46d8f4 100644
> > --- a/ffserver_config.c
> > +++ b/ffserver_config.c
> > @@ -358,8 +358,11 @@ 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") || !av_strcasecmp(cmd,
> > "NoDaemon")) {
> > +        if (!av_strcasecmp(cmd, "Daemon"))
> > +            config->ffserver_daemon = 1;
> > +        if (!av_strcasecmp(cmd, "NoDaemon"))
> > +            config->ffserver_daemon = 0;
>
> As Michael pointed out, no need to group them in a nested test, you can
> just
> treat them as two separate options:
>
>             ...
>         else if Daemon
>             ffserver_daemon = 1;
>         else if NoDaemon
>             ffserver_daemon = 0;
>         else ...
>
> >      } 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;
>
> You define that field, but you never init it: that means the initial value
> is 0. In other words, the default when no option is specified is NoDaemon.
> This is very fine, since this is currently the default and it is better not
> to change it. But the documentation (both the example snippet and the
> reference manual) must state it accordingly.
>
> >      int debug;
> >      char logfilename[1024];
> >      struct sockaddr_in http_addr;
>
> Regards,
>
> --
>   Nicolas George
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>


More information about the ffmpeg-devel mailing list