[Ffmpeg-devel] [PATCH] More AAC codec IDs in matroska

[Måns Rullgård]

Steve Lhomme <slhomme at divxcorp.com> writes:

> Hi (again),
>
> Matroska has plenty of AAC codec IDs to differentiate each of
> them. This has been cleaned some time ago, but the old ones remain. So

I still see no less than 9 tags for AAC in the matroska specs on the
web...

> this patch makes sure all AAC codec IDs are handled. It's safe with
> all the other codec IDs.
>
> Steve
> Index: libavformat/matroska.c
> ===================================================================
> --- libavformat/matroska.c	(revision 6919)
> +++ libavformat/matroska.c	(working copy)
> @@ -221,6 +221,7 @@
>      {"A_VORBIS"         , CODEC_ID_VORBIS},
>      {"A_AAC/MPEG2/"     , CODEC_ID_AAC},
>      {"A_AAC/MPEG4/"     , CODEC_ID_AAC},
> +    {"A_AAC"            , CODEC_ID_AAC},
>      {"A_WAVPACK4"       , CODEC_ID_WAVPACK},
>      {NULL               , CODEC_ID_NONE}
>  /* TODO: AC3-9/10 (?), Real, Musepack, Quicktime */
> @@ -2154,7 +2155,7 @@
>                  continue;
>
>              for(j=0; codec_tags[j].str; j++){
> -                if(!strcmp(codec_tags[j].str, track->codec_id)){
> +                if(!strncmp(codec_tags[j].str, track->codec_id,strlen(codec_tags[j].str))){
>                      codec_id= codec_tags[j].id;
>                      break;
>                  }

A few comments:

Did AAC work at all previously?  With this change, are not the first
two AAC entries no longer needed?

I'd be a bit wary of changing exact string matching to prefix
matching.  It could introduce mis-detections.  Better add a third
field to the table to indicate if prefix matching is desired.
Including a trailing slash ("A_AAC/") would also reduce the risk of
something being matched by mistake.  There is currently no tag in the
spec that is a prefix of another, but I'd rather not depend on it
remaining that way.

-- 
M?ns Rullg?rd
mru at inprovide.com