[FFmpeg-devel] "OPW Qualification Task: Enable daemon mode for FFserver"
Nicolas George
george at nsup.org
Sat Nov 1 17:35:13 CET 2014
Le primidi 11 brumaire, an CCXXIII, Binathi Bingi a écrit :
> 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.
Thanks. It looks almost there.
>
> 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
Small nitpick: for the first line of the commit message, something like that
is preferred:
ffserver: implement daemon mode
(possibly with "again" at the end, since it used to exist)
> 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>
Are those repeated lines really in the commit message? If so, some cleanup
is necessary.
Also, for copyright reasons, the full name would be better if you do not
mind.
> ---
> 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.
The patch was mangled by the mail user agent again. This will become an
issue soon, as the patch is almost in shape to be applied but mangled patch
can not be applied.
> +# 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;
This one looked better the last time: you do not want to fork when
debugging.
> 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)));
This is correct, but the message is a bit clumsy.
"Impossible to start in daemon mode: %s"
would probably be enough and more readable. Also, do not hesitate to split
long lines between arguments, aligning everything with the opening
parentheses (see similar code for examples).
> + 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);
> + }
This part is still nonsensical. If your trouble with it is that you struggle
to understand why it is necessary and what it does exactly, do not hesitate
to ask on the mailing list or in private.
> + }
> /* 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;
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/20141101/dab22e8f/attachment.asc>
More information about the ffmpeg-devel
mailing list