[FFmpeg-devel] [PATCH] Add TargetTypeValue in Matroska tag prefix

Nicolas George george at nsup.org
Thu Jan 28 19:17:07 CET 2016


L'octidi 8 pluviôse, an CCXXIV, Pierre Choffet a écrit :
> Previously, the Matroska tag names were structured like this:
>   [<TargetType>/]<TagName>
> This lead to an issue when <TargetType> is not available: the different levels tags overwrite each
> other when they have the same name.
> 
> This patch transforms the name prefix into:
>   <TargetTypeValue>[-<TargetType>]/<TagName>
> 
> As the TargetTypeValue has default value in case it has no data, it prevents from overriding of other levels
> tags.
> ---
>  libavformat/matroskadec.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index d788232..ba3c4e1 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1460,8 +1460,14 @@ static void matroska_convert_tags(AVFormatContext *s)
>                         i, tags[i].target.trackuid);
>              }
>          } else {

> +            char prefix[1024];
> +            sprintf(prefix, "%" PRIu64, tags[i].target.typevalue);
> +            if (tags[i].target.type) {
> +                strncat(prefix, "-", sizeof(char));
> +                av_strlcat(prefix, tags[i].target.type, strlen(tags[i].target.type) - 22);
> +            }

This looks brittle (magic constant), the strncat() use very strange, and the
av_strlcat() call completely invalid.

I suggest you use a single snprintf() for each case:

    if (tags[i].target.type)
        snprintf(prefix, sizeof(prefix), "%"PRIu64"-%s",
                 tags[i].target.typevalue, tags[i].target.type);
    else
        snprintf(prefix, sizeof(prefix), "%"PRIu64,
                 tags[i].target.typevalue);


Also note that this silently truncates typevalue if it is too long. This may
or may not be acceptable, depending on the use case.

>              matroska_convert_tag(s, &tags[i].tag, &s->metadata,
> -                                 tags[i].target.type);
> +                                 prefix);
>          }
>      }
>  }

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160128/85b9e430/attachment.sig>


More information about the ffmpeg-devel mailing list