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

2014-11-13 Thread Nicolas George
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

2014-11-12 Thread Stefano Sabatini
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

2014-11-12 Thread Binathi Bingi
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

2014-11-11 Thread Stefano Sabatini
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

2014-11-11 Thread Binathi Bingi
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

2014-11-11 Thread Reynaldo H. Verdejo Pinochet

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

2014-11-10 Thread Nicolas George
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

2014-11-10 Thread Stefano Sabatini
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

2014-11-10 Thread Binathi Bingi
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

2014-11-09 Thread Nicolas George
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

2014-11-07 Thread Binathi Bingi
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

2014-11-05 Thread Nicolas George
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

2014-11-04 Thread Binathi Bingi
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

2014-11-03 Thread Binathi Bingi
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

2014-11-03 Thread Reynaldo H. Verdejo Pinochet
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

2014-11-01 Thread Binathi Bingi
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

2014-11-01 Thread Nicolas George
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

2014-11-01 Thread Lukasz Marek

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

2014-10-31 Thread Binathi Bingi
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

2014-10-31 Thread Michael Niedermayer
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

2014-10-30 Thread Binathi Bingi
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

2014-10-30 Thread Nicolas George
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

2014-10-30 Thread Reynaldo H. Verdejo Pinochet
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

2014-10-29 Thread Michael Niedermayer
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

2014-10-29 Thread Michael Niedermayer
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