[1/5] movenc: use correct tag list for AVOutputFormat.codec_tag

Message ID 20170619150209.27009-1-stebbins@jetheaddev.com
State New
Headers show

Commit Message

John Stebbins June 19, 2017, 3:02 p.m.
ff_mp4_obj_type contains the wrong type of tags for
AVOutputFormat.codec_tag. AVOutputFormat.codec_tag is used to
validate AVCodecParameters.codec_tag so needs to be the same
type of tag.

Creates new tag lists for mp4 and ismv.  New tag lists support
same list of codecs found in ff_mp4_obj_type. psp uses the same
tag list as mp4 since these both use mp4_get_codec_tag to look up tags.
---
 libavformat/movenc.c | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

Comments

Hendrik Leppkes June 19, 2017, 3:06 p.m. | #1
On Mon, Jun 19, 2017 at 5:02 PM, John Stebbins <stebbins@jetheaddev.com> wrote:
> ff_mp4_obj_type contains the wrong type of tags for
> AVOutputFormat.codec_tag. AVOutputFormat.codec_tag is used to
> validate AVCodecParameters.codec_tag so needs to be the same
> type of tag.
>
> Creates new tag lists for mp4 and ismv.  New tag lists support
> same list of codecs found in ff_mp4_obj_type. psp uses the same
> tag list as mp4 since these both use mp4_get_codec_tag to look up tags.
> ---
>  libavformat/movenc.c | 37 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index e389029..6c3d36c 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -4444,6 +4444,36 @@ error:
>      return res;
>  }
>
> +const AVCodecTag codec_mp4_tags[] = {
> +    { AV_CODEC_ID_MOV_TEXT    , MKTAG('t', 'x', '3', 'g') },
> +    { AV_CODEC_ID_MPEG4       , MKTAG('m', 'p', '4', 'v') },
> +    { AV_CODEC_ID_H264        , MKTAG('a', 'v', 'c', '1') },
> +    { AV_CODEC_ID_HEVC        , MKTAG('h', 'e', 'v', '1') },
> +    { AV_CODEC_ID_AAC         , MKTAG('m', 'p', '4', 'a') },
> +    { AV_CODEC_ID_MP4ALS      , MKTAG('m', 'p', '4', 'a') },
> +    { AV_CODEC_ID_MPEG2VIDEO  , MKTAG('m', 'p', '4', 'v') },
> +    { AV_CODEC_ID_MP3         , MKTAG('m', 'p', '4', 'a') },
> +    { AV_CODEC_ID_MP2         , MKTAG('m', 'p', '4', 'a') },
> +    { AV_CODEC_ID_MPEG1VIDEO  , MKTAG('m', 'p', '4', 'v') },
> +    { AV_CODEC_ID_MJPEG       , MKTAG('m', 'p', '4', 'v') },
> +    { AV_CODEC_ID_PNG         , MKTAG('m', 'p', '4', 'v') },
> +    { AV_CODEC_ID_JPEG2000    , MKTAG('m', 'p', '4', 'v') },
> +    { AV_CODEC_ID_VC1         , MKTAG('v', 'c', '-', '1') },
> +    { AV_CODEC_ID_DIRAC       , MKTAG('d', 'r', 'a', 'c') },
> +    { AV_CODEC_ID_AC3         , MKTAG('a', 'c', '-', '3') },
> +    { AV_CODEC_ID_DTS         , MKTAG('m', 'p', '4', 'a') },
> +    { AV_CODEC_ID_TSCC2       , MKTAG('m', 'p', '4', 'v') },
> +    { AV_CODEC_ID_VORBIS      , MKTAG('m', 'p', '4', 'a') },
> +    { AV_CODEC_ID_DVD_SUBTITLE, MKTAG('m', 'p', '4', 's') },
> +    { AV_CODEC_ID_QCELP       , MKTAG('m', 'p', '4', 'a') },
> +    { AV_CODEC_ID_NONE        ,    0 },
> +};

I'm not sure if anyone mentioned any other order of codecs or whatnot,
but grouping them by codec type would greatly improve
understandability of such a list, imho (video -> audio -> subtitle)

