On 25.10.2014 19:53, Lukasz Marek wrote:
On 24.10.2014 00:18, Reynaldo H. Verdejo Pinochet wrote:
Hi Lukasz
+static int ffserver_apply_stream_config(AVCodecContext *enc, const
AVDictionary *conf, AVDictionary **opts)
+{
+ static const char *error_message = "Cannot parse '%s' as number
for %s parameter.\n";
+ AVDictionaryEntry *e;
+ char *tailp;
+ int ret = 0;
+
+#define SET_INT_PARAM(factor, param, key) \
+ if ((e = av_dict_get(conf, #key, NULL, 0))) { \
+ if (!e->value[0]) { \
+ av_log(NULL, AV_LOG_ERROR, error_message, e->value,
#key); \
+ ret = AVERROR(EINVAL); \
+ } \
+ enc->param = strtol(e->value, &tailp, 0); \
+ if (factor) enc->param *= (factor); \
+ if (tailp[0] || errno) { \
+ av_log(NULL, AV_LOG_ERROR, error_message, e->value,
#key); \
+ ret = AVERROR(errno); \
+ } \
+ }
+#define SET_DOUBLE_PARAM(factor, param, key) \
+ if ((e = av_dict_get(conf, #key, NULL, 0))) { \
+ if (!e->value[0]) { \
+ av_log(NULL, AV_LOG_ERROR, error_message, e->value,
#key); \
+ ret = AVERROR(EINVAL); \
+ } \
+ enc->param = strtod(e->value, &tailp); \
+ if (factor) enc->param *= (factor); \
+ if (tailp[0] || errno) { \
+ av_log(NULL, AV_LOG_ERROR, error_message, e->value,
#key); \
+ ret = AVERROR(errno); \
+ } \
+ }
Can you turn those two macros into static inline funcs?. Also,
looks like it shouldn't be too hard to factor those two into
just 1 func. This is mostly to aid debugging. Nothing vital.
I changed macros to functions. I think inline is not required in such
code. Small comment, there is plenty of atoi in parsing code.
It returns 0 in case parsed string is not a number and ignores random
suffixed. Probably more problems can be pointed here. I prepared these
functions to replace all atoi with them.
Since these functions allows to check ranges, I split back all options
to separate if's, so every option can have its own range.
[..]
@@ -624,136 +712,104 @@ static int
ffserver_parse_config_stream(FFServerConfig *config, const char *cmd,
stream->max_time = atof(arg) * 1000;
} else if (!av_strcasecmp(cmd, "AudioBitRate")) {
ffserver_get_arg(arg, sizeof(arg), p);
- config->audio_enc.bit_rate = lrintf(atof(arg) * 1000);
- } else if (!av_strcasecmp(cmd, "AudioChannels")) {
+ av_dict_set_int(&config->audio_conf, cmd, lrintf(atof(arg) *
1000), 0);
+ } else if (!av_strcasecmp(cmd, "AudioChannels") ||
+ !av_strcasecmp(cmd, "AudioSampleRate")) {
ffserver_get_arg(arg, sizeof(arg), p);
- config->audio_enc.channels = atoi(arg);
- } else if (!av_strcasecmp(cmd, "AudioSampleRate")) {
- ffserver_get_arg(arg, sizeof(arg), p);
- config->audio_enc.sample_rate = atoi(arg);
+ av_dict_set(&config->audio_conf, cmd, arg, 0);
Here and on every occurrence:
Any particular reason why not to check av_dict_set*()'s return
for < 0? Only reason I ask is because the config code already
has failed allocation checks elsewhere. Also, should help avoiding
some coverity scan noise.
I forgot about that. Checks added.
[..]
-
+ AVDictionary *video_opts; /* Contains AVOptions for video
encoder */
+ AVDictionary *video_conf; /* Contains values stored in video
AVCodecContext.fields */
+ AVDictionary *audio_opts; /* Contains AVOptions for audio
encoder */
+ AVDictionary *audio_conf; /* Contains values stored in audio
AVCodecContext.fields */
Would drop the repeated "Contains".
Dropped.
Patch OK'ed by Reynaldo in private email. Pushed.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
