diff mbox series

[FFmpeg-devel,v2,1/5] avutil/mpegts_audio_desc_metadata: add helper function for AC3 descriptor 0x6a

Message ID 1596121138-20997-1-git-send-email-lance.lmwang@gmail.com
State New
Headers show
Series [FFmpeg-devel,v2,1/5] avutil/mpegts_audio_desc_metadata: add helper function for AC3 descriptor 0x6a
Related show

Checks

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

Commit Message

Limin Wang July 30, 2020, 2:58 p.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavutil/Makefile                     |  2 ++
 libavutil/mpegts_audio_desc_metadata.c | 33 ++++++++++++++++++++
 libavutil/mpegts_audio_desc_metadata.h | 57 ++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+)
 create mode 100644 libavutil/mpegts_audio_desc_metadata.c
 create mode 100644 libavutil/mpegts_audio_desc_metadata.h

Comments

Marton Balint Aug. 5, 2020, 7:55 a.m. UTC | #1
On Thu, 30 Jul 2020, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> libavutil/Makefile                     |  2 ++
> libavutil/mpegts_audio_desc_metadata.c | 33 ++++++++++++++++++++
> libavutil/mpegts_audio_desc_metadata.h | 57 ++++++++++++++++++++++++++++++++++
> 3 files changed, 92 insertions(+)
> create mode 100644 libavutil/mpegts_audio_desc_metadata.c
> create mode 100644 libavutil/mpegts_audio_desc_metadata.h
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 9b08372..4b4aa68 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -57,6 +57,7 @@ HEADERS = adler32.h                                                     \
>           md5.h                                                         \
>           mem.h                                                         \
>           motion_vector.h                                               \
> +          mpegts_audio_desc_metadata.h                                  \
>           murmur3.h                                                     \
>           opt.h                                                         \
>           parseutils.h                                                  \
> @@ -140,6 +141,7 @@ OBJS = adler32.o                                                        \
>        mastering_display_metadata.o                                     \
>        md5.o                                                            \
>        mem.o                                                            \
> +       mpegts_audio_desc_metadata.o                                     \
>        murmur3.o                                                        \
>        opt.o                                                            \
>        parseutils.o                                                     \
> diff --git a/libavutil/mpegts_audio_desc_metadata.c b/libavutil/mpegts_audio_desc_metadata.c
> new file mode 100644
> index 0000000..14d9100
> --- /dev/null
> +++ b/libavutil/mpegts_audio_desc_metadata.c
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright (c) 2020 Limin Wang <lance.lmwang@gmail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +#include "mpegts_audio_desc_metadata.h"
> +#include "mem.h"
> +
> +AVDescriptor6A *av_desc6a_alloc(size_t *size)

The naming is not OK, as others have already pointed out, because 6A 
is not a good name.

AVMPEGTSAC3Descriptor or something similar would be better.

Same goes for the side data type, e.g. AV_PKT_DATA_MPEGTS_AC3_DESC

> +{
> +    AVDescriptor6A *desc6a = (AVDescriptor6A*)av_mallocz(sizeof(*desc6a));
> +
> +    if (!desc6a)
> +        return NULL;
> +    if (size)
> +        *size = sizeof(*desc6a);
> +    return desc6a;
> +
> +}

The same kind of alloc function is used for other kinds of side data. 
Maybe it would make sense to figure out some #define magic or something to 
not duplicate this kind of code, but somehow keep type safety?

E.g. a generic uint8_t *av_alloc_side_data_type(type, *size) and then 
using #defines:

#define av_alloc_mpegts_ac3_descriptor(size) \
     (AVMPEGTSAC3Descriptor *)av_alloc_side_data_type(AV_PKT_DATA_MPEGTS_AC3_DESC, size)

Just an idea...

Regards,
Marton
Limin Wang Aug. 5, 2020, 11:18 a.m. UTC | #2
On Wed, Aug 05, 2020 at 09:55:52AM +0200, Marton Balint wrote:
> 
> 
> On Thu, 30 Jul 2020, lance.lmwang@gmail.com wrote:
> 
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> > libavutil/Makefile                     |  2 ++
> > libavutil/mpegts_audio_desc_metadata.c | 33 ++++++++++++++++++++
> > libavutil/mpegts_audio_desc_metadata.h | 57 ++++++++++++++++++++++++++++++++++
> > 3 files changed, 92 insertions(+)
> > create mode 100644 libavutil/mpegts_audio_desc_metadata.c
> > create mode 100644 libavutil/mpegts_audio_desc_metadata.h
> > 
> > diff --git a/libavutil/Makefile b/libavutil/Makefile
> > index 9b08372..4b4aa68 100644
> > --- a/libavutil/Makefile
> > +++ b/libavutil/Makefile
> > @@ -57,6 +57,7 @@ HEADERS = adler32.h                                                     \
> >           md5.h                                                         \
> >           mem.h                                                         \
> >           motion_vector.h                                               \
> > +          mpegts_audio_desc_metadata.h                                  \
> >           murmur3.h                                                     \
> >           opt.h                                                         \
> >           parseutils.h                                                  \
> > @@ -140,6 +141,7 @@ OBJS = adler32.o                                                        \
> >        mastering_display_metadata.o                                     \
> >        md5.o                                                            \
> >        mem.o                                                            \
> > +       mpegts_audio_desc_metadata.o                                     \
> >        murmur3.o                                                        \
> >        opt.o                                                            \
> >        parseutils.o                                                     \
> > diff --git a/libavutil/mpegts_audio_desc_metadata.c b/libavutil/mpegts_audio_desc_metadata.c
> > new file mode 100644
> > index 0000000..14d9100
> > --- /dev/null
> > +++ b/libavutil/mpegts_audio_desc_metadata.c
> > @@ -0,0 +1,33 @@
> > +/*
> > + * Copyright (c) 2020 Limin Wang <lance.lmwang@gmail.com>
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +#include "mpegts_audio_desc_metadata.h"
> > +#include "mem.h"
> > +
> > +AVDescriptor6A *av_desc6a_alloc(size_t *size)
> 
> The naming is not OK, as others have already pointed out, because 6A is not
> a good name.
> 
> AVMPEGTSAC3Descriptor or something similar would be better.
> 
> Same goes for the side data type, e.g. AV_PKT_DATA_MPEGTS_AC3_DESC
For AC-3 have two different descriptor for DVB and ATSC standard, so if use
AC-3, DVB or ATSC is necessary also. Also for E-AC3. So I feel they are
too long for the structure and data type. Below is list for compare:

AVMPEGTSDVBAC3Descriptor 
AVMPEGTSDVBEAC3Descriptor 
AVMPEGTSATSCAC3Descriptor 
AVMPEGTSATSCEAC3Descriptor 

vs

AVDescriptor6A
AVDescriptor7A
AVDescriptor81
AVDescriptorCC

AV_PKT_DATA_MPEGTS_DVB_AC3_DESC
AV_PKT_DATA_MPEGTS_DVB_EAC3_DESC
AV_PKT_DATA_MPEGTS_ATSC_AC3_DESC
AV_PKT_DATA_MPEGTS_ATSC_EAC3_DESC

vs

AV_PKT_DATA_MPEGTS_DESC_6A
AV_PKT_DATA_MPEGTS_DESC_7A
AV_PKT_DATA_MPEGTS_DESC_81
AV_PKT_DATA_MPEGTS_DESC_CC

If you prefer to use the long name or have better suggestion, please comments.

Also, I'm not sure wheter it's good solution to add them as side data, how to
process more descriptor like DTS etc?

> 
> > +{
> > +    AVDescriptor6A *desc6a = (AVDescriptor6A*)av_mallocz(sizeof(*desc6a));
> > +
> > +    if (!desc6a)
> > +        return NULL;
> > +    if (size)
> > +        *size = sizeof(*desc6a);
> > +    return desc6a;
> > +
> > +}
> 
> The same kind of alloc function is used for other kinds of side data. Maybe
> it would make sense to figure out some #define magic or something to not
> duplicate this kind of code, but somehow keep type safety?
> 
> E.g. a generic uint8_t *av_alloc_side_data_type(type, *size) and then using
> #defines:
> 
> #define av_alloc_mpegts_ac3_descriptor(size) \
>     (AVMPEGTSAC3Descriptor *)av_alloc_side_data_type(AV_PKT_DATA_MPEGTS_AC3_DESC, size)
> 
> Just an idea...
> 
> Regards,
> Marton
> _______________________________________________
> 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/libavutil/Makefile b/libavutil/Makefile
index 9b08372..4b4aa68 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -57,6 +57,7 @@  HEADERS = adler32.h                                                     \
           md5.h                                                         \
           mem.h                                                         \
           motion_vector.h                                               \
+          mpegts_audio_desc_metadata.h                                  \
           murmur3.h                                                     \
           opt.h                                                         \
           parseutils.h                                                  \
@@ -140,6 +141,7 @@  OBJS = adler32.o                                                        \
        mastering_display_metadata.o                                     \
        md5.o                                                            \
        mem.o                                                            \
+       mpegts_audio_desc_metadata.o                                     \
        murmur3.o                                                        \
        opt.o                                                            \
        parseutils.o                                                     \
diff --git a/libavutil/mpegts_audio_desc_metadata.c b/libavutil/mpegts_audio_desc_metadata.c
new file mode 100644
index 0000000..14d9100
--- /dev/null
+++ b/libavutil/mpegts_audio_desc_metadata.c
@@ -0,0 +1,33 @@ 
+/*
+ * Copyright (c) 2020 Limin Wang <lance.lmwang@gmail.com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+#include "mpegts_audio_desc_metadata.h"
+#include "mem.h"
+
+AVDescriptor6A *av_desc6a_alloc(size_t *size)
+{
+    AVDescriptor6A *desc6a = (AVDescriptor6A*)av_mallocz(sizeof(*desc6a));
+
+    if (!desc6a)
+        return NULL;
+    if (size)
+        *size = sizeof(*desc6a);
+    return desc6a;
+
+}
diff --git a/libavutil/mpegts_audio_desc_metadata.h b/libavutil/mpegts_audio_desc_metadata.h
new file mode 100644
index 0000000..ae0567e
--- /dev/null
+++ b/libavutil/mpegts_audio_desc_metadata.h
@@ -0,0 +1,57 @@ 
+/*
+ * Copyright (c) 2020 Limin Wang <lance.lmwang@gmail.com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * MPEGTS audio stream descriptor
+ */
+
+#ifndef AVUTIL_MPEGTS_AUDIO_DESC_METADATA_H
+#define AVUTIL_MPEGTS_AUDIO_DESC_METADATA_H
+#include <stdint.h>
+#include <stddef.h>
+
+/* ETSI 300 468 descriptor 0x6A(AC-3)
+ * Refer to: ETSI EN 300 468 V1.11.1 (2010-04) (SI in DVB systems)
+ *
+ * @note The struct must be allocated with av_desc6a_alloc() and
+ *       its size is not a part of the public ABI.
+ */
+typedef struct AVDescriptor6A {
+    uint8_t  component_type_flag;
+    uint8_t  bsid_flag;
+    uint8_t  mainid_flag;
+    uint8_t  asvc_flag;
+    uint8_t  reserved_flags;
+    uint8_t  component_type;
+    uint8_t  bsid;
+    uint8_t  mainid;
+    uint8_t  asvc;
+} AVDescriptor6A;
+
+/**
+ * Allocate a AVDescriptor6A structure and initialize its
+ * fields to default values.
+ *
+ * @return the newly allocated struct or NULL on failure
+ */
+AVDescriptor6A *av_desc6a_alloc(size_t *size);
+
+#endif /* AVUTIL_MPEGTS_AUDIO_DESC_METADATA_H */