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

Binathi Bingi binti179 at gmail.com
Fri Nov 7 16:37:26 CET 2014


Hi there,

Please find the attached patch.

Regards
Binathi

On Thu, Nov 6, 2014 at 1:55 AM, Nicolas George <george at nsup.org> wrote:

> Le quintidi 15 brumaire, an CCXXIII, Binathi Bingi a écrit :
> > I see, we need dup2() to redirect the output to logfile. Therefore, I put
> > it back in the patch.
> >
> > But, I am not sure if we should definitely use it, because I can't see
> any
> > messages on the console as all are being redirected to log file
> > For instance, when I run ffserver, I can't see the output like"FFserver
> > started" or when I try to re-run while it is already running as daemon, I
> > can't see the messages like "bind(port 8090): Address already in use"
> >
> > So, I did ps -ux to see if ffserver was running as daemon, and it was.
> > I am not sure if we should use dup2(), as it is redirecting messages from
> > console.
>
> I am not sure I understand exactly the scenario you are describing. Did you
> set the logfile option for your tests?
>
> > From 091aa564dc43bf18abd5dc596bf5350e92cad5dd 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
>
> This is near the final form for inclusion, so even minor comments are
> included. There are only two real issues: the tabs and the
> config.logfilename test; the rest is mostly cosmetic.
>
> >
> > Signed-off-by: Binathi Bingi <binti179 at gmail.com>
> > ---
> >  doc/ffserver.conf |  5 +++++
> >  doc/ffserver.texi |  7 ++++---
> >  ffserver.c        | 38 ++++++++++++++++++++++++++++++++++++--
> >  ffserver_config.c |  6 ++++--
> >  ffserver_config.h |  1 +
> >  5 files changed, 50 insertions(+), 7 deletions(-)
> >
> > diff --git a/doc/ffserver.conf b/doc/ffserver.conf
> > index b756961..8101f15 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
>
> Please leave NoDaemon the default.
>
> (I suspect you change it for your tests; in that case, maybe make a copy
> that you can change and will not commit.)
>
> > +
> >  ##################################################################
> >  # 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
>
> Final period.
>
> >  @end table
> >
> >  @section Feed section
> > diff --git a/ffserver.c b/ffserver.c
> > index ea2a2ae..eda3496 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;
> > -
>
> > +    int fd;
>
> Leave the empty line. And possibly move fd along with ffserver_id and sid.
>
> >      config.filename = av_strdup("/etc/ffserver.conf");
> >
> >      parse_loglevel(argc, argv, options);
> > @@ -3736,7 +3737,40 @@ int main(int argc, char **argv)
> >      build_feed_streams();
> >
> >      compute_bandwidth();
>
> > -
>
> Please leave the empty line.
>
> > +     if (config.ffserver_daemon) {
>
> Here and in the following lines, you have tabs for indentation. Tabs can
> not
> be committed to the official repository. The standard indentation is four
> spaces for each level.
>
> > +             pid_t ffserver_id = 0;
> > +             pid_t sid = 0;
> > +
>
> > +             ffserver_id = fork();
> > +
> > +             if (ffserver_id < 0) {
>
> This empty line, OTOH, can go :)
>
> > +                     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);
> > +
> > +             fd = open("/dev/null", O_RDWR);
> > +
> > +             if (fd != -1){
> > +                     dup2 (fd, 0);
> > +                     dup2 (fd, 2);
>
> > +                     while(!strcmp(config.logfilename,"-"))
> > +                             dup2 (fd, 1);
>
> I am pretty sure this was not tested: config.logfilename can not change in
> the loop, so if the program enters the loop, it will never stop.
>
> > +             } else {
> > +                     ret = AVERROR(errno);
> > +                     av_log(NULL, AV_LOG_ERROR, "Unable to repoen file
> descriptors: %s\n", av_err2str(ret));
> > +                     exit(1);
> > +             }
>
> Since there is an exit in the clause, it can use the usual pattern:
>
>     if (operation fails) {
>         print message;
>         exit;
>     }
>     rest of the code
>
> Also, "if (... < 0)" is more consistent with the code than comparing to
> exactly -1.
>
> > +
> > +    }
> > +
> >      /* 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;
>
> Regards,
>
> --
>   Nicolas George
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Restore-Daemon-mode-in-FFserver.patch
Type: text/x-patch
Size: 4841 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141107/843891f6/attachment.bin>


More information about the ffmpeg-devel mailing list