diff mbox

[FFmpeg-devel,1/3] lavf: identify MP1 and MP2 as distinct containers from MP3

Message ID 20170318093805.20972-1-rodger.combs@gmail.com
State Superseded
Headers show

Commit Message

Rodger Combs March 18, 2017, 9:38 a.m. UTC
This allows us to report the correct codec ID here
---
 libavformat/allformats.c |  3 ++-
 libavformat/mp3dec.c     | 66 +++++++++++++++++++++++++++++++-----------------
 libavformat/rawenc.c     | 13 ++++++++++
 libavformat/utils.c      |  4 +--
 libavformat/version.h    |  4 +--
 5 files changed, 62 insertions(+), 28 deletions(-)

Comments

wm4 March 18, 2017, 9:52 a.m. UTC | #1
On Sat, 18 Mar 2017 04:38:03 -0500
Rodger Combs <rodger.combs@gmail.com> wrote:

> This allows us to report the correct codec ID here
> ---
>  libavformat/allformats.c |  3 ++-
>  libavformat/mp3dec.c     | 66 +++++++++++++++++++++++++++++++-----------------
>  libavformat/rawenc.c     | 13 ++++++++++
>  libavformat/utils.c      |  4 +--
>  libavformat/version.h    |  4 +--
>  5 files changed, 62 insertions(+), 28 deletions(-)
> 
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 09e62c3cfc..b9caeb3737 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -185,7 +185,8 @@ static void register_all(void)
>      REGISTER_DEMUXER (MM,               mm);
>      REGISTER_MUXDEMUX(MMF,              mmf);
>      REGISTER_MUXDEMUX(MOV,              mov);
> -    REGISTER_MUXER   (MP2,              mp2);
> +    REGISTER_MUXDEMUX(MP1,              mp1);
> +    REGISTER_MUXDEMUX(MP2,              mp2);
>      REGISTER_MUXDEMUX(MP3,              mp3);
>      REGISTER_MUXER   (MP4,              mp4);
>      REGISTER_DEMUXER (MPC,              mpc);
> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> index b45a066686..8773a55f80 100644
> --- a/libavformat/mp3dec.c
> +++ b/libavformat/mp3dec.c
> @@ -66,7 +66,7 @@ static int check(AVIOContext *pb, int64_t pos, uint32_t *header);
>  
>  /* mp3 read */
>  
> -static int mp3_read_probe(AVProbeData *p)
> +static int mpa_read_probe(AVProbeData *p, int layer)
>  {
>      int max_frames, first_frames = 0;
>      int whole_used = 0;
> @@ -89,7 +89,7 @@ static int mp3_read_probe(AVProbeData *p)
>  
>              header = AV_RB32(buf2);
>              ret = avpriv_mpegaudio_decode_header(&h, header);
> -            if (ret != 0)
> +            if (ret != 0 || h.layer != layer)
>                  break;
>              buf2 += h.frame_size;
>          }
> @@ -105,7 +105,8 @@ static int mp3_read_probe(AVProbeData *p)
>      if   (first_frames>=7) return AVPROBE_SCORE_EXTENSION + 1;
>      else if(max_frames>200)return AVPROBE_SCORE_EXTENSION;
>      else if(max_frames>=4 && max_frames >= p->buf_size/10000) return AVPROBE_SCORE_EXTENSION / 2;
> -    else if(ff_id3v2_match(buf0, ID3v2_DEFAULT_MAGIC) && 2*ff_id3v2_tag_len(buf0) >= p->buf_size)
> +    else if(ff_id3v2_match(buf0, ID3v2_DEFAULT_MAGIC) && 2*ff_id3v2_tag_len(buf0) >= p->buf_size &&
> +            (p->buf_size < PROBE_BUF_MAX || layer == 3))
>                             return p->buf_size < PROBE_BUF_MAX ? AVPROBE_SCORE_EXTENSION / 4 : AVPROBE_SCORE_EXTENSION - 2;
>      else if(first_frames > 1 && whole_used) return 5;
>      else if(max_frames>=1 && max_frames >= p->buf_size/10000) return 1;
> @@ -113,6 +114,12 @@ static int mp3_read_probe(AVProbeData *p)
>  //mpegps_mp3_unrecognized_format.mpg has max_frames=3
>  }
>  
> +#define READ_PROBE(l) \
> +static int mp##l##_read_probe(AVProbeData *p) \
> +{ \
> +    return mpa_read_probe(p, l); \
> +}
> +
>  static void read_xing_toc(AVFormatContext *s, int64_t filesize, int64_t duration)
>  {
>      int i;
> @@ -341,7 +348,7 @@ static int mp3_parse_vbr_tags(AVFormatContext *s, AVStream *st, int64_t base)
>      return 0;
>  }
>  
> -static int mp3_read_header(AVFormatContext *s)
> +static int mpa_read_header(AVFormatContext *s, enum AVCodecID id)
>  {
>      MP3DecContext *mp3 = s->priv_data;
>      AVStream *st;
> @@ -357,7 +364,7 @@ static int mp3_read_header(AVFormatContext *s)
>          return AVERROR(ENOMEM);
>  
>      st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
> -    st->codecpar->codec_id = AV_CODEC_ID_MP3;
> +    st->codecpar->codec_id = id;
>      st->need_parsing = AVSTREAM_PARSE_FULL_RAW;
>      st->start_time = 0;
>  
> @@ -422,6 +429,12 @@ static int mp3_read_header(AVFormatContext *s)
>      return 0;
>  }
>  
> +#define READ_HEADER(l) \
> +static int mp##l##_read_header(AVFormatContext *s) \
> +{ \
> +    return mpa_read_header(s, AV_CODEC_ID_MP##l); \
> +}
> +
>  #define MP3_PACKET_SIZE 1024
>  
>  static int mp3_read_packet(AVFormatContext *s, AVPacket *pkt)
> @@ -582,23 +595,30 @@ static const AVOption options[] = {
>      { NULL },
>  };
>  
> -static const AVClass demuxer_class = {
> -    .class_name = "mp3",
> -    .item_name  = av_default_item_name,
> -    .option     = options,
> -    .version    = LIBAVUTIL_VERSION_INT,
> -    .category   = AV_CLASS_CATEGORY_DEMUXER,
> +#define DECLARE_LAYER(l, ext) \
> +READ_PROBE(l) \
> +READ_HEADER(l) \
> +static const AVClass demuxer_class_##l = { \
> +    .class_name = "mp" #l, \
> +    .item_name  = av_default_item_name, \
> +    .option     = options, \
> +    .version    = LIBAVUTIL_VERSION_INT, \
> +    .category   = AV_CLASS_CATEGORY_DEMUXER, \
> +}; \
> +\
> +AVInputFormat ff_mp##l##_demuxer = { \
> +    .name           = "mp" #l, \
> +    .long_name      = NULL_IF_CONFIG_SMALL("MP" #l " (MPEG audio layer " #l ")"), \
> +    .read_probe     = mp##l##_read_probe, \
> +    .read_header    = mp##l##_read_header, \
> +    .read_packet    = mp3_read_packet, \
> +    .read_seek      = mp3_seek, \
> +    .priv_data_size = sizeof(MP3DecContext), \
> +    .flags          = AVFMT_GENERIC_INDEX, \
> +    .extensions     = ext, /* XXX: use probe */ \
> +    .priv_class     = &demuxer_class_##l, \
>  };
>  
> -AVInputFormat ff_mp3_demuxer = {
> -    .name           = "mp3",
> -    .long_name      = NULL_IF_CONFIG_SMALL("MP2/3 (MPEG audio layer 2/3)"),
> -    .read_probe     = mp3_read_probe,
> -    .read_header    = mp3_read_header,
> -    .read_packet    = mp3_read_packet,
> -    .read_seek      = mp3_seek,
> -    .priv_data_size = sizeof(MP3DecContext),
> -    .flags          = AVFMT_GENERIC_INDEX,
> -    .extensions     = "mp2,mp3,m2a,mpa", /* XXX: use probe */
> -    .priv_class     = &demuxer_class,
> -};
> +DECLARE_LAYER(1, "mp1,mpa")
> +DECLARE_LAYER(2, "mp2,m2a,mpa")
> +DECLARE_LAYER(3, "mp3,mpa")

I don't understand why you messily create 3 demuxers, instead of
limiting it to determine the correct codec ID.

> diff --git a/libavformat/rawenc.c b/libavformat/rawenc.c
> index 0edcd1cf99..4e72aa9dfd 100644
> --- a/libavformat/rawenc.c
> +++ b/libavformat/rawenc.c
> @@ -337,6 +337,19 @@ AVOutputFormat ff_mlp_muxer = {
>  };
>  #endif
>  
> +#if CONFIG_MP1_MUXER
> +AVOutputFormat ff_mp1_muxer = {
> +    .name              = "mp1",
> +    .long_name         = NULL_IF_CONFIG_SMALL("MP1 (MPEG audio layer 1)"),
> +    .mime_type         = "audio/mpeg",
> +    .extensions        = "mp1,m1a",
> +    .audio_codec       = AV_CODEC_ID_MP1,
> +    .video_codec       = AV_CODEC_ID_NONE,
> +    .write_packet      = ff_raw_write_packet,
> +    .flags             = AVFMT_NOTIMESTAMPS,
> +};
> +#endif
> +
>  #if CONFIG_MP2_MUXER
>  AVOutputFormat ff_mp2_muxer = {
>      .name              = "mp2",
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 37d7024465..84acb9c795 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -609,8 +609,8 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
>      }
>  
>      if (id3v2_extra_meta) {
> -        if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, "aac") ||
> -            !strcmp(s->iformat->name, "tta")) {
> +        if (!strcmp(s->iformat->name, "mp1") || !strcmp(s->iformat->name, "mp2") || !strcmp(s->iformat->name, "mp3") ||
> +            !strcmp(s->iformat->name, "aac") || !strcmp(s->iformat->name, "tta")) {

Jesus Christ, but I guess that's how this works.

>              if ((ret = ff_id3v2_parse_apic(s, &id3v2_extra_meta)) < 0)
>                  goto fail;
>          } else
> diff --git a/libavformat/version.h b/libavformat/version.h
> index dc689d45fb..418701e470 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -32,8 +32,8 @@
>  // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
>  // Also please add any ticket numbers that you believe might be affected here
>  #define LIBAVFORMAT_VERSION_MAJOR  57
> -#define LIBAVFORMAT_VERSION_MINOR  66
> -#define LIBAVFORMAT_VERSION_MICRO 104
> +#define LIBAVFORMAT_VERSION_MINOR  67
> +#define LIBAVFORMAT_VERSION_MICRO 100
>  
>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
>                                                 LIBAVFORMAT_VERSION_MINOR, \
Clément Bœsch March 18, 2017, 9:59 a.m. UTC | #2
On Sat, Mar 18, 2017 at 10:52:27AM +0100, wm4 wrote:
[...]
> > -        if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, "aac") ||
> > -            !strcmp(s->iformat->name, "tta")) {
> > +        if (!strcmp(s->iformat->name, "mp1") || !strcmp(s->iformat->name, "mp2") || !strcmp(s->iformat->name, "mp3") ||
> > +            !strcmp(s->iformat->name, "aac") || !strcmp(s->iformat->name, "tta")) {
> 
> Jesus Christ, but I guess that's how this works.
> 

alternative: av_match_name(s->iformat->name, "mp1,mp2,mp3,aac,tta,")
Carl Eugen Hoyos March 18, 2017, 3:51 p.m. UTC | #3
2017-03-18 10:38 GMT+01:00 Rodger Combs <rodger.combs@gmail.com>:
> This allows us to report the correct codec ID here

What does this fix?

> --- a/libavformat/rawenc.c
> +++ b/libavformat/rawenc.c
> @@ -337,6 +337,19 @@ AVOutputFormat ff_mlp_muxer = {
>  };
>  #endif
>
> +#if CONFIG_MP1_MUXER
> +AVOutputFormat ff_mp1_muxer = {
> +    .name              = "mp1",
> +    .long_name         = NULL_IF_CONFIG_SMALL("MP1 (MPEG audio layer 1)"),
> +    .mime_type         = "audio/mpeg",
> +    .extensions        = "mp1,m1a",
> +    .audio_codec       = AV_CODEC_ID_MP1,
> +    .video_codec       = AV_CODEC_ID_NONE,
> +    .write_packet      = ff_raw_write_packet,
> +    .flags             = AVFMT_NOTIMESTAMPS,
> +};
> +#endif

This looks ok but unrelated, please split.

Thank you, Carl Eugen
Michael Niedermayer March 19, 2017, 11:29 p.m. UTC | #4
On Sat, Mar 18, 2017 at 04:38:03AM -0500, Rodger Combs wrote:
> This allows us to report the correct codec ID here
> ---
>  libavformat/allformats.c |  3 ++-
>  libavformat/mp3dec.c     | 66 +++++++++++++++++++++++++++++++-----------------
>  libavformat/rawenc.c     | 13 ++++++++++
>  libavformat/utils.c      |  4 +--
>  libavformat/version.h    |  4 +--
>  5 files changed, 62 insertions(+), 28 deletions(-)

this seems to change the file from https://trac.ffmpeg.org/ticket/5689
if thats intended please add a mention of the ticket to the commit
message

thx

[...]
diff mbox

Patch

diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index 09e62c3cfc..b9caeb3737 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -185,7 +185,8 @@  static void register_all(void)
     REGISTER_DEMUXER (MM,               mm);
     REGISTER_MUXDEMUX(MMF,              mmf);
     REGISTER_MUXDEMUX(MOV,              mov);
-    REGISTER_MUXER   (MP2,              mp2);
+    REGISTER_MUXDEMUX(MP1,              mp1);
+    REGISTER_MUXDEMUX(MP2,              mp2);
     REGISTER_MUXDEMUX(MP3,              mp3);
     REGISTER_MUXER   (MP4,              mp4);
     REGISTER_DEMUXER (MPC,              mpc);
diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index b45a066686..8773a55f80 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -66,7 +66,7 @@  static int check(AVIOContext *pb, int64_t pos, uint32_t *header);
 
 /* mp3 read */
 
-static int mp3_read_probe(AVProbeData *p)
+static int mpa_read_probe(AVProbeData *p, int layer)
 {
     int max_frames, first_frames = 0;
     int whole_used = 0;
@@ -89,7 +89,7 @@  static int mp3_read_probe(AVProbeData *p)
 
             header = AV_RB32(buf2);
             ret = avpriv_mpegaudio_decode_header(&h, header);
-            if (ret != 0)
+            if (ret != 0 || h.layer != layer)
                 break;
             buf2 += h.frame_size;
         }
@@ -105,7 +105,8 @@  static int mp3_read_probe(AVProbeData *p)
     if   (first_frames>=7) return AVPROBE_SCORE_EXTENSION + 1;
     else if(max_frames>200)return AVPROBE_SCORE_EXTENSION;
     else if(max_frames>=4 && max_frames >= p->buf_size/10000) return AVPROBE_SCORE_EXTENSION / 2;
-    else if(ff_id3v2_match(buf0, ID3v2_DEFAULT_MAGIC) && 2*ff_id3v2_tag_len(buf0) >= p->buf_size)
+    else if(ff_id3v2_match(buf0, ID3v2_DEFAULT_MAGIC) && 2*ff_id3v2_tag_len(buf0) >= p->buf_size &&
+            (p->buf_size < PROBE_BUF_MAX || layer == 3))
                            return p->buf_size < PROBE_BUF_MAX ? AVPROBE_SCORE_EXTENSION / 4 : AVPROBE_SCORE_EXTENSION - 2;
     else if(first_frames > 1 && whole_used) return 5;
     else if(max_frames>=1 && max_frames >= p->buf_size/10000) return 1;
@@ -113,6 +114,12 @@  static int mp3_read_probe(AVProbeData *p)
 //mpegps_mp3_unrecognized_format.mpg has max_frames=3
 }
 
+#define READ_PROBE(l) \
+static int mp##l##_read_probe(AVProbeData *p) \
+{ \
+    return mpa_read_probe(p, l); \
+}
+
 static void read_xing_toc(AVFormatContext *s, int64_t filesize, int64_t duration)
 {
     int i;
@@ -341,7 +348,7 @@  static int mp3_parse_vbr_tags(AVFormatContext *s, AVStream *st, int64_t base)
     return 0;
 }
 
-static int mp3_read_header(AVFormatContext *s)
+static int mpa_read_header(AVFormatContext *s, enum AVCodecID id)
 {
     MP3DecContext *mp3 = s->priv_data;
     AVStream *st;
@@ -357,7 +364,7 @@  static int mp3_read_header(AVFormatContext *s)
         return AVERROR(ENOMEM);
 
     st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
-    st->codecpar->codec_id = AV_CODEC_ID_MP3;
+    st->codecpar->codec_id = id;
     st->need_parsing = AVSTREAM_PARSE_FULL_RAW;
     st->start_time = 0;
 
@@ -422,6 +429,12 @@  static int mp3_read_header(AVFormatContext *s)
     return 0;
 }
 
+#define READ_HEADER(l) \
+static int mp##l##_read_header(AVFormatContext *s) \
+{ \
+    return mpa_read_header(s, AV_CODEC_ID_MP##l); \
+}
+
 #define MP3_PACKET_SIZE 1024
 
 static int mp3_read_packet(AVFormatContext *s, AVPacket *pkt)
@@ -582,23 +595,30 @@  static const AVOption options[] = {
     { NULL },
 };
 
-static const AVClass demuxer_class = {
-    .class_name = "mp3",
-    .item_name  = av_default_item_name,
-    .option     = options,
-    .version    = LIBAVUTIL_VERSION_INT,
-    .category   = AV_CLASS_CATEGORY_DEMUXER,
+#define DECLARE_LAYER(l, ext) \
+READ_PROBE(l) \
+READ_HEADER(l) \
+static const AVClass demuxer_class_##l = { \
+    .class_name = "mp" #l, \
+    .item_name  = av_default_item_name, \
+    .option     = options, \
+    .version    = LIBAVUTIL_VERSION_INT, \
+    .category   = AV_CLASS_CATEGORY_DEMUXER, \
+}; \
+\
+AVInputFormat ff_mp##l##_demuxer = { \
+    .name           = "mp" #l, \
+    .long_name      = NULL_IF_CONFIG_SMALL("MP" #l " (MPEG audio layer " #l ")"), \
+    .read_probe     = mp##l##_read_probe, \
+    .read_header    = mp##l##_read_header, \
+    .read_packet    = mp3_read_packet, \
+    .read_seek      = mp3_seek, \
+    .priv_data_size = sizeof(MP3DecContext), \
+    .flags          = AVFMT_GENERIC_INDEX, \
+    .extensions     = ext, /* XXX: use probe */ \
+    .priv_class     = &demuxer_class_##l, \
 };
 
-AVInputFormat ff_mp3_demuxer = {
-    .name           = "mp3",
-    .long_name      = NULL_IF_CONFIG_SMALL("MP2/3 (MPEG audio layer 2/3)"),
-    .read_probe     = mp3_read_probe,
-    .read_header    = mp3_read_header,
-    .read_packet    = mp3_read_packet,
-    .read_seek      = mp3_seek,
-    .priv_data_size = sizeof(MP3DecContext),
-    .flags          = AVFMT_GENERIC_INDEX,
-    .extensions     = "mp2,mp3,m2a,mpa", /* XXX: use probe */
-    .priv_class     = &demuxer_class,
-};
+DECLARE_LAYER(1, "mp1,mpa")
+DECLARE_LAYER(2, "mp2,m2a,mpa")
+DECLARE_LAYER(3, "mp3,mpa")
diff --git a/libavformat/rawenc.c b/libavformat/rawenc.c
index 0edcd1cf99..4e72aa9dfd 100644
--- a/libavformat/rawenc.c
+++ b/libavformat/rawenc.c
@@ -337,6 +337,19 @@  AVOutputFormat ff_mlp_muxer = {
 };
 #endif
 
+#if CONFIG_MP1_MUXER
+AVOutputFormat ff_mp1_muxer = {
+    .name              = "mp1",
+    .long_name         = NULL_IF_CONFIG_SMALL("MP1 (MPEG audio layer 1)"),
+    .mime_type         = "audio/mpeg",
+    .extensions        = "mp1,m1a",
+    .audio_codec       = AV_CODEC_ID_MP1,
+    .video_codec       = AV_CODEC_ID_NONE,
+    .write_packet      = ff_raw_write_packet,
+    .flags             = AVFMT_NOTIMESTAMPS,
+};
+#endif
+
 #if CONFIG_MP2_MUXER
 AVOutputFormat ff_mp2_muxer = {
     .name              = "mp2",
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 37d7024465..84acb9c795 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -609,8 +609,8 @@  int avformat_open_input(AVFormatContext **ps, const char *filename,
     }
 
     if (id3v2_extra_meta) {
-        if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, "aac") ||
-            !strcmp(s->iformat->name, "tta")) {
+        if (!strcmp(s->iformat->name, "mp1") || !strcmp(s->iformat->name, "mp2") || !strcmp(s->iformat->name, "mp3") ||
+            !strcmp(s->iformat->name, "aac") || !strcmp(s->iformat->name, "tta")) {
             if ((ret = ff_id3v2_parse_apic(s, &id3v2_extra_meta)) < 0)
                 goto fail;
         } else
diff --git a/libavformat/version.h b/libavformat/version.h
index dc689d45fb..418701e470 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,8 +32,8 @@ 
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  57
-#define LIBAVFORMAT_VERSION_MINOR  66
-#define LIBAVFORMAT_VERSION_MICRO 104
+#define LIBAVFORMAT_VERSION_MINOR  67
+#define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
                                                LIBAVFORMAT_VERSION_MINOR, \