[FFmpeg-devel] [PATCH 6/9] ffserver_config: handle codec private options

[Lukasz Marek]

On 12.11.2014 08:23, Reynaldo H. Verdejo Pinochet wrote:
>> [..]
>> @@ -293,16 +295,31 @@ static int ffserver_opt_preset(const char *arg,
>>               ret = AVERROR(EINVAL);
>>               break;
>>           }
>> -        if (audio_id && !strcmp(tmp, "acodec")) {
>> -            *audio_id = opt_codec(tmp2, AVMEDIA_TYPE_AUDIO);
>> -        } else if (video_id && !strcmp(tmp, "vcodec")){
>> -            *video_id = opt_codec(tmp2, AVMEDIA_TYPE_VIDEO);
>> -        } else if(!strcmp(tmp, "scodec")) {
>> -            /* opt_subtitle_codec(tmp2); */
>> -        } else if (avctx && (ret = ffserver_opt_default(tmp, tmp2,
>> avctx, type)) < 0) {
>> -            fprintf(stderr, "%s: Invalid option or argument: '%s',
>> parsed as "
>> -                    "'%s' = '%s'\n", filename, line, tmp, tmp2);
>> -            break;
>> +        if ((!strcmp(tmp, "acodec") && avctx->codec_type ==
>> AVMEDIA_TYPE_AUDIO) ||
>> +             !strcmp(tmp, "vcodec") && avctx->codec_type ==
>> AVMEDIA_TYPE_VIDEO)
>> +        {
>> +            if (ffserver_set_codec(avctx, tmp2, config, line_num) < 0)
>> +                break;
>
> Factor in previous if() condition?

Not sure it is possible. Outer if has an else block, this should break 
whole loop.

>
>> +        } else if (!strcmp(tmp, "scodec")) {
>> +            /* nothing to do */
>
> Not sure why are we leaving this one there? Proly drop and add
> an informative comment if needed.

It cannot be dropped. It would cause to try to apply it as an option.
I added return here with an error.

>
>> +        } else {
>> +            int type;
>> +            AVDictionary **opts;
>
> Add blank line please (Can you do it for other similar occurrences?)
>
>> [..]
>> @@ -406,27 +424,67 @@ static int ffserver_set_float_param(float *dest,
>> const char *value, float factor
>>       return AVERROR(EINVAL);
>>   }
>>
>> -static int ffserver_save_avoption(const char *opt, const char *arg,
>> AVDictionary **dict,
>> +static int ffserver_save_avoption(AVCodecContext *ctx, const char
>> *opt, const char *arg, AVDictionary **dict,
>>                                     int type, FFServerConfig *config,
>> int line_num)
>>   {
>> +    static int hinted = 0;
>>       int ret = 0;
>>       AVDictionaryEntry *e;
>> -    const AVOption *o = av_opt_find(config->dummy_ctx, opt, NULL,
>> type | AV_OPT_FLAG_ENCODING_PARAM, AV_OPT_SEARCH_CHILDREN);
>> +    const AVOption *o = NULL;
>> +    const char *option = NULL;
>> +    const char *codec_name = NULL;
>> +
>> +    if (strchr(opt, ':')) {
>> +        //explicit private option
>> +        char buff[1024];
>
> See above (there are others though)
>
>> +        snprintf(buff, sizeof(buff), "%s", opt);
>
> Pointless sizeof but it's OK.
>
>> +        codec_name = buff;
>> +        option = strchr(buff, ':');
>> +        buff[option - buff] = '\0';
>
> Can't strchr() return NULL and wreak havoc here?

It can, but not here. String already was checked for ':' existence.

> Rest looks OK at a first glance. Thanks a lot

Updated patch is attached.
I hope I haven't missed something.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ffserver_config-handle-codec-private-options.patch
Type: text/x-patch
Size: 22423 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141112/183e5798/attachment.bin>