[FFmpeg-devel] "OPW Qualification Task: Enable daemon mode for FFserver"
Nicolas George
george at nsup.org
Wed Nov 5 21:25:31 CET 2014
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
-------------- 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/20141105/412ecd7f/attachment.asc>
More information about the ffmpeg-devel
mailing list