> +
> +const AVCodecTag codec_ism_tags[] = {
> +    { AV_CODEC_ID_WMAPRO      , MKTAG('w', 'm', 'a', ' ') },
> +    { AV_CODEC_ID_NONE        ,    0 },
> +};
> +
>  #if CONFIG_MOV_MUXER
>  MOV_CLASS(mov)
>  AVOutputFormat ff_mov_muxer = {
> @@ -4496,7 +4526,7 @@ AVOutputFormat ff_mp4_muxer = {
>      .write_packet      = mov_write_packet,
>      .write_trailer     = mov_write_trailer,
>      .flags             = AVFMT_GLOBALHEADER | AVFMT_ALLOW_FLUSH | AVFMT_TS_NEGATIVE,
> -    .codec_tag         = (const AVCodecTag* const []){ ff_mp4_obj_type, 0 },
> +    .codec_tag         = (const AVCodecTag* const []){ codec_mp4_tags, 0 },
>      .priv_class        = &mp4_muxer_class,
>  };
>  #endif
> @@ -4514,7 +4544,7 @@ AVOutputFormat ff_psp_muxer = {
>      .write_packet      = mov_write_packet,
>      .write_trailer     = mov_write_trailer,
>      .flags             = AVFMT_GLOBALHEADER | AVFMT_ALLOW_FLUSH | AVFMT_TS_NEGATIVE,
> -    .codec_tag         = (const AVCodecTag* const []){ ff_mp4_obj_type, 0 },
> +    .codec_tag         = (const AVCodecTag* const []){ codec_mp4_tags, 0 },
>      .priv_class        = &psp_muxer_class,
>  };
>  #endif
> @@ -4567,7 +4597,8 @@ AVOutputFormat ff_ismv_muxer = {
>      .write_packet      = mov_write_packet,
>      .write_trailer     = mov_write_trailer,
>      .flags             = AVFMT_GLOBALHEADER | AVFMT_ALLOW_FLUSH | AVFMT_TS_NEGATIVE,
> -    .codec_tag         = (const AVCodecTag* const []){ ff_mp4_obj_type, 0 },
> +    .codec_tag         = (const AVCodecTag* const []){
> +        codec_mp4_tags, codec_ism_tags, 0 },
>      .priv_class        = &ismv_muxer_class,
>  };
>  #endif
> --
> 2.9.4
>
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
John Stebbins June 19, 2017, 3:11 p.m. | #2
On 06/19/2017 08:06 AM, Hendrik Leppkes wrote:
> On Mon, Jun 19, 2017 at 5:02 PM, John Stebbins <stebbins@jetheaddev.com> wrote:
>> ff_mp4_obj_type contains the wrong type of tags for
>> AVOutputFormat.codec_tag. AVOutputFormat.codec_tag is used to
>> validate AVCodecParameters.codec_tag so needs to be the same
>> type of tag.
>>
>> Creates new tag lists for mp4 and ismv.  New tag lists support
>> same list of codecs found in ff_mp4_obj_type. psp uses the same
>> tag list as mp4 since these both use mp4_get_codec_tag to look up tags.
>> ---
>>  libavformat/movenc.c | 37 ++++++++++++++++++++++++++++++++++---
>>  1 file changed, 34 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> index e389029..6c3d36c 100644
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -4444,6 +4444,36 @@ error:
>>      return res;
>>  }
>>
>> +const AVCodecTag codec_mp4_tags[] = {
>> +    { AV_CODEC_ID_MOV_TEXT    , MKTAG('t', 'x', '3', 'g') },
>> +    { AV_CODEC_ID_MPEG4       , MKTAG('m', 'p', '4', 'v') },
>> +    { AV_CODEC_ID_H264        , MKTAG('a', 'v', 'c', '1') },
>> +    { AV_CODEC_ID_HEVC        , MKTAG('h', 'e', 'v', '1') },
>> +    { AV_CODEC_ID_AAC         , MKTAG('m', 'p', '4', 'a') },
>> +    { AV_CODEC_ID_MP4ALS      , MKTAG('m', 'p', '4', 'a') },
>> +    { AV_CODEC_ID_MPEG2VIDEO  , MKTAG('m', 'p', '4', 'v') },
>> +    { AV_CODEC_ID_MP3         , MKTAG('m', 'p', '4', 'a') },
>> +    { AV_CODEC_ID_MP2         , MKTAG('m', 'p', '4', 'a') },
>> +    { AV_CODEC_ID_MPEG1VIDEO  , MKTAG('m', 'p', '4', 'v') },
>> +    { AV_CODEC_ID_MJPEG       , MKTAG('m', 'p', '4', 'v') },
>> +    { AV_CODEC_ID_PNG         , MKTAG('m', 'p', '4', 'v') },
>> +    { AV_CODEC_ID_JPEG2000    , MKTAG('m', 'p', '4', 'v') },
>> +    { AV_CODEC_ID_VC1         , MKTAG('v', 'c', '-', '1') },
>> +    { AV_CODEC_ID_DIRAC       , MKTAG('d', 'r', 'a', 'c') },
>> +    { AV_CODEC_ID_AC3         , MKTAG('a', 'c', '-', '3') },
>> +    { AV_CODEC_ID_DTS         , MKTAG('m', 'p', '4', 'a') },
>> +    { AV_CODEC_ID_TSCC2       , MKTAG('m', 'p', '4', 'v') },
>> +    { AV_CODEC_ID_VORBIS      , MKTAG('m', 'p', '4', 'a') },
>> +    { AV_CODEC_ID_DVD_SUBTITLE, MKTAG('m', 'p', '4', 's') },
>> +    { AV_CODEC_ID_QCELP       , MKTAG('m', 'p', '4', 'a') },
>> +    { AV_CODEC_ID_NONE        ,    0 },
>> +};
> I'm not sure if anyone mentioned any other order of codecs or whatnot,
> but grouping them by codec type would greatly improve
> understandability of such a list, imho (video -> audio -> subtitle)

No one has mentioned that yet ;)  The current order is the same as the ff_mp4_obj_type table.  I basically copied that
table and changed tag values.  If nobody disapproves, I'll change the order to group by type.

>> +
>> +const AVCodecTag codec_ism_tags[] = {
>> +    { AV_CODEC_ID_WMAPRO      , MKTAG('w', 'm', 'a', ' ') },
>> +    { AV_CODEC_ID_NONE        ,    0 },
>> +};
>> +
>>  #if CONFIG_MOV_MUXER
>>  MOV_CLASS(mov)
>>  AVOutputFormat ff_mov_muxer = {
>> @@ -4496,7 +4526,7 @@ AVOutputFormat ff_mp4_muxer = {
>>      .write_packet      = mov_write_packet,
>>      .write_trailer     = mov_write_trailer,
>>      .flags             = AVFMT_GLOBALHEADER | AVFMT_ALLOW_FLUSH | AVFMT_TS_NEGATIVE,
>> -    .codec_tag         = (const AVCodecTag* const []){ ff_mp4_obj_type, 0 },
>> +    .codec_tag         = (const AVCodecTag* const []){ codec_mp4_tags, 0 },
>>      .priv_class        = &mp4_muxer_class,
>>  };
>>  #endif
>> @@ -4514,7 +4544,7 @@ AVOutputFormat ff_psp_muxer = {
>>      .write_packet      = mov_write_packet,
>>      .write_trailer     = mov_write_trailer,
>>      .flags             = AVFMT_GLOBALHEADER | AVFMT_ALLOW_FLUSH | AVFMT_TS_NEGATIVE,
>> -    .codec_tag         = (const AVCodecTag* const []){ ff_mp4_obj_type, 0 },
>> +    .codec_tag         = (const AVCodecTag* const []){ codec_mp4_tags, 0 },
>>      .priv_class        = &psp_muxer_class,
>>  };
>>  #endif
>> @@ -4567,7 +4597,8 @@ AVOutputFormat ff_ismv_muxer = {
>>      .write_packet      = mov_write_packet,
>>      .write_trailer     = mov_write_trailer,
>>      .flags             = AVFMT_GLOBALHEADER | AVFMT_ALLOW_FLUSH | AVFMT_TS_NEGATIVE,
>> -    .codec_tag         = (const AVCodecTag* const []){ ff_mp4_obj_type, 0 },
>> +    .codec_tag         = (const AVCodecTag* const []){
>> +        codec_mp4_tags, codec_ism_tags, 0 },
>>      .priv_class        = &ismv_muxer_class,
>>  };
>>  #endif
>> --
>> 2.9.4
>>
>> _______________________________________________
>> libav-devel mailing list
>> libav-devel@libav.org
>> https://lists.libav.org/mailman/listinfo/libav-devel
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
Martin Storsjö June 20, 2017, 5:59 a.m. | #3
On Mon, 19 Jun 2017, John Stebbins wrote:

> ff_mp4_obj_type contains the wrong type of tags for
> AVOutputFormat.codec_tag. AVOutputFormat.codec_tag is used to
> validate AVCodecParameters.codec_tag so needs to be the same
> type of tag.
>
> Creates new tag lists for mp4 and ismv.  New tag lists support
> same list of codecs found in ff_mp4_obj_type. psp uses the same
> tag list as mp4 since these both use mp4_get_codec_tag to look up tags.
> ---
> libavformat/movenc.c | 37 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 34 insertions(+), 3 deletions(-)

Seems ok to me now

// Martin

Patch

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index e389029..6c3d36c 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -4444,6 +4444,36 @@  error:
     return res;
 }
 
+const AVCodecTag codec_mp4_tags[] = {
+    { AV_CODEC_ID_MOV_TEXT    , MKTAG('t', 'x', '3', 'g') },
+    { AV_CODEC_ID_MPEG4       , MKTAG('m', 'p', '4', 'v') },
+    { AV_CODEC_ID_H264        , MKTAG('a', 'v', 'c', '1') },
+    { AV_CODEC_ID_HEVC        , MKTAG('h', 'e', 'v', '1') },
+    { AV_CODEC_ID_AAC         , MKTAG('m', 'p', '4', 'a') },
+    { AV_CODEC_ID_MP4ALS      , MKTAG('m', 'p', '4', 'a') },
+    { AV_CODEC_ID_MPEG2VIDEO  , MKTAG('m', 'p', '4', 'v') },
+    { AV_CODEC_ID_MP3         , MKTAG('m', 'p', '4', 'a') },
+    { AV_CODEC_ID_MP2         , MKTAG('m', 'p', '4', 'a') },
+    { AV_CODEC_ID_MPEG1VIDEO  , MKTAG('m', 'p', '4', 'v') },
+    { AV_CODEC_ID_MJPEG       , MKTAG('m', 'p', '4', 'v') },
+    { AV_CODEC_ID_PNG         , MKTAG('m', 'p', '4', 'v') },
+    { AV_CODEC_ID_JPEG2000    , MKTAG('m', 'p', '4', 'v') },
+    { AV_CODEC_ID_VC1         , MKTAG('v', 'c', '-', '1') },
+    { AV_CODEC_ID_DIRAC       , MKTAG('d', 'r', 'a', 'c') },
+    { AV_CODEC_ID_AC3         , MKTAG('a', 'c', '-', '3') },
+    { AV_CODEC_ID_DTS         , MKTAG('m', 'p', '4', 'a') },
+    { AV_CODEC_ID_TSCC2       , MKTAG('m', 'p', '4', 'v') },
+    { AV_CODEC_ID_VORBIS      , MKTAG('m', 'p', '4', 'a') },
+    { AV_CODEC_ID_DVD_SUBTITLE, MKTAG('m', 'p', '4', 's') },
+    { AV_CODEC_ID_QCELP       , MKTAG('m', 'p', '4', 'a') },
+    { AV_CODEC_ID_NONE        ,    0 },
+};
+
+const AVCodecTag codec_ism_tags[] = {
+    { AV_CODEC_ID_WMAPRO      , MKTAG('w', 'm', 'a', ' ') },
+    { AV_CODEC_ID_NONE        ,    0 },
+};
+
 #if CONFIG_MOV_MUXER
 MOV_CLASS(mov)
 AVOutputFormat ff_mov_muxer = {
@@ -4496,7 +4526,7 @@  AVOutputFormat ff_mp4_muxer = {
     .write_packet      = mov_write_packet,
     .write_trailer     = mov_write_trailer,
     .flags             = AVFMT_GLOBALHEADER | AVFMT_ALLOW_FLUSH | AVFMT_TS_NEGATIVE,
-    .codec_tag         = (const AVCodecTag* const []){ ff_mp4_obj_type, 0 },
+    .codec_tag         = (const AVCodecTag* const []){ codec_mp4_tags, 0 },
     .priv_class        = &mp4_muxer_class,
 };
 #endif
@@ -4514,7 +4544,7 @@  AVOutputFormat ff_psp_muxer = {
     .write_packet      = mov_write_packet,
     .write_trailer     = mov_write_trailer,
     .flags             = AVFMT_GLOBALHEADER | AVFMT_ALLOW_FLUSH | AVFMT_TS_NEGATIVE,
-    .codec_tag         = (const AVCodecTag* const []){ ff_mp4_obj_type, 0 },
+    .codec_tag         = (const AVCodecTag* const []){ codec_mp4_tags, 0 },
     .priv_class        = &psp_muxer_class,
 };
 #endif
@@ -4567,7 +4597,8 @@  AVOutputFormat ff_ismv_muxer = {
     .write_packet      = mov_write_packet,
     .write_trailer     = mov_write_trailer,
     .flags             = AVFMT_GLOBALHEADER | AVFMT_ALLOW_FLUSH | AVFMT_TS_NEGATIVE,
-    .codec_tag         = (const AVCodecTag* const []){ ff_mp4_obj_type, 0 },
+    .codec_tag         = (const AVCodecTag* const []){
+        codec_mp4_tags, codec_ism_tags, 0 },
     .priv_class        = &ismv_muxer_class,
 };
 #endif