On 18.11.2014 21:44, Reynaldo H. Verdejo Pinochet wrote:
Hi Lukasz

On 11/16/2014 10:46 PM, Lukasz Marek wrote:
[..]
@@ -174,13 +174,20 @@ void ffserver_parse_acl_row(FFServerStream *stream, 
FFServerStream* feed,
  }

  /* add a codec and set the default parameters */
-static void add_codec(FFServerStream *stream, AVCodecContext *av)
+static void add_codec(FFServerStream *stream, AVCodecContext *av, 
FFServerConfig *config)
  {
      AVStream *st;
+    AVDictionary **opts;

      if(stream->nb_streams >= FF_ARRAY_ELEMS(stream->streams))
          return;

+    opts = av->codec_type == AVMEDIA_TYPE_AUDIO ? &config->audio_opts : 
&config->video_opts;
+    av_opt_set_dict2(av->priv_data, opts, AV_OPT_SEARCH_CHILDREN);
+    av_opt_set_dict2(av, opts, AV_OPT_SEARCH_CHILDREN);
+    if (av_dict_count(*opts))
+        av_log(NULL, AV_LOG_ERROR, "Something went wrong, %d options not 
set!!!\n", av_dict_count(*opts));
+

Is this really an error? OK to push if so. Otherwise demote to
warning and push the updated patch. Usual comments about your
line lengths apply too but are not blockers.

As a general comment, I would avoid the grammatical "!!!"s and
such to denote severity. We have the log categories for that.
This is also just a tip, not something you need to change.

Bests,

updated patch
>From 59ccba52f3d90f992ecd65be585d0b75294ba263 Mon Sep 17 00:00:00 2001
From: Lukasz Marek <lukasz.m.lu...@gmail.com>
Date: Sun, 16 Nov 2014 01:33:19 +0100
Subject: [PATCH 4/9] ffserver_config: remove ffserver_apply_stream_config
 function

This function became very short and can be logically merged with add_codec().

Signed-off-by: Lukasz Marek <lukasz.m.lu...@gmail.com>
---
 ffserver_config.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/ffserver_config.c b/ffserver_config.c
index 67c4890..73cd881 100644
--- a/ffserver_config.c
+++ b/ffserver_config.c
@@ -174,13 +174,23 @@ void ffserver_parse_acl_row(FFServerStream *stream, FFServerStream* feed,
 }
 
 /* add a codec and set the default parameters */
-static void add_codec(FFServerStream *stream, AVCodecContext *av)
+static void add_codec(FFServerStream *stream, AVCodecContext *av,
+                      FFServerConfig *config)
 {
     AVStream *st;
+    AVDictionary **opts;
 
     if(stream->nb_streams >= FF_ARRAY_ELEMS(stream->streams))
         return;
 
+    opts = av->codec_type == AVMEDIA_TYPE_AUDIO ?
+           &config->audio_opts : &config->video_opts;
+    av_opt_set_dict2(av->priv_data, opts, AV_OPT_SEARCH_CHILDREN);
+    av_opt_set_dict2(av, opts, AV_OPT_SEARCH_CHILDREN);
+    if (av_dict_count(*opts))
+        av_log(NULL, AV_LOG_WARNING,
+               "Something is wrong, %d options are not set!\n", av_dict_count(*opts));
+
     /* compute default parameters */
     switch(av->codec_type) {
     case AVMEDIA_TYPE_AUDIO:
@@ -684,14 +694,6 @@ static int ffserver_parse_config_feed(FFServerConfig *config, const char *cmd, c
     return 0;
 }
 
-static void ffserver_apply_stream_config(AVCodecContext *enc, AVDictionary **opts)
-{
-    av_opt_set_dict2(enc->priv_data, opts, AV_OPT_SEARCH_CHILDREN);
-    av_opt_set_dict2(enc, opts, AV_OPT_SEARCH_CHILDREN);
-    if (av_dict_count(*opts))
-        av_log(NULL, AV_LOG_ERROR, "Something went wrong, %d options not set!!!\n", av_dict_count(*opts));
-}
-
 static int ffserver_parse_config_stream(FFServerConfig *config, const char *cmd, const char **p,
                                         FFServerStream **pstream)
 {
@@ -1013,15 +1015,13 @@ static int ffserver_parse_config_stream(FFServerConfig *config, const char *cmd,
                 config->dummy_actx->codec_id = config->guessed_audio_codec_id;
             if (!config->no_audio && config->dummy_actx->codec_id != AV_CODEC_ID_NONE) {
                 AVCodecContext *audio_enc = avcodec_alloc_context3(avcodec_find_encoder(config->dummy_actx->codec_id));
-                ffserver_apply_stream_config(audio_enc, &config->audio_opts);
-                add_codec(stream, audio_enc);
+                add_codec(stream, audio_enc, config);
             }
             if (config->dummy_vctx->codec_id == AV_CODEC_ID_NONE)
                 config->dummy_vctx->codec_id = config->guessed_video_codec_id;
             if (!config->no_video && config->dummy_vctx->codec_id != AV_CODEC_ID_NONE) {
                 AVCodecContext *video_enc = avcodec_alloc_context3(avcodec_find_encoder(config->dummy_vctx->codec_id));
-                ffserver_apply_stream_config(video_enc, &config->video_opts);
-                add_codec(stream, video_enc);
+                add_codec(stream, video_enc, config);
             }
         }
         av_dict_free(&config->video_opts);
-- 
1.9.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to