[FFmpeg-devel] [PATCH 2/6] lavf/concat: add file_packet_meta directive

[Andreas Rheinhardt]

Nicolas George:
> Same as file_packet_metadata without the double parsing.
> 
> Signed-off-by: Nicolas George <george at nsup.org>
> ---
>  doc/demuxers.texi       | 5 +++++
>  libavformat/concatdec.c | 8 ++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> index eb3351833a..f338700396 100644
> --- a/doc/demuxers.texi
> +++ b/doc/demuxers.texi
> @@ -151,6 +151,11 @@ Metadata of the packets of the file. The specified metadata will be set for
>  each file packet. You can specify this directive multiple times to add multiple
>  metadata entries.
>  
> + at item @code{file_packet_meta @var{key} @var{value}}
> +Metadata of the packets of the file. The specified metadata will be set for
> +each file packet. You can specify this directive multiple times to add multiple
> +metadata entries.
> +
>  @item @code{option @var{key} @var{value}}
>  Option to access, open and probe the file.
>  Can be present multiple times.
> diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
> index 223c7e36c4..76f3fafa50 100644
> --- a/libavformat/concatdec.c
> +++ b/libavformat/concatdec.c
> @@ -425,6 +425,7 @@ typedef enum ParseDirective {
>     DIR_DURATION,
>     DIR_INPOINT,
>     DIR_OUTPOINT,
> +   DIR_FPMETA,
>     DIR_FPMETAS,
>     DIR_OPTION,
>     DIR_STREAM,
> @@ -437,6 +438,7 @@ static const ParseSyntax syntax[] = {
>      [DIR_DURATION ] = { "duration",             "d",  NEEDS_FILE },
>      [DIR_INPOINT  ] = { "inpoint",              "d",  NEEDS_FILE },
>      [DIR_OUTPOINT ] = { "outpoint",             "d",  NEEDS_FILE },
> +    [DIR_FPMETA   ] = { "file_packet_meta",     "ks", NEEDS_FILE },
>      [DIR_FPMETAS  ] = { "file_packet_metadata", "s",  NEEDS_FILE },
>      [DIR_OPTION   ] = { "option",               "ks", NEEDS_FILE | NEEDS_UNSAFE },
>      [DIR_STREAM   ] = { "stream",               "",   0 },
> @@ -544,6 +546,12 @@ static int concat_parse_script(AVFormatContext *avf)
>          case DIR_OUTPOINT:
>              file->outpoint = arg_int[0];
>              break;
> +        case DIR_FPMETA:
> +            ret = av_dict_set(&file->metadata, arg_kw[0], arg_str[1], AV_DICT_DONT_STRDUP_VAL);
> +            if (ret < 0)
> +                FAIL(ret);
> +            arg_str[1] = NULL;
> +            break;
>          case DIR_FPMETAS:
>              if ((ret = av_dict_parse_string(&file->metadata, arg_str[0], "=", "", 0)) < 0) {
>                  av_log(avf, AV_LOG_ERROR, "Line %d: failed to parse metadata string\n", line);
> 
You seem to misunderstand the semantics of the AV_DICT_DONT_STRDUP_*
flags: av_dict_set() takes complete ownership of the strings, even on
errors. So in case of errors the above code would lead to a double-free.
Patches #1 and #4 of this set are also affected by this.

- Andreas