Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
Le duodi 22 brumaire, an CCXXIII, Binathi Bingi a écrit : From a7c6a1f74902b6df8e4cc62480e8fd6f023905c4 Mon Sep 17 00:00:00 2001 From: Binathi Bingi binti...@gmail.com Date: Tue, 4 Nov 2014 21:42:07 +0530 Subject: [PATCH] Restore Daemon mode in FFserver Signed-off-by: Binathi Bingi binti...@gmail.com Author:Binathi Bingi binti...@gmail.com --- doc/ffserver.conf | 4 doc/ffserver.texi | 11 +++ ffserver.c| 34 ++ ffserver_config.c | 4 ++-- ffserver_config.h | 1 + 5 files changed, 48 insertions(+), 6 deletions(-) I was about to say that the patch can be applied, but unfortunately another patch to ffserver was applied very recently, and it no longer applies cleanly: it needs to be rebased once more. The conflict does not seem severe though. Since there is one more iteration, I have added very minor comments below. Regards, -- Nicolas George diff --git a/doc/ffserver.conf b/doc/ffserver.conf index b756961..a10bd77 100644 --- a/doc/ffserver.conf +++ b/doc/ffserver.conf @@ -25,6 +25,10 @@ MaxBandwidth 1000 # '-' is the standard output. CustomLog - +# Enable Daemon, to launch FFserver in Daemon mode. +# NoDaemon is the default. +#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..9938500 100644 --- a/doc/ffserver.texi +++ b/doc/ffserver.texi @@ -404,10 +404,13 @@ If not specified @command{ffserver} will produce no log. 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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +@item Daemon +Set Daemon mode. The default is NoDaemon. +In daemon mode @command{ffserver} will fork in background upon +starting, without the need to specify the shell operator in the +command line. In daemon mode also @command{ffserver} will continue to +log on stdout, unlike in NoDaemon mode. I believe that is not completely exact. If I read the code correctly, the logging works exactly the same in Daemon and NoDaemon mode. Probably best to just remove the last sentence. + @end table @section Feed section diff --git a/ffserver.c b/ffserver.c index ea2a2ae..32b48ee 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), -); } @@ -3737,6 +3738,39 @@ int main(int argc, char **argv) compute_bandwidth(); +if (config.ffserver_daemon) { +pid_t ffserver_id = 0; +pid_t sid = 0; +int fd; +ffserver_id = fork(); + +if (ffserver_id 0) { I would be slightly happier with an empty line after the variable declarations and no empty line between system call and error check. But that is very minor. +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,0); + +if (fd 0) { Same here. +ret = AVERROR(errno); +av_log(NULL, AV_LOG_ERROR, Unable to reopen file descriptors: %s\n, av_err2str(ret)); +exit(1); +} +dup2(fd, 0); +dup2(fd, 2); +if (strcmp(config.logfilename, -) != 0) +dup2(fd, 1); +close(fd); +} + /* signal init */ signal(SIGPIPE, SIG_IGN); diff --git a/ffserver_config.c b/ffserver_config.c index e44cdf7..f0368c1 100644 --- a/ffserver_config.c +++ b/ffserver_config.c @@ -358,8 +358,8 @@ 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, 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
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
On date Wednesday 2014-11-12 00:46:09 +0530, Binathi Bingi encoded: [...] From 692fb9ee19d5f9acf080eb98d22ca15fa1176393 Mon Sep 17 00:00:00 2001 From: Binathi Bingi binti...@gmail.com Date: Tue, 4 Nov 2014 21:42:07 +0530 Subject: [PATCH] Restore Daemon mode in FFserver Signed-off-by: Binathi Bingi binti...@gmail.com Author:Binathi Bingi binti...@gmail.com --- doc/ffserver.conf | 4 doc/ffserver.texi | 11 +++ ffserver.c| 34 ++ ffserver_config.c | 4 ++-- ffserver_config.h | 1 + 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/doc/ffserver.conf b/doc/ffserver.conf index b756961..a017b3c 100644 --- a/doc/ffserver.conf +++ b/doc/ffserver.conf @@ -25,6 +25,10 @@ MaxBandwidth 1000 # '-' is the standard output. CustomLog - +# Enable Daemon, to launch FFserver in Daemon mode. +# For NoDaemon mode, suppress Daemon. I'd replace this line with: # NoDaemon is the default. since suppress is very ambiguous. +#Daemon [...] No more comments from me, thanks. -- FFmpeg = Fostering and Fierce Majestic Powerful Extroverse Gladiator ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
Hi there, Updated changes in the patch. Please let me know if any further enhancements are required. Thanks, Binathi On Wed, Nov 12, 2014 at 8:50 PM, Stefano Sabatini stefa...@gmail.com wrote: On date Wednesday 2014-11-12 00:46:09 +0530, Binathi Bingi encoded: [...] From 692fb9ee19d5f9acf080eb98d22ca15fa1176393 Mon Sep 17 00:00:00 2001 From: Binathi Bingi binti...@gmail.com Date: Tue, 4 Nov 2014 21:42:07 +0530 Subject: [PATCH] Restore Daemon mode in FFserver Signed-off-by: Binathi Bingi binti...@gmail.com Author:Binathi Bingi binti...@gmail.com --- doc/ffserver.conf | 4 doc/ffserver.texi | 11 +++ ffserver.c| 34 ++ ffserver_config.c | 4 ++-- ffserver_config.h | 1 + 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/doc/ffserver.conf b/doc/ffserver.conf index b756961..a017b3c 100644 --- a/doc/ffserver.conf +++ b/doc/ffserver.conf @@ -25,6 +25,10 @@ MaxBandwidth 1000 # '-' is the standard output. CustomLog - +# Enable Daemon, to launch FFserver in Daemon mode. +# For NoDaemon mode, suppress Daemon. I'd replace this line with: # NoDaemon is the default. since suppress is very ambiguous. +#Daemon [...] No more comments from me, thanks. -- FFmpeg = Fostering and Fierce Majestic Powerful Extroverse Gladiator ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel From a7c6a1f74902b6df8e4cc62480e8fd6f023905c4 Mon Sep 17 00:00:00 2001 From: Binathi Bingi binti...@gmail.com Date: Tue, 4 Nov 2014 21:42:07 +0530 Subject: [PATCH] Restore Daemon mode in FFserver Signed-off-by: Binathi Bingi binti...@gmail.com Author:Binathi Bingi binti...@gmail.com --- doc/ffserver.conf | 4 doc/ffserver.texi | 11 +++ ffserver.c| 34 ++ ffserver_config.c | 4 ++-- ffserver_config.h | 1 + 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/doc/ffserver.conf b/doc/ffserver.conf index b756961..a10bd77 100644 --- a/doc/ffserver.conf +++ b/doc/ffserver.conf @@ -25,6 +25,10 @@ MaxBandwidth 1000 # '-' is the standard output. CustomLog - +# Enable Daemon, to launch FFserver in Daemon mode. +# NoDaemon is the default. +#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..9938500 100644 --- a/doc/ffserver.texi +++ b/doc/ffserver.texi @@ -404,10 +404,13 @@ If not specified @command{ffserver} will produce no log. 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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +@item Daemon +Set Daemon mode. The default is NoDaemon. +In daemon mode @command{ffserver} will fork in background upon +starting, without the need to specify the shell operator in the +command line. In daemon mode also @command{ffserver} will continue to +log on stdout, unlike in NoDaemon mode. + @end table @section Feed section diff --git a/ffserver.c b/ffserver.c index ea2a2ae..32b48ee 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), -); } @@ -3737,6 +3738,39 @@ int main(int argc, char **argv) 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); + +fd = open(/dev/null, O_RDWR,0); + +if (fd 0) { +ret = AVERROR(errno); +av_log(NULL, AV_LOG_ERROR, Unable to reopen file descriptors: %s\n, av_err2str(ret)); +exit(1); +} +dup2(fd, 0); +dup2(fd, 2); +if (strcmp(config.logfilename, -) != 0) +dup2(fd, 1); +close(fd); +} + /* signal init */ signal(SIGPIPE, SIG_IGN); diff --git a/ffserver_config.c b/ffserver_config.c index e44cdf7..f0368c1 100644 --- a/ffserver_config.c +++ b/ffserver_config.c @@ -358,8 +358,8 @@ static int ffserver_parse_config_global(FFServerConfig *config, const char *cmd, ffserver_get_arg(arg, sizeof(arg), p); if
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
On date Tuesday 2014-11-11 00:19:26 +0530, Binathi Bingi encoded: Hi there, Please find the attached patch with modifications. [...] From 091b4a02c9325bea32b7f745d028ea72c8e1537e Mon Sep 17 00:00:00 2001 From: Binathi Bingi binti...@gmail.com Date: Tue, 4 Nov 2014 21:42:07 +0530 Subject: [PATCH] Restore Daemon mode in FFserver Signed-off-by: Binathi Bingi binti...@gmail.com Author:Binathi Bingi binti...@gmail.com --- doc/ffserver.conf | 4 doc/ffserver.texi | 11 +++ ffserver.c| 34 ++ ffserver_config.c | 4 ++-- ffserver_config.h | 1 + 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/doc/ffserver.conf b/doc/ffserver.conf index b756961..a017b3c 100644 --- a/doc/ffserver.conf +++ b/doc/ffserver.conf @@ -25,6 +25,10 @@ MaxBandwidth 1000 # '-' is the standard output. CustomLog - +# Enable Daemon, to launch FFserver in Daemon mode. +# For NoDaemon mode, suppress Daemon. +#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..0c10c2f 100644 --- a/doc/ffserver.texi +++ b/doc/ffserver.texi @@ -404,10 +404,13 @@ If not specified @command{ffserver} will produce no log. 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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +@item Daemon +Set Daemon mode. The default is NoDaemon. +Enabling Daemon mode would allow FFserver, at 2.4 level, fork in background +without shell operator. It has logging capability to file, unlike in +NoDaemon mode, forks in background upon starting like any daemon and in +debug mode, it is prevented from forking, forcing foreground mode. In daemon mode @command{ffserver} will fork in background upon starting, without the need to specify the shell operator in the command line. In daemon mode also @command{ffserver} will continue to log on stdout, unlike in NoDaemon mode. I'm not sure about what you mean by 2.4 level, and about the interaction wih the debug mode. I also missed the discussion about why Daemon mode was removed and then we are re-enabling it now, so I probably miss some fundamental information. [...] -- FFmpeg = Forgiving Fierce MultiPurpose Exxagerate Game ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
Hi there, Updated patch. Please let me know if there are any other changes needed. I meant FFmpeg version 2.4, by 2.4 level. In case Daemon mode is made default, and user launches FFserver in debug mode, in that case, FFserver would be prevented from forking, forcing foreground mode. I am not sure about why daemon mode was removed. More information on re-enabling daemon mode can be obtained from below link: https://trac.ffmpeg.org/ticket/3731 Thanks, Binathi On Tue, Nov 11, 2014 at 6:02 PM, Stefano Sabatini stefa...@gmail.com wrote: On date Tuesday 2014-11-11 00:19:26 +0530, Binathi Bingi encoded: Hi there, Please find the attached patch with modifications. [...] From 091b4a02c9325bea32b7f745d028ea72c8e1537e Mon Sep 17 00:00:00 2001 From: Binathi Bingi binti...@gmail.com Date: Tue, 4 Nov 2014 21:42:07 +0530 Subject: [PATCH] Restore Daemon mode in FFserver Signed-off-by: Binathi Bingi binti...@gmail.com Author:Binathi Bingi binti...@gmail.com --- doc/ffserver.conf | 4 doc/ffserver.texi | 11 +++ ffserver.c| 34 ++ ffserver_config.c | 4 ++-- ffserver_config.h | 1 + 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/doc/ffserver.conf b/doc/ffserver.conf index b756961..a017b3c 100644 --- a/doc/ffserver.conf +++ b/doc/ffserver.conf @@ -25,6 +25,10 @@ MaxBandwidth 1000 # '-' is the standard output. CustomLog - +# Enable Daemon, to launch FFserver in Daemon mode. +# For NoDaemon mode, suppress Daemon. +#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..0c10c2f 100644 --- a/doc/ffserver.texi +++ b/doc/ffserver.texi @@ -404,10 +404,13 @@ If not specified @command{ffserver} will produce no log. 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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +@item Daemon +Set Daemon mode. The default is NoDaemon. +Enabling Daemon mode would allow FFserver, at 2.4 level, fork in background +without shell operator. It has logging capability to file, unlike in +NoDaemon mode, forks in background upon starting like any daemon and in +debug mode, it is prevented from forking, forcing foreground mode. In daemon mode @command{ffserver} will fork in background upon starting, without the need to specify the shell operator in the command line. In daemon mode also @command{ffserver} will continue to log on stdout, unlike in NoDaemon mode. I'm not sure about what you mean by 2.4 level, and about the interaction wih the debug mode. I also missed the discussion about why Daemon mode was removed and then we are re-enabling it now, so I probably miss some fundamental information. [...] -- FFmpeg = Forgiving Fierce MultiPurpose Exxagerate Game ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
Hello On 11/11/2014 04:16 PM, Binathi Bingi wrote: [..] Updated patch. Please let me know if there are any other changes needed. [..] Hello Binathi. Thanks for your continuous attention to this matter. Appreciated. Your patch does no longer apply cleanly, you need to rebase it on top of a recent checkout of master. For future reference, you need to do this every time you send a patch over. Bests, -- Reynaldo H. Verdejo Pinochet Open Source Group Samsung Research America / Silicon Valley ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
Le decadi 20 brumaire, an CCXXIII, Binathi Bingi a écrit : I use nano editor. If you expect you will work on source codes (not necessarily just FFmpeg) a lot, I believe taking the time of learning a better editor would be a very good investment. I tried to remove trailing whitespace in git patch using git format-patch -b -w -1 Please find the attached patch. Unfortunately, that will not work: the -w option (and -b is just a subset) will make diff consider that foo and foo are the same and not output a pair of lines for them. But if you change foo into bar , or if you insert bar , then diff must output a '+' line, and then it will output +foo , not +foo. Furthermore, I am not sure it would have been a good idea anyway: the trailing spaces could have leaked again somewhere, for example if you forget -w sometimes. Better remove them as upstream as possible. If your editor does not allow it directly, you can easily use sed to fix the files: sed -i 's/ *$//' ffserver.c From c9d037758693a1522258a64849f7629d7cbd7408 Mon Sep 17 00:00:00 2001 From: Binathi binti...@gmail.com Date: Tue, 4 Nov 2014 21:42:07 +0530 Subject: [PATCH] Restore Daemon mode in FFserver Signed-off-by: Binathi Bingi binti...@gmail.com --- doc/ffserver.conf | 5 + doc/ffserver.texi | 7 --- ffserver.c| 33 + ffserver_config.c | 4 +++- ffserver_config.h | 1 + 5 files changed, 46 insertions(+), 4 deletions(-) Apart from the trailing spaces problem, the code looks good to me now. In normal circumstances, the trailing spaces could probably be removed by the committer, but for a qualification task it is probably best to have the patch committed as is. Also, I other people have given comments on the patches, they should be allowed some time to give advice if any. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
On date Monday 2014-11-10 13:26:14 +0530, Binathi Bingi encoded: [...] From c9d037758693a1522258a64849f7629d7cbd7408 Mon Sep 17 00:00:00 2001 From: Binathi binti...@gmail.com Provide complete name, the patch is used to track copyright. Date: Tue, 4 Nov 2014 21:42:07 +0530 Subject: [PATCH] Restore Daemon mode in FFserver Signed-off-by: Binathi Bingi binti...@gmail.com --- doc/ffserver.conf | 5 + doc/ffserver.texi | 7 --- ffserver.c| 33 + ffserver_config.c | 4 +++- ffserver_config.h | 1 + 5 files changed, 46 insertions(+), 4 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 Why NoDaemon followed by Daemon? This is confusing. + ## # 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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +Set no-daemon mode. This is the default. It would be nice to provide a more lenghty explanation about the daemon mode. Why is it useful? How does it differ with the NoDaemon mode? Why and how is it useful? + +@item Daemon +Set daemon mode. The default is NoDaemon missing ending dot. @end table @section Feed section diff --git a/ffserver.c b/ffserver.c index ea2a2ae..8b005b9 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), -); } @@ -3737,6 +3738,38 @@ int main(int argc, char **argv) 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); + +fd = open(/dev/null, O_RDWR,0); +if (fd 0) { +ret = AVERROR(errno); +av_log(NULL, AV_LOG_ERROR, Unable to repoen file descriptors: %s\n, av_err2str(ret)); typo: repoen +exit(1); +} +dup2(fd, 0); +dup2(fd, 2); +if (strcmp(config.logfilename,-) != 0) nit: logfilename,_- [...] -- FFmpeg = Fanciful Fabulous Mysterious Perennial Evil Generator ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
Hi there, Please find the attached patch with modifications. Regards, Binathi On Mon, Nov 10, 2014 at 8:42 PM, Stefano Sabatini stefa...@gmail.com wrote: On date Monday 2014-11-10 13:26:14 +0530, Binathi Bingi encoded: [...] From c9d037758693a1522258a64849f7629d7cbd7408 Mon Sep 17 00:00:00 2001 From: Binathi binti...@gmail.com Provide complete name, the patch is used to track copyright. Date: Tue, 4 Nov 2014 21:42:07 +0530 Subject: [PATCH] Restore Daemon mode in FFserver Signed-off-by: Binathi Bingi binti...@gmail.com --- doc/ffserver.conf | 5 + doc/ffserver.texi | 7 --- ffserver.c| 33 + ffserver_config.c | 4 +++- ffserver_config.h | 1 + 5 files changed, 46 insertions(+), 4 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 Why NoDaemon followed by Daemon? This is confusing. + ## # 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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +Set no-daemon mode. This is the default. It would be nice to provide a more lenghty explanation about the daemon mode. Why is it useful? How does it differ with the NoDaemon mode? Why and how is it useful? + +@item Daemon +Set daemon mode. The default is NoDaemon missing ending dot. @end table @section Feed section diff --git a/ffserver.c b/ffserver.c index ea2a2ae..8b005b9 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), -); } @@ -3737,6 +3738,38 @@ int main(int argc, char **argv) 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); + +fd = open(/dev/null, O_RDWR,0); +if (fd 0) { +ret = AVERROR(errno); +av_log(NULL, AV_LOG_ERROR, Unable to repoen file descriptors: %s\n, av_err2str(ret)); typo: repoen +exit(1); +} +dup2(fd, 0); +dup2(fd, 2); +if (strcmp(config.logfilename,-) != 0) nit: logfilename,_- [...] -- FFmpeg = Fanciful Fabulous Mysterious Perennial Evil Generator ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel From 091b4a02c9325bea32b7f745d028ea72c8e1537e Mon Sep 17 00:00:00 2001 From: Binathi Bingi binti...@gmail.com Date: Tue, 4 Nov 2014 21:42:07 +0530 Subject: [PATCH] Restore Daemon mode in FFserver Signed-off-by: Binathi Bingi binti...@gmail.com Author:Binathi Bingi binti...@gmail.com --- doc/ffserver.conf | 4 doc/ffserver.texi | 11 +++ ffserver.c| 34 ++ ffserver_config.c | 4 ++-- ffserver_config.h | 1 + 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/doc/ffserver.conf b/doc/ffserver.conf index b756961..a017b3c 100644 --- a/doc/ffserver.conf +++ b/doc/ffserver.conf @@ -25,6 +25,10 @@ MaxBandwidth 1000 # '-' is the standard output. CustomLog - +# Enable Daemon, to launch FFserver in Daemon mode. +# For NoDaemon mode, suppress Daemon. +#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..0c10c2f 100644 --- a/doc/ffserver.texi +++ b/doc/ffserver.texi @@ -404,10 +404,13 @@ If
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
Le septidi 17 brumaire, an CCXXIII, Binathi Bingi a écrit : From f06f2b656feb9b01c42533bcdf51fc5190ca6f91 Mon Sep 17 00:00:00 2001 From: Binathi binti...@gmail.com Date: Tue, 4 Nov 2014 21:42:07 +0530 Subject: [PATCH] Restore Daemon mode in FFserver Signed-off-by: Binathi Bingi binti...@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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +Set no-daemon mode. This is the default. + +@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,
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
Hi there, Please find the attached patch. Regards Binathi On Thu, Nov 6, 2014 at 1:55 AM, Nicolas George geo...@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 likeFFserver 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 binti...@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 binti...@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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +Set no-daemon mode. This is the default. + +@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
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
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 likeFFserver 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 binti...@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 binti...@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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +Set no-daemon mode. This is the default. + +@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)); +
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
Hi there, 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 likeFFserver 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. Regards, Binathi On Wed, Nov 5, 2014 at 12:04 AM, Binathi Bingi binti...@gmail.com wrote: Hi there, I am sorry for the indentation errors in the above mail, it was because of bad email agent. In the attached patch, if the config.logfilename is not -, then stdout is closed and then I reopened. Reynaldo, is right. I don't think it is good idea to use dup2(). There were problems in running ffserver it couldn't start when I redirected the file descriptor to stdin, stdout, stderr using dup2(). I removed dup2() and just opened something neutral in /dev/null In this patch, I included error check for return value of open(). Cheers, Binathi On Tue, Nov 4, 2014 at 7:23 AM, Reynaldo H. Verdejo Pinochet reyna...@osg.samsung.com wrote: Hi On 11/03/2014 04:09 PM, Binathi Bingi wrote: Hello, Inside the child process, I closed the file descriptor and then reopened and redirected them using dup2() and later closed the opened file. I am not sure if I understood and used the functionality of dup2() in the right sense. Looks about right but see bellow. [..] @@ -3736,10 +3737,42 @@ int main(int argc, char **argv) build_feed_streams(); compute_bandwidth(); - + Remove trailing withe space +if (config.ffserver_daemon) { What follows needs to be indented +pid_t ffserver_id = 0; +pid_t sid = 0; + +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); Fix wrong indentation +} + +if (ffserver_id 0) { +exit(0); +} Drop the braces from single statement ifs blocks like this one. + +sid = setsid(); +if (sid 0) { +exit(1); +} Same as above + +if (strcmp(config.logfilename, -) != 0) { +close(0); Drop the != 0. Anything but 0 will evaluate to true anyway. You do this elsewhere on your own code. Be consistent. + +fd = open(/dev/null, O_RDWR); +dup2(fd,0); +dup2(fd,1); +dup2(fd,2); You sure you wana dup2() stdin, stdout and stderr on the above condition? Please double check. +close(fd); Above block needs to be re-indented. Also, check for failures in open and dup2() and react accordingly. I think Nicolas mentioned this already. + +} +} /* signal init */ signal(SIGPIPE, SIG_IGN); - + Again, drop the trailing white space you introduced here and on multiple lines in this same patch. It's getting there. Congrats ;) Bests, -- Reynaldo H. Verdejo Pinochet Open Source Group Samsung Research America / Silicon Valley ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel From 091aa564dc43bf18abd5dc596bf5350e92cad5dd Mon Sep 17 00:00:00 2001 From: Binathi binti...@gmail.com Date: Tue, 4 Nov 2014 21:42:07 +0530 Subject: [PATCH] Restore Daemon mode in FFserver Signed-off-by: Binathi Bingi binti...@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 + ## # 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 -@command{ffserver} will always work in no-daemon mode, and is
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
Hello, Inside the child process, I closed the file descriptor and then reopened and redirected them using dup2() and later closed the opened file. I am not sure if I understood and used the functionality of dup2() in the right sense. Regards, Binathi On Sat, Nov 1, 2014 at 10:18 PM, Lukasz Marek lukasz.m.lu...@gmail.com wrote: On 01.11.2014 17:20, Binathi Bingi wrote: +if (config.ffserver_daemon) { +int ffserver_id = 0; You may change int to pid_t too, which is actually returned by fork() +pid_t sid = 0; + +ffserver_id = fork(); ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel From 19c03969797800d43b7cf29fb0dfdbc2ac272a40 Mon Sep 17 00:00:00 2001 From: Binathi binti...@gmail.com Date: Fri, 31 Oct 2014 23:27:20 +0530 Subject: [PATCH] Restore Daemon functionality in FFserver Signed-off-by: Binathi Bingi binti...@gmail.com --- doc/ffserver.conf | 5 + doc/ffserver.texi | 7 --- ffserver.c| 39 --- ffserver_config.c | 6 -- ffserver_config.h | 1 + 5 files changed, 50 insertions(+), 8 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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +Set no-daemon mode. This is the default. + +@item Daemon +Set daemon mode. The default is NoDaemon @end table @section Feed section diff --git a/ffserver.c b/ffserver.c index ea2a2ae..5143b84 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; config.filename = av_strdup(/etc/ffserver.conf); parse_loglevel(argc, argv, options); @@ -3736,10 +3737,42 @@ int main(int argc, char **argv) build_feed_streams(); compute_bandwidth(); - + +if (config.ffserver_daemon) { + pid_t ffserver_id = 0; + pid_t sid = 0; + + 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); + + fd = open(/dev/null, O_RDWR); + dup2(fd,0); + dup2(fd,1); + dup2(fd,2); + close(fd); + +} +} /* 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..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; -- 1.9.1
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
Hi On 11/03/2014 04:09 PM, Binathi Bingi wrote: Hello, Inside the child process, I closed the file descriptor and then reopened and redirected them using dup2() and later closed the opened file. I am not sure if I understood and used the functionality of dup2() in the right sense. Looks about right but see bellow. [..] @@ -3736,10 +3737,42 @@ int main(int argc, char **argv) build_feed_streams(); compute_bandwidth(); - + Remove trailing withe space +if (config.ffserver_daemon) { What follows needs to be indented +pid_t ffserver_id = 0; +pid_t sid = 0; + +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); Fix wrong indentation +} + +if (ffserver_id 0) { +exit(0); +} Drop the braces from single statement ifs blocks like this one. + +sid = setsid(); +if (sid 0) { +exit(1); +} Same as above + +if (strcmp(config.logfilename, -) != 0) { +close(0); Drop the != 0. Anything but 0 will evaluate to true anyway. You do this elsewhere on your own code. Be consistent. + +fd = open(/dev/null, O_RDWR); +dup2(fd,0); +dup2(fd,1); +dup2(fd,2); You sure you wana dup2() stdin, stdout and stderr on the above condition? Please double check. +close(fd); Above block needs to be re-indented. Also, check for failures in open and dup2() and react accordingly. I think Nicolas mentioned this already. + +} +} /* signal init */ signal(SIGPIPE, SIG_IGN); - + Again, drop the trailing white space you introduced here and on multiple lines in this same patch. It's getting there. Congrats ;) Bests, -- Reynaldo H. Verdejo Pinochet Open Source Group Samsung Research America / Silicon Valley ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
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 binti...@gmail.com Date: Fri, 31 Oct 2014 23:27:20 +0530 Subject: [PATCH] Enable Daemon mode for FFServer Signed-off-by: Binathi binti...@gmail.com Improvements: Enable Daemon mode in FFServer Signed-off-by: Binathi binti...@gmail.com Improvements: Enable Daemon mode in FFServer Signed-off-by: Binathi binti...@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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +Set no-daemon mode. This is the default. + +@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 geo...@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 binti...@gmail.com Date: Fri, 31 Oct 2014 23:27:20 +0530 Subject: [PATCH]
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
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 binti...@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 binti...@gmail.com Improvements: Enable Daemon mode in FFServer Signed-off-by: Binathi binti...@gmail.com Improvements: Enable Daemon mode in FFServer Signed-off-by: Binathi binti...@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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +Set no-daemon mode. This is the default. + +@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); -}
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
On 01.11.2014 17:20, Binathi Bingi wrote: +if (config.ffserver_daemon) { +int ffserver_id = 0; You may change int to pid_t too, which is actually returned by fork() +pid_t sid = 0; + +ffserver_id = fork(); ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
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 binti...@gmail.com Date: Fri, 31 Oct 2014 23:27:20 +0530 Subject: [PATCH] Enable Daemon mode for FFServer Signed-off-by: Binathi binti...@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 + ## # 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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +@command{ffserver} will always work in daemon mode. + +@ Daemon +Set daemon mode. +@command{ffserver} will always work in daemon mode. To enable no-daemon mode, suppress this and enable NoDaemon. @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) { +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); +} + +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); +} +} /* 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; } 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 Binathi. On Thu, Oct 30, 2014 at 8:41 PM, Reynaldo H. Verdejo Pinochet reyna...@osg.samsung.com wrote: Hello On 10/30/2014 10:50 AM, Nicolas George wrote: [..] Third, I do not think this exact version is correct. If you make the daemon mode the default, then NoDaemon must not be ignored, it must have its specified effect: turn daemon off; if you do not make the daemon mode the default, then there must be an option to turn it on. IMHO, the best is to have both Daemon and NoDaemon mode. [..] I would prefer no-deamon mode been the default to not break current behavior till deamon-mode has had a good deal of production testing. I also think having a single Daemonize yes/no toggle should be enough but I'm OK either way in the context of this patch. Binathi: As an added bonus, try adding an -s to your commit command line so you get the Signed-off-by:
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
On Fri, Oct 31, 2014 at 11:32:37PM +0530, Binathi Bingi wrote: 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 binti...@gmail.com Date: Fri, 31 Oct 2014 23:27:20 +0530 Subject: [PATCH] Enable Daemon mode for FFServer Signed-off-by: Binathi binti...@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 + ## # 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 -@command{ffserver} will always work in no-daemon mode, and is -deprecated. +@command{ffserver} will always work in daemon mode. + +@ Daemon +Set daemon mode. +@command{ffserver} will always work in daemon mode. To enable no-daemon mode, suppress this and enable NoDaemon. @end table it seems the patch has been mangled @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) { +int ffserver_id = 0; +pid_t sid = 0; + +ffserver_id = fork(); there is something wrong with the indention here the code inside the if() probably should be indented by 4 more spaces + +if (ffserver_id 0) { +av_log(NULL, AV_LOG_WARNING, Fork failed!Couldn't launch ffserver in daemon mode.\n); IIRC Nicolas suggested to print the error corresponding to errno, that is not a fixed string. +exit(1); +} + +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); +} +} /* 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; why the 2 level duplicated comparission, a simple addition of a check for Daemon should be simpler [...] Thanks -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Freedom in capitalist society always remains about the same as it was in ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
I see your point. Changing the ffserver_config.c would help in addressing config files having NoDaemon option. You were right! Thanks. From 476c8605fab4d6c575c38796dd9dccaf854cf536 Mon Sep 17 00:00:00 2001 From: Binathi Bingi binti...@gmail.com Date: Thu, 30 Oct 2014 13:43:13 +0530 Subject: [PATCH] Restoring back NoDaemon option in ffserver.conf --- ffserver_config.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ffserver_config.c b/ffserver_config.c index e2c78d8..ab33ec5 100644 --- a/ffserver_config.c +++ b/ffserver_config.c @@ -358,6 +358,8 @@ 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, RTSPPort)) { ffserver_get_arg(arg, sizeof(arg), p); val = atoi(arg); -- 1.9.1 Binathi On Thu, Oct 30, 2014 at 8:04 AM, Michael Niedermayer michae...@gmx.at wrote: On Thu, Oct 30, 2014 at 07:49:10AM +0530, Binathi Bingi wrote: If you check the latest ffserver.conf file on GIT [ http://git.videolan.org/?p=ffmpeg.git http://www.google.com/url?q=http%3A%2F%2Fgit.videolan.org%2F%3Fp%3Dffmpeg.gitsa=Dsntz=1usg=AFQjCNEA5QH18TtMxcLhGx4b04pMUwSgYA ], there is NoDaemon option in it. So the patch is written as per latest version. But if desired can include NoDameon option in ffserver.conf and make futher changes. ffserver must work with any config file it worked with previously not only the ffserver.conf file from git [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are best at talking, realize last or never when they are wrong. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
Le nonidi 9 brumaire, an CCXXIII, Binathi Bingi a écrit : From 0fb7dcf1f126bd137e2b2025c5cd6cff4af65801 Mon Sep 17 00:00:00 2001 From: Binathi Bingi binti...@gmail.com Date: Thu, 30 Oct 2014 01:14:08 +0530 Subject: [PATCH] ffserver: enable back deamon mode --- ffserver.c| 34 ++ ffserver_config.c | 2 -- 2 files changed, 30 insertions(+), 6 deletions(-) Thanks for the submission. I believe you forgot to update the docs (doc/ffserver.texi). diff --git a/ffserver.c b/ffserver.c index ea2a2ae..96e312e 100644 --- a/ffserver.c +++ b/ffserver.c @@ -236,7 +236,7 @@ static int rtp_new_av_stream(HTTPContext *c, HTTPContext *rtsp_c); static const char *my_program_name; - I think the empty line could stay. +static int ffserver_daemon; static int no_launch; static int need_to_start_children; @@ -3671,6 +3671,7 @@ static void handle_child_exit(int sig) static void opt_debug(void) { config.debug = 1; +ffserver_daemon = 1; This is strange: debugging is precisely when you do not want ffserver to fork into background. If daemon mode is enabled by default, debug mode should probably disable it, but not the other way around. snprintf(config.logfilename, sizeof(config.logfilename), -); } @@ -3694,7 +3695,7 @@ int main(int argc, char **argv) { struct sigaction sigact = { { 0 } }; int ret = 0; - The empty line can stay. +ffserver_daemon = 1; This is changing the default behaviour again, I am not sure this is a good idea nowadays. We can probably leave that line out for the time being. config.filename = av_strdup(/etc/ffserver.conf); parse_loglevel(argc, argv, options); @@ -3736,10 +3737,35 @@ int main(int argc, char **argv) build_feed_streams(); compute_bandwidth(); - Ditto for the empty line. +if(ffserver_daemon){ Style nitpick: space between if/for/while/switch and parentheses and before the brace. +int ffserver_id = 0; +pid_t sid = 0; + +ffserver_id = fork(); // Create child process Small nitpick: the comment seems useless. + +if (ffserver_id 0){ +printf(fork failed!\n); //Indication of failure This kind of message should go to stderr, never stdout. In FFmpeg code, use av_log() for that. Also, please include the error reason, using errno (probably through AVERROR and av_err2str()). And the comment seems useless too. +exit(1); +} + +if(ffserver_id 0){ //Parent process need to kill I do not understand the comment. And ditto for spacing. +exit(0); +} + +sid = setsid(); //set new session +if(sid 0){ +exit(1); //return failure Ditto for spacing and comment. +} + +open (/dev/null, O_RDWR); + +if (strcmp(config.logfilename, -) != 0) { +close(1); +} This looks strange. If the purpose is to sanitize stdio after detaching, there are some bits missing. +} /* signal init */ signal(SIGPIPE, SIG_IGN); - + I have no idea why Git proposes to replace an empty line with an identical empty line, but that does not matter. 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..e2c78d8 100644 --- a/ffserver_config.c +++ b/ffserver_config.c @@ -358,8 +358,6 @@ 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); Superseded by: +} else if (!av_strcasecmp(cmd, NoDaemon)) { +WARNING(NoDaemon option has no effect, you should remove it\n); First, you may notice that your mail user agent has mangled the patch by adding a line break before the end of the last line. That happens when pasting a patch directly in the body of a mail with a bad mail user agent. This is of no matter here because of the next point. Second, this patch needs to be squashed with the previous one, so that they are a single commit. If you had not yet committed, you could do that using the --amend option to git commit. Since you have already committed, you must use git rebase -i master (assuming you created a branch from the branch called master). Third, I do not think this exact version is correct. If you make the daemon mode the default, then NoDaemon must not be ignored, it must have its specified effect: turn daemon off; if you do not make the daemon mode the default, then there must be an option to turn it on. IMHO, the best is to have both Daemon and NoDaemon mode. } else if (!av_strcasecmp(cmd, RTSPPort)) { ffserver_get_arg(arg,
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
Hello On 10/30/2014 10:50 AM, Nicolas George wrote: [..] Third, I do not think this exact version is correct. If you make the daemon mode the default, then NoDaemon must not be ignored, it must have its specified effect: turn daemon off; if you do not make the daemon mode the default, then there must be an option to turn it on. IMHO, the best is to have both Daemon and NoDaemon mode. [..] I would prefer no-deamon mode been the default to not break current behavior till deamon-mode has had a good deal of production testing. I also think having a single Daemonize yes/no toggle should be enough but I'm OK either way in the context of this patch. Binathi: As an added bonus, try adding an -s to your commit command line so you get the Signed-off-by: line added, this is customary. Bests, -- Reynaldo H. Verdejo Pinochet Open Source Group Samsung Research America / Silicon Valley ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
On Thu, Oct 30, 2014 at 02:14:59AM +0530, Binathi Bingi wrote: From 0fb7dcf1f126bd137e2b2025c5cd6cff4af65801 Mon Sep 17 00:00:00 2001 From: Binathi Bingi binti...@gmail.com Date: Thu, 30 Oct 2014 01:14:08 +0530 Subject: [PATCH] ffserver: enable back deamon mode --- ffserver.c| 34 ++ ffserver_config.c | 2 -- 2 files changed, 30 insertions(+), 6 deletions(-) [...] diff --git a/ffserver_config.c b/ffserver_config.c index e44cdf7..e2c78d8 100644 --- a/ffserver_config.c +++ b/ffserver_config.c @@ -358,8 +358,6 @@ 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, RTSPPort)) { ffserver_get_arg(arg, sizeof(arg), p); val = atoi(arg); this would break configuration files which contain the NoDaemon option [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 1 Used only once- Some unspecified defect prevented a second use In good condition - Can be repaird by experienced expert As is - You wouldnt want it even if you were payed for it, if you knew ... signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] OPW Qualification Task: Enable daemon mode for FFserver
On Thu, Oct 30, 2014 at 07:49:10AM +0530, Binathi Bingi wrote: If you check the latest ffserver.conf file on GIT [ http://git.videolan.org/?p=ffmpeg.git http://www.google.com/url?q=http%3A%2F%2Fgit.videolan.org%2F%3Fp%3Dffmpeg.gitsa=Dsntz=1usg=AFQjCNEA5QH18TtMxcLhGx4b04pMUwSgYA], there is NoDaemon option in it. So the patch is written as per latest version. But if desired can include NoDameon option in ffserver.conf and make futher changes. ffserver must work with any config file it worked with previously not only the ffserver.conf file from git [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are best at talking, realize last or never when they are wrong. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel