Re: [FFmpeg-devel] [PATCH 05/11] ffserver: allow skip setting defaults
Hi
On 11/21/2014 09:13 PM, Lukasz Marek wrote:
[...]
@@ -1032,6 +1059,14 @@ static int ffserver_parse_config_stream(FFServerConfig
*config, const char *cmd,
} else if (!av_strcasecmp(cmd, File) || !av_strcasecmp(cmd,
ReadOnlyFile)) {
ffserver_get_arg(stream-feed_filename,
sizeof(stream-feed_filename),
p);
+} else if (!av_strcasecmp(cmd, UseDefaults)) {
+if (config-stream_use_defaults 1)
+ERROR(Multiple UseDefaults/NoDefaults entries.\n);
+config-stream_use_defaults = 3;
+} else if (!av_strcasecmp(cmd, NoDefaults)) {
+if (config-stream_use_defaults 1)
+ERROR(Multiple UseDefaults/NoDefaults entries.\n);
+config-stream_use_defaults = 2;
} else {
ERROR(Invalid entry '%s' inside Stream/Stream\n, cmd);
}
I think these should be WARNING()s. Patch looks good otherwise.
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] [PATCH 05/11] ffserver: allow skip setting defaults
On 21.11.2014 16:31, Reynaldo H. Verdejo Pinochet wrote:
Hi
On 11/20/2014 10:09 PM, Lukasz Marek wrote:
On 18.11.2014 23:25, Reynaldo H. Verdejo Pinochet wrote:
[..]
I do think undefined behavior should be avoided if possible
without too much hassle, so if we go with the former I would
appreciate doc entries specifying the options precedence. Brownie
points+ if an odd combination fires a warning().
Not sure I follow, should I change anything? IMHO my proposal it is not
really complex and fully customizable.
After Nicolas observation I think it's OK for you to keep
the original implementation. Just see if you can document
the options' precedence when an XYES and XNO are present in
the config file and consider adding a warning in such case.
Thanks a lot!
Bests,
updated patch
From 03619921125bf66562d6487e8b5ea64dfa2ac27b Mon Sep 17 00:00:00 2001
From: Lukasz Marek lukasz.m.lu...@gmail.com
Date: Sat, 15 Nov 2014 18:43:41 +0100
Subject: [PATCH 5/9] ffserver: allow skip setting defaults
Signed-off-by: Lukasz Marek lukasz.m.lu...@gmail.com
---
doc/ffserver.texi | 11 +++
ffserver.c| 1 +
ffserver_config.c | 35 +++
ffserver_config.h | 2 ++
4 files changed, 49 insertions(+)
diff --git a/doc/ffserver.texi b/doc/ffserver.texi
index b7c5b6a..83b6520 100644
--- a/doc/ffserver.texi
+++ b/doc/ffserver.texi
@@ -408,6 +408,12 @@ ignored, and the log is written to standard output.
Set no-daemon mode. This option is currently ignored since now
@command{ffserver} will always work in no-daemon mode, and is
deprecated.
+
+@item UseDefaults
+@item NoDefaults
+Control whether default codec options are used for the all streams or not.
+Each stream may overwrite this setting for its own. Default is @var{UseDefaults}.
+The lastest occurrence overrides previous if multiple definitions.
@end table
@section Feed section
@@ -571,6 +577,11 @@ deprecated in favor of @option{Metadata}.
@item Metadata @var{key} @var{value}
Set metadata value on the output stream.
+@item UseDefaults
+@item NoDefaults
+Control whether default codec options are used for the stream or not.
+Default is @var{UseDefaults} unless disabled globally.
+
@item NoAudio
@item NoVideo
Suppress audio/video.
diff --git a/ffserver.c b/ffserver.c
index 933eb0e..e24243d 100644
--- a/ffserver.c
+++ b/ffserver.c
@@ -201,6 +201,7 @@ static FFServerConfig config = {
.nb_max_http_connections = 2000,
.nb_max_connections = 5,
.max_bandwidth = 1000,
+.use_defaults = 1,
};
static void new_connection(int server_fd, int is_rtsp);
diff --git a/ffserver_config.c b/ffserver_config.c
index 73cd881..8339638 100644
--- a/ffserver_config.c
+++ b/ffserver_config.c
@@ -191,6 +191,8 @@ static void add_codec(FFServerStream *stream, AVCodecContext *av,
av_log(NULL, AV_LOG_WARNING,
Something is wrong, %d options are not set!\n, av_dict_count(*opts));
+if (config-stream_use_defaults) {
+//TODO: reident
/* compute default parameters */
switch(av-codec_type) {
case AVMEDIA_TYPE_AUDIO:
@@ -255,6 +257,25 @@ static void add_codec(FFServerStream *stream, AVCodecContext *av,
default:
abort();
}
+} else {
+switch(av-codec_type) {
+case AVMEDIA_TYPE_AUDIO:
+if (av-bit_rate == 0)
+report_config_error(config-filename, config-line_num, AV_LOG_ERROR,
+config-errors, audio bit rate is not set\n);
+if (av-sample_rate == 0)
+report_config_error(config-filename, config-line_num, AV_LOG_ERROR,
+config-errors, audio sample rate is not set\n);
+break;
+case AVMEDIA_TYPE_VIDEO:
+if (av-width == 0 || av-height == 0)
+report_config_error(config-filename, config-line_num, AV_LOG_ERROR,
+config-errors, video size is not set\n);
+break;
+default:
+av_assert0(0);
+}
+}
st = av_mallocz(sizeof(AVStream));
if (!st)
@@ -583,6 +604,10 @@ static int ffserver_parse_config_global(FFServerConfig *config, const char *cmd,
ffserver_get_arg(config-logfilename, sizeof(config-logfilename), p);
} else if (!av_strcasecmp(cmd, LoadModule)) {
ERROR(Loadable modules are no longer supported\n);
+} else if (!av_strcasecmp(cmd, NoDefaults)) {
+config-use_defaults = 0;
+} else if (!av_strcasecmp(cmd, UseDefaults)) {
+config-use_defaults = 1;
} else
ERROR(Incorrect keyword: '%s'\n, cmd);
return 0;
@@ -738,6 +763,7 @@ static int ffserver_parse_config_stream(FFServerConfig *config, const char *cmd,
config-guessed_audio_codec_id = AV_CODEC_ID_NONE;
config-guessed_video_codec_id = AV_CODEC_ID_NONE;
}
+config-stream_use_defaults = config-use_defaults;
*pstream = stream;
Re: [FFmpeg-devel] [PATCH 05/11] ffserver: allow skip setting defaults
On 18.11.2014 23:25, Reynaldo H. Verdejo Pinochet wrote: Hi On 11/18/2014 06:54 PM, Nicolas George wrote: L'octidi 28 brumaire, an CCXXIII, Reynaldo H. Verdejo Pinochet a écrit : I think I commented about this before but having yesvar novar options seems redundant. Having var = yes or no if absent (or the other way around depending on the intended default) seems less cumbersome and should simplify the code a bit too, avoiding checking I think exactly the opposite. People who build reliable configurations want to avoid relying on default values as much as possible, because default values can change without notice. Therefore, they need to be able to specify explicitly any behaviour, even if it is currently the default. for the two and imposing a precedence, which is not always documented. I think we can implicitly document that the behaviour for contradictory options is nasal demons. Good point I guess. OK all the same as long as we keep it consistent across config files. Phrased as a question to the author for this very reason. I do think undefined behavior should be avoided if possible without too much hassle, so if we go with the former I would appreciate doc entries specifying the options precedence. Brownie points+ if an odd combination fires a warning(). Not sure I follow, should I change anything? IMHO my proposal it is not really complex and fully customizable. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 05/11] ffserver: allow skip setting defaults
Hi
On 11/16/2014 10:46 PM, Lukasz Marek wrote:
[..]
diff --git a/doc/ffserver.texi b/doc/ffserver.texi
index b7c5b6a..d3ff13e 100644
--- a/doc/ffserver.texi
+++ b/doc/ffserver.texi
@@ -408,6 +408,11 @@ ignored, and the log is written to standard output.
Set no-daemon mode. This option is currently ignored since now
@command{ffserver} will always work in no-daemon mode, and is
deprecated.
+
+@item UseDefaults
+@item NoDefaults
+Control whether default codec options are used for the all streams or not.
+Each stream may overwrite this setting for its own. Default is
@var{UseDefaults}.
[...]
I think I commented about this before but having yesvar novar
options seems redundant. Having var = yes or no if absent (or the
other way around depending on the intended default) seems less
cumbersome and should simplify the code a bit too, avoiding checking
for the two and imposing a precedence, which is not always
documented. What do you think? This happens twice on this patch
IIRC.
Otherwise seems OK to push.
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] [PATCH 05/11] ffserver: allow skip setting defaults
L'octidi 28 brumaire, an CCXXIII, Reynaldo H. Verdejo Pinochet a écrit : I think I commented about this before but having yesvar novar options seems redundant. Having var = yes or no if absent (or the other way around depending on the intended default) seems less cumbersome and should simplify the code a bit too, avoiding checking I think exactly the opposite. People who build reliable configurations want to avoid relying on default values as much as possible, because default values can change without notice. Therefore, they need to be able to specify explicitly any behaviour, even if it is currently the default. for the two and imposing a precedence, which is not always documented. I think we can implicitly document that the behaviour for contradictory options is nasal demons. 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] [PATCH 05/11] ffserver: allow skip setting defaults
Hi On 11/18/2014 06:54 PM, Nicolas George wrote: L'octidi 28 brumaire, an CCXXIII, Reynaldo H. Verdejo Pinochet a écrit : I think I commented about this before but having yesvar novar options seems redundant. Having var = yes or no if absent (or the other way around depending on the intended default) seems less cumbersome and should simplify the code a bit too, avoiding checking I think exactly the opposite. People who build reliable configurations want to avoid relying on default values as much as possible, because default values can change without notice. Therefore, they need to be able to specify explicitly any behaviour, even if it is currently the default. for the two and imposing a precedence, which is not always documented. I think we can implicitly document that the behaviour for contradictory options is nasal demons. Good point I guess. OK all the same as long as we keep it consistent across config files. Phrased as a question to the author for this very reason. I do think undefined behavior should be avoided if possible without too much hassle, so if we go with the former I would appreciate doc entries specifying the options precedence. Brownie points+ if an odd combination fires a warning(). 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
