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

[Marton Balint]

On Thu, 30 Jul 2020, lance.lmwang at gmail.com wrote:

> From: Limin Wang <lance.lmwang at gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang at 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 at 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