| 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 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
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
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 --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 */