[FFmpeg-devel] [PATCH 04/11] ffserver_config: remove ffserver_apply_stream_config function

[Lukasz Marek]

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.