diff mbox series

[FFmpeg-devel,1/5] API: add AV_PKT_DATA_MPEGTS_DESC_6A to AVPacketSideDataType

Message ID 1595948614-10861-1-git-send-email-lance.lmwang@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/5] API: add AV_PKT_DATA_MPEGTS_DESC_6A to AVPacketSideDataType | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Lance Wang July 28, 2020, 3:03 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 doc/APIchanges        | 3 +++
 libavcodec/avpacket.c | 1 +
 libavcodec/packet.h   | 7 +++++++
 libavcodec/version.h  | 2 +-
 4 files changed, 12 insertions(+), 1 deletion(-)

Comments

Andreas Rheinhardt July 28, 2020, 3:09 p.m. UTC | #1 
lance.lmwang@gmail.com:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  doc/APIchanges        | 3 +++
>  libavcodec/avpacket.c | 1 +
>  libavcodec/packet.h   | 7 +++++++
>  libavcodec/version.h  | 2 +-
>  4 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 72a4833..2c76fdc 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil:     2017-10-21
>  
>  API changes, most recent first:
>  
> +2020-07-xx - xxxxxxxxxx - lavc 58.98.102 - packet.h
> +  Add AV_PKT_DATA_MPEGTS_DESC_6A.
> +
>  2020-07-xx - xxxxxxxxxx - lavu 56.57.100 - cpu.h
>    Add AV_CPU_FLAG_MMI and AV_CPU_FLAG_MSA.
>  
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 4801163..0e88112 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -401,6 +401,7 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type)
>      case AV_PKT_DATA_ICC_PROFILE:                return "ICC Profile";
>      case AV_PKT_DATA_DOVI_CONF:                  return "DOVI configuration record";
>      case AV_PKT_DATA_S12M_TIMECODE:              return "SMPTE ST 12-1:2014 timecode";
> +    case AV_PKT_DATA_MPEGTS_DESC_6A:             return "ETSI 300 468 descriptor 0x6A(AC-3)";
>      }
>      return NULL;
>  }
> diff --git a/libavcodec/packet.h b/libavcodec/packet.h
> index 0a19a0e..a18824a 100644
> --- a/libavcodec/packet.h
> +++ b/libavcodec/packet.h
> @@ -291,6 +291,13 @@ enum AVPacketSideDataType {
>      AV_PKT_DATA_S12M_TIMECODE,
>  
>      /**
> +     * ETSI 300 468 descriptor 0x6A(AC-3)
> +     * Refer to: ETSI EN 300 468 V1.11.1 (2010-04) (SI in DVB systems)
> +     * Tags are stored in struct AVDescriptor6A.

I don't know this struct; it has to be added before the commit adding
the new side data.

> +     */
> +    AV_PKT_DATA_MPEGTS_DESC_6A,
> +
> +    /**
>       * The number of side data types.
>       * This is not part of the public API/ABI in the sense that it may
>       * change when new side data types are added.
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 291e6b5..4750e25 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -28,7 +28,7 @@
>  #include "libavutil/version.h"
>  
>  #define LIBAVCODEC_VERSION_MAJOR  58
> -#define LIBAVCODEC_VERSION_MINOR  97
> +#define LIBAVCODEC_VERSION_MINOR  98
>  #define LIBAVCODEC_VERSION_MICRO 102
>  
You need to reset micro when updating minor.

>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>
Derek Buitenhuis July 28, 2020, 4:40 p.m. UTC | #2 
On 28/07/2020 16:03, lance.lmwang@gmail.com wrote:
> +    case AV_PKT_DATA_MPEGTS_DESC_6A:             return "ETSI 300 468 descriptor 0x6A(AC-3)";

Is there not anything more... descriptive it could be called,
rather than simply the hex literal it uses... ?

- Derek
Kieran Kunhya July 28, 2020, 4:48 p.m. UTC | #3 
On Tue, 28 Jul 2020 at 16:04, <lance.lmwang@gmail.com> wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  doc/APIchanges        | 3 +++
>  libavcodec/avpacket.c | 1 +
>  libavcodec/packet.h   | 7 +++++++
>  libavcodec/version.h  | 2 +-
>  4 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 72a4833..2c76fdc 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil:     2017-10-21
>
>  API changes, most recent first:
>
> +2020-07-xx - xxxxxxxxxx - lavc 58.98.102 - packet.h
> +  Add AV_PKT_DATA_MPEGTS_DESC_6A.
> +
>  2020-07-xx - xxxxxxxxxx - lavu 56.57.100 - cpu.h
>    Add AV_CPU_FLAG_MMI and AV_CPU_FLAG_MSA.
>
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 4801163..0e88112 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -401,6 +401,7 @@ const char *av_packet_side_data_name(enum
> AVPacketSideDataType type)
>      case AV_PKT_DATA_ICC_PROFILE:                return "ICC Profile";
>      case AV_PKT_DATA_DOVI_CONF:                  return "DOVI
> configuration record";
>      case AV_PKT_DATA_S12M_TIMECODE:              return "SMPTE ST
> 12-1:2014 timecode";
> +    case AV_PKT_DATA_MPEGTS_DESC_6A:             return "ETSI 300 468
> descriptor 0x6A(AC-3)";
>

This ancillary data is not associated with the packet. It's a descriptor
which is not the same thing.

Kieran
Andreas Rheinhardt July 28, 2020, 4:55 p.m. UTC | #4 
lance.lmwang@gmail.com:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavformat/mpegts.c    | 49 +++++++++++++++++++++++++++++++++++++++++++++----
>  tests/ref/fate/ts-demux |  2 +-
>  2 files changed, 46 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index c6fd3e1..b5ea5a1 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -29,6 +29,7 @@
>  #include "libavutil/opt.h"
>  #include "libavutil/avassert.h"
>  #include "libavutil/dovi_meta.h"
> +#include "libavutil/mpegts_audio_desc_metadata.h"
>  #include "libavcodec/bytestream.h"
>  #include "libavcodec/get_bits.h"
>  #include "libavcodec/opus.h"
> @@ -2073,16 +2074,56 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
>          break;
>      case 0x6a: /* ac-3_descriptor */
>          {
> -            int component_type_flag = get8(pp, desc_end) & (1 << 7);
> -            if (component_type_flag) {
> -                int component_type = get8(pp, desc_end);
> +            int     ret;
> +            uint8_t buf;
> +            size_t  desc6a_size;
> +            AVDescriptor6A *desc6a;
> +
> +            if (desc_end - *pp < 1)
> +                return AVERROR_INVALIDDATA;
> +
> +            desc6a = av_desc6a_alloc(&desc6a_size);
> +            if (!desc6a)
> +                return AVERROR(ENOMEM);
> +            buf = get8(pp, desc_end);
> +            desc6a->component_type_flag = (buf >> 7) & 0x1;
> +            desc6a->bsid_flag           = (buf >> 6) & 0x1;
> +            desc6a->mainid_flag         = (buf >> 5) & 0x1;
> +            desc6a->asvc_flag           = (buf >> 4) & 0x1;
> +            if (desc6a->component_type_flag) {
>                  int service_type_mask = 0x38;  // 0b00111000
> -                int service_type = ((component_type & service_type_mask) >> 3);
> +                int service_type;
> +
> +                if (desc_end - *pp < 1)
> +                    return AVERROR_INVALIDDATA;
> +                desc6a->component_type = get8(pp, desc_end);
> +                service_type = ((desc6a->component_type & service_type_mask) >> 3);
>                  if (service_type == 0x02 /* 0b010 */) {
>                      st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
>                      av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, "New track disposition for id %u: %u\n", st->id, st->disposition);
>                  }
>              }
> +            if (desc6a->bsid_flag) {
> +                if (desc_end - *pp < 1)
> +                    return AVERROR_INVALIDDATA;
> +                desc6a->bsid = get8(pp, desc_end);
> +            }
> +            if (desc6a->mainid_flag) {
> +                if (desc_end - *pp < 1)
> +                    return AVERROR_INVALIDDATA;
> +                desc6a->mainid = get8(pp, desc_end);
> +            }
> +            if (desc6a->asvc_flag) {
> +                if (desc_end - *pp < 1)
> +                    return AVERROR_INVALIDDATA;

desc6a leaks in each of these error paths.

> +                desc6a->asvc_flag = get8(pp, desc_end);
> +            }
> +            ret = av_stream_add_side_data(st, AV_PKT_DATA_MPEGTS_DESC_6A,
> +                                         (uint8_t *)desc6a, desc6a_size);
> +            if (ret < 0) {
> +                av_free(desc6a);
> +                return ret;
> +            }
>          }
>          break;
>      case 0x7a: /* enhanced_ac-3_descriptor */
> diff --git a/tests/ref/fate/ts-demux b/tests/ref/fate/ts-demux
> index cdf34d6..dfe0374 100644
> --- a/tests/ref/fate/ts-demux
> +++ b/tests/ref/fate/ts-demux
> @@ -10,7 +10,7 @@
>  #sample_rate 1: 48000
>  #channel_layout 1: 60f
>  #channel_layout_name 1: 5.1(side)
> -1,          0,          0,     2880,     1536, 0x773ffeea, S=1,        1, 0x00bd00bd
> +1,          0,          0,     2880,     1536, 0x773ffeea, S=2,        1, 0x00bd00bd,        9, 0x00000000
>  1,       2880,       2880,     2880,     1536, 0x6dc10748
>  1,       5760,       5760,     2880,     1536, 0xbab5129c
>  1,       8640,       8640,     2880,     1536, 0x602f034b, S=1,        1, 0x00bd00bd
>
Lance Wang July 29, 2020, 12:13 a.m. UTC | #5 
On Tue, Jul 28, 2020 at 06:55:14PM +0200, Andreas Rheinhardt wrote:
> lance.lmwang@gmail.com:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavformat/mpegts.c    | 49 +++++++++++++++++++++++++++++++++++++++++++++----
> >  tests/ref/fate/ts-demux |  2 +-
> >  2 files changed, 46 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> > index c6fd3e1..b5ea5a1 100644
> > --- a/libavformat/mpegts.c
> > +++ b/libavformat/mpegts.c
> > @@ -29,6 +29,7 @@
> >  #include "libavutil/opt.h"
> >  #include "libavutil/avassert.h"
> >  #include "libavutil/dovi_meta.h"
> > +#include "libavutil/mpegts_audio_desc_metadata.h"
> >  #include "libavcodec/bytestream.h"
> >  #include "libavcodec/get_bits.h"
> >  #include "libavcodec/opus.h"
> > @@ -2073,16 +2074,56 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
> >          break;
> >      case 0x6a: /* ac-3_descriptor */
> >          {
> > -            int component_type_flag = get8(pp, desc_end) & (1 << 7);
> > -            if (component_type_flag) {
> > -                int component_type = get8(pp, desc_end);
> > +            int     ret;
> > +            uint8_t buf;
> > +            size_t  desc6a_size;
> > +            AVDescriptor6A *desc6a;
> > +
> > +            if (desc_end - *pp < 1)
> > +                return AVERROR_INVALIDDATA;
> > +
> > +            desc6a = av_desc6a_alloc(&desc6a_size);
> > +            if (!desc6a)
> > +                return AVERROR(ENOMEM);
> > +            buf = get8(pp, desc_end);
> > +            desc6a->component_type_flag = (buf >> 7) & 0x1;
> > +            desc6a->bsid_flag           = (buf >> 6) & 0x1;
> > +            desc6a->mainid_flag         = (buf >> 5) & 0x1;
> > +            desc6a->asvc_flag           = (buf >> 4) & 0x1;
> > +            if (desc6a->component_type_flag) {
> >                  int service_type_mask = 0x38;  // 0b00111000
> > -                int service_type = ((component_type & service_type_mask) >> 3);
> > +                int service_type;
> > +
> > +                if (desc_end - *pp < 1)
> > +                    return AVERROR_INVALIDDATA;
> > +                desc6a->component_type = get8(pp, desc_end);
> > +                service_type = ((desc6a->component_type & service_type_mask) >> 3);
> >                  if (service_type == 0x02 /* 0b010 */) {
> >                      st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
> >                      av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, "New track disposition for id %u: %u\n", st->id, st->disposition);
> >                  }
> >              }
> > +            if (desc6a->bsid_flag) {
> > +                if (desc_end - *pp < 1)
> > +                    return AVERROR_INVALIDDATA;
> > +                desc6a->bsid = get8(pp, desc_end);
> > +            }
> > +            if (desc6a->mainid_flag) {
> > +                if (desc_end - *pp < 1)
> > +                    return AVERROR_INVALIDDATA;
> > +                desc6a->mainid = get8(pp, desc_end);
> > +            }
> > +            if (desc6a->asvc_flag) {
> > +                if (desc_end - *pp < 1)
> > +                    return AVERROR_INVALIDDATA;
> 
> desc6a leaks in each of these error paths.

thanks, will fix it.

> 
> > +                desc6a->asvc_flag = get8(pp, desc_end);
> > +            }
> > +            ret = av_stream_add_side_data(st, AV_PKT_DATA_MPEGTS_DESC_6A,
> > +                                         (uint8_t *)desc6a, desc6a_size);
> > +            if (ret < 0) {
> > +                av_free(desc6a);
> > +                return ret;
> > +            }
> >          }
> >          break;
> >      case 0x7a: /* enhanced_ac-3_descriptor */
> > diff --git a/tests/ref/fate/ts-demux b/tests/ref/fate/ts-demux
> > index cdf34d6..dfe0374 100644
> > --- a/tests/ref/fate/ts-demux
> > +++ b/tests/ref/fate/ts-demux
> > @@ -10,7 +10,7 @@
> >  #sample_rate 1: 48000
> >  #channel_layout 1: 60f
> >  #channel_layout_name 1: 5.1(side)
> > -1,          0,          0,     2880,     1536, 0x773ffeea, S=1,        1, 0x00bd00bd
> > +1,          0,          0,     2880,     1536, 0x773ffeea, S=2,        1, 0x00bd00bd,        9, 0x00000000
> >  1,       2880,       2880,     2880,     1536, 0x6dc10748
> >  1,       5760,       5760,     2880,     1536, 0xbab5129c
> >  1,       8640,       8640,     2880,     1536, 0x602f034b, S=1,        1, 0x00bd00bd
> > 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Lance Wang July 29, 2020, 12:15 a.m. UTC | #6 
On Tue, Jul 28, 2020 at 05:09:33PM +0200, Andreas Rheinhardt wrote:
> lance.lmwang@gmail.com:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  doc/APIchanges        | 3 +++
> >  libavcodec/avpacket.c | 1 +
> >  libavcodec/packet.h   | 7 +++++++
> >  libavcodec/version.h  | 2 +-
> >  4 files changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index 72a4833..2c76fdc 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -15,6 +15,9 @@ libavutil:     2017-10-21
> >  
> >  API changes, most recent first:
> >  
> > +2020-07-xx - xxxxxxxxxx - lavc 58.98.102 - packet.h
> > +  Add AV_PKT_DATA_MPEGTS_DESC_6A.
> > +
> >  2020-07-xx - xxxxxxxxxx - lavu 56.57.100 - cpu.h
> >    Add AV_CPU_FLAG_MMI and AV_CPU_FLAG_MSA.
> >  
> > diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> > index 4801163..0e88112 100644
> > --- a/libavcodec/avpacket.c
> > +++ b/libavcodec/avpacket.c
> > @@ -401,6 +401,7 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type)
> >      case AV_PKT_DATA_ICC_PROFILE:                return "ICC Profile";
> >      case AV_PKT_DATA_DOVI_CONF:                  return "DOVI configuration record";
> >      case AV_PKT_DATA_S12M_TIMECODE:              return "SMPTE ST 12-1:2014 timecode";
> > +    case AV_PKT_DATA_MPEGTS_DESC_6A:             return "ETSI 300 468 descriptor 0x6A(AC-3)";
> >      }
> >      return NULL;
> >  }
> > diff --git a/libavcodec/packet.h b/libavcodec/packet.h
> > index 0a19a0e..a18824a 100644
> > --- a/libavcodec/packet.h
> > +++ b/libavcodec/packet.h
> > @@ -291,6 +291,13 @@ enum AVPacketSideDataType {
> >      AV_PKT_DATA_S12M_TIMECODE,
> >  
> >      /**
> > +     * ETSI 300 468 descriptor 0x6A(AC-3)
> > +     * Refer to: ETSI EN 300 468 V1.11.1 (2010-04) (SI in DVB systems)
> > +     * Tags are stored in struct AVDescriptor6A.
> 
> I don't know this struct; it has to be added before the commit adding
> the new side data.

OK, will reorder the patchset#2 before.

> 
> > +     */
> > +    AV_PKT_DATA_MPEGTS_DESC_6A,
> > +
> > +    /**
> >       * The number of side data types.
> >       * This is not part of the public API/ABI in the sense that it may
> >       * change when new side data types are added.
> > diff --git a/libavcodec/version.h b/libavcodec/version.h
> > index 291e6b5..4750e25 100644
> > --- a/libavcodec/version.h
> > +++ b/libavcodec/version.h
> > @@ -28,7 +28,7 @@
> >  #include "libavutil/version.h"
> >  
> >  #define LIBAVCODEC_VERSION_MAJOR  58
> > -#define LIBAVCODEC_VERSION_MINOR  97
> > +#define LIBAVCODEC_VERSION_MINOR  98
> >  #define LIBAVCODEC_VERSION_MICRO 102
> >  
> You need to reset micro when updating minor.
Will fix it in local.

> 
> >  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> > 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index 72a4833..2c76fdc 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@  libavutil:     2017-10-21
 
 API changes, most recent first:
 
+2020-07-xx - xxxxxxxxxx - lavc 58.98.102 - packet.h
+  Add AV_PKT_DATA_MPEGTS_DESC_6A.
+
 2020-07-xx - xxxxxxxxxx - lavu 56.57.100 - cpu.h
   Add AV_CPU_FLAG_MMI and AV_CPU_FLAG_MSA.
 
diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 4801163..0e88112 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -401,6 +401,7 @@  const char *av_packet_side_data_name(enum AVPacketSideDataType type)
     case AV_PKT_DATA_ICC_PROFILE:                return "ICC Profile";
     case AV_PKT_DATA_DOVI_CONF:                  return "DOVI configuration record";
     case AV_PKT_DATA_S12M_TIMECODE:              return "SMPTE ST 12-1:2014 timecode";
+    case AV_PKT_DATA_MPEGTS_DESC_6A:             return "ETSI 300 468 descriptor 0x6A(AC-3)";
     }
     return NULL;
 }
diff --git a/libavcodec/packet.h b/libavcodec/packet.h
index 0a19a0e..a18824a 100644
--- a/libavcodec/packet.h
+++ b/libavcodec/packet.h
@@ -291,6 +291,13 @@  enum AVPacketSideDataType {
     AV_PKT_DATA_S12M_TIMECODE,
 
     /**
+     * ETSI 300 468 descriptor 0x6A(AC-3)
+     * Refer to: ETSI EN 300 468 V1.11.1 (2010-04) (SI in DVB systems)
+     * Tags are stored in struct AVDescriptor6A.
+     */
+    AV_PKT_DATA_MPEGTS_DESC_6A,
+
+    /**
      * The number of side data types.
      * This is not part of the public API/ABI in the sense that it may
      * change when new side data types are added.
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 291e6b5..4750e25 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -28,7 +28,7 @@ 
 #include "libavutil/version.h"
 
 #define LIBAVCODEC_VERSION_MAJOR  58
-#define LIBAVCODEC_VERSION_MINOR  97
+#define LIBAVCODEC_VERSION_MINOR  98
 #define LIBAVCODEC_VERSION_MICRO 102
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \