[FFmpeg-devel] [PATCH 6/9] ffserver_config: handle codec private options
Lukasz Marek
lukasz.m.luki2 at gmail.com
Wed Nov 12 22:07:36 CET 2014
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>
More information about the ffmpeg-devel
mailing list