[FFmpeg-devel] [PATCH 2/5] lavf/concatdec: support per-file options.

[Nicolas George]

Andreas Rheinhardt (12021-07-28):
> > -    if ((ret = avformat_open_input(&cat->avf, file->url, NULL, NULL)) < 0 ||
> > +    ret = av_dict_copy(&options, file->options, 0);
> > +    if (ret < 0)
> > +        return ret;
> > +
> > +    if ((ret = avformat_open_input(&cat->avf, file->url, NULL, &options)) < 0 ||
> >          (ret = avformat_find_stream_info(cat->avf, NULL)) < 0) {
> >          av_log(avf, AV_LOG_ERROR, "Impossible to open '%s'\n", file->url);
> >          avformat_close_input(&cat->avf);
> >          return ret;
> 
> options would leak here.

Fixed.

> > +            if (!(key = av_get_token((const char **)&cursor, SPACE_CHARS)) ||
> > +                !(val = av_get_token((const char **)&cursor, SPACE_CHARS))) {
> > +                av_log(avf, AV_LOG_ERROR, "Line %d: key and val required\n", line);
> > +                FAIL(AVERROR_INVALIDDATA);
> 
> 1. key might leak here.
> 
> 2. The error message and the return code are wrong: av_get_token() only
> fails upon allocation error; it does not fail when *cursor == \0.

Fixed.

> (The code already suffers from this misconception.)

I intend to rewrite the parser entirely at some point.

> 
> > +            }
> > +            ret = av_dict_set(&file->options, key, val,
> > +                              AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL);
> > +            if (ret < 0) {
> > +                av_freep(&key);
> > +                av_freep(&val);
> 
> Double free.

Uh? Oh, I had the logic backwards. Fixed.

Will send a new series soon.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210814/9aae6862/attachment.sig>