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.

Yes an no. For now it could be assert, as options are already validated, but AVOption API may change (some dynamic validation or whatever) so decided just to notice, but you are right, demoted to a warning locally. Error should terminate, but I think warning is good enough.


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.

No, it is good point. replaced "!!!" by "!" locally.
TBH it was just debug comment, but I decided to keep it.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to