Re: [FFmpeg-devel] [PATCH] avcodec/mxfdec: Recognize AAC per SMPTE ST 381-4

2023-05-10 Thread Ammon Riley
Hi Marton,

Thanks for your feedback.

On Mon, May 8, 2023 at 3:58 PM Marton Balint  wrote:
> On Thu, 27 Apr 2023, Ammon Riley wrote:
> > This patch simply recognizes the AAC audio tracks during
> > decode -- it does not add functionality to encode AAC in
> > MXF.
> >
> > A sample file (st381-4-sample.mxf) has been uploaded to
> > https://streams.videolan.org/upload/, and is also available
> > at https://harmonicinc.box.com/v/st381-4-sample.  Audio
> > and video are both licensed as CC0.
>
> > From 9765ec18f65b8ae147660e0d71bfa80293e57f56 Mon Sep 17 00:00:00 2001
> > From: Ammon Riley 
> > Date: Wed, 26 Apr 2023 18:26:35 -0700
> > Subject: [PATCH] avcodec/mxfdec: Recognize AAC per SMPTE ST 381-4
> >
> > This patch simply recognizes the AAC audio track during
> > decode -- it does not add functionality to encode AAC in
> > MXF.
> >
> > Signed-off-by: Ammon Riley 
> > ---
> >  Changelog| 1 +
> >  libavformat/mxf.c| 1 +
> >  libavformat/mxfdec.c | 5 +
> >  3 files changed, 7 insertions(+)
> >
> > diff --git a/Changelog b/Changelog
> > index a40f32c23f..e68ee0f2c9 100644
> > --- a/Changelog
> > +++ b/Changelog
> > @@ -3,6 +3,7 @@ releases are sorted from youngest to oldest.
> >
> >  version :
> >  - libaribcaption decoder
> > +- recognize AAC in MXF (SMPTE ST 381-4)
>
> I think such small change does not warrant a changelog entry.

Removed.

> > diff --git a/libavformat/mxf.c b/libavformat/mxf.c
> > index 8ef928b8fc..deb6091003 100644
> > --- a/libavformat/mxf.c
> > +++ b/libavformat/mxf.c
> > @@ -78,6 +78,7 @@ const MXFCodecUL ff_mxf_codec_uls[] = {
> >  { {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x02,0x03,0x02,0x01,0x00
}, 15,AV_CODEC_ID_AC3 },
> >  { {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x02,0x03,0x02,0x05,0x00
}, 15,AV_CODEC_ID_MP2 }, /* MP2 or MP3 */
> >//{ {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x02,0x03,0x02,0x1C,0x00
}, 15,AV_CODEC_ID_DOLBY_E }, /* Dolby-E */
> > +{ {
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x02,0x02,0x02,0x04,0x00,0x00,0x00
}, 13,AV_CODEC_ID_AAC }, /* AAC SMPTE 381-4 */
>
> If I read the SMPTE registry correctly, AAC is only ... 0x04 0x03, and
... 0x04 0x04.

Correct.  I wasn't sure whether adding both of those more specific ULs
would be considered polluting the table unnecessarily.  In retrospect,
the UL above merely "Identifies MPEG Audio Compression" (per Table 5),
which could also include non-AAC MPEG audio.  Though Table 6 doesn't
currently define anything other than AAC audio, that's not a guarantee some
future update to the spec won't add it.

> >  { {
0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00
},  0,   AV_CODEC_ID_NONE },
> >  };
> >
> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > index 8a7008b298..c5960ecf0c 100644
> > --- a/libavformat/mxfdec.c
> > +++ b/libavformat/mxfdec.c
> > @@ -1628,6 +1628,9 @@ static const MXFCodecUL
mxf_sound_essence_container_uls[] = {
> >  { {
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x01,0x01,0x01
}, 14, AV_CODEC_ID_PCM_S16LE, NULL, 13 }, /* D-10 Mapping 50Mbps PAL
Extended Template */
> >  { {
0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0xff,0x4b,0x46,0x41,0x41,0x00,0x0d,0x4d,0x4F
}, 14, AV_CODEC_ID_PCM_S16LE }, /* 0001GL00.MXF.A1.mxf_opatom.mxf */
> >  { {
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x03,0x04,0x02,0x02,0x02,0x03,0x03,0x01,0x00
}, 14,   AV_CODEC_ID_AAC }, /* MPEG-2 AAC ADTS (legacy) */
> > +{ {
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0d,0x0d,0x01,0x03,0x01,0x02,0x16,0x00,0x00
}, 14,   AV_CODEC_ID_AAC, "aac_adif_smpte_381_4", 14 }, /* AAC SMPTE
381-4 */
> > +{ {
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0d,0x0d,0x01,0x03,0x01,0x02,0x17,0x00,0x00
}, 14,   AV_CODEC_ID_AAC, "aac_adts_smpte_381_4", 14 }, /* AAC SMPTE
381-4 */
> > +    { {
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0d,0x0d,0x01,0x03,0x01,0x02,0x18,0x00,0x00
}, 14,   AV_CODEC_ID_AAC, "aac_latm_loas_smpte_381_4", 14 }, /* AAC
SMPTE 381-4 */
>
> description fields are only used for data, so you can simply use NULL for
them for AAC.

Changed.

Updated patch attached.

Cheers,
Ammon
From 488f586a5cc1ccf7880a0191151093524c409c28 Mon Sep 17 00:00:00 2001
From: Ammon Riley 
Date: Wed, 26 Apr 2023 18:26:35 -0700
Subject: [PATCH] avcodec/mxfdec: Recognize AAC per SMPTE ST 381-4

This patch simply recognizes the AAC audio track during
decode -- it does not add functionality to encode AAC in
MXF.

Signed-off-by: Ammon Riley 
---
 libavformat/m

[FFmpeg-devel] [PATCH] avcodec/mxfdec: Recognize AAC per SMPTE ST 381-4

2023-04-27 Thread Ammon Riley
This patch simply recognizes the AAC audio tracks during
decode -- it does not add functionality to encode AAC in
MXF.

A sample file (st381-4-sample.mxf) has been uploaded to
https://streams.videolan.org/upload/, and is also available
at https://harmonicinc.box.com/v/st381-4-sample.  Audio
and video are both licensed as CC0.
From 9765ec18f65b8ae147660e0d71bfa80293e57f56 Mon Sep 17 00:00:00 2001
From: Ammon Riley 
Date: Wed, 26 Apr 2023 18:26:35 -0700
Subject: [PATCH] avcodec/mxfdec: Recognize AAC per SMPTE ST 381-4

This patch simply recognizes the AAC audio track during
decode -- it does not add functionality to encode AAC in
MXF.

Signed-off-by: Ammon Riley 
---
 Changelog| 1 +
 libavformat/mxf.c| 1 +
 libavformat/mxfdec.c | 5 +
 3 files changed, 7 insertions(+)

diff --git a/Changelog b/Changelog
index a40f32c23f..e68ee0f2c9 100644
--- a/Changelog
+++ b/Changelog
@@ -3,6 +3,7 @@ releases are sorted from youngest to oldest.
 
 version :
 - libaribcaption decoder
+- recognize AAC in MXF (SMPTE ST 381-4)
 
 version 6.0:
 - Radiance HDR image support
diff --git a/libavformat/mxf.c b/libavformat/mxf.c
index 8ef928b8fc..deb6091003 100644
--- a/libavformat/mxf.c
+++ b/libavformat/mxf.c
@@ -78,6 +78,7 @@ const MXFCodecUL ff_mxf_codec_uls[] = {
 { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x02,0x03,0x02,0x01,0x00 }, 15,AV_CODEC_ID_AC3 },
 { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x02,0x03,0x02,0x05,0x00 }, 15,AV_CODEC_ID_MP2 }, /* MP2 or MP3 */
   //{ { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x02,0x03,0x02,0x1C,0x00 }, 15,AV_CODEC_ID_DOLBY_E }, /* Dolby-E */
+{ { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0D,0x04,0x02,0x02,0x02,0x04,0x00,0x00,0x00 }, 13,AV_CODEC_ID_AAC }, /* AAC SMPTE 381-4 */
 { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 },  0,   AV_CODEC_ID_NONE },
 };
 
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 8a7008b298..c5960ecf0c 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -1628,6 +1628,9 @@ static const MXFCodecUL mxf_sound_essence_container_uls[] = {
 { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x01,0x01,0x01 }, 14, AV_CODEC_ID_PCM_S16LE, NULL, 13 }, /* D-10 Mapping 50Mbps PAL Extended Template */
 { { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0xff,0x4b,0x46,0x41,0x41,0x00,0x0d,0x4d,0x4F }, 14, AV_CODEC_ID_PCM_S16LE }, /* 0001GL00.MXF.A1.mxf_opatom.mxf */
 { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x03,0x04,0x02,0x02,0x02,0x03,0x03,0x01,0x00 }, 14,   AV_CODEC_ID_AAC }, /* MPEG-2 AAC ADTS (legacy) */
+{ { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0d,0x0d,0x01,0x03,0x01,0x02,0x16,0x00,0x00 }, 14,   AV_CODEC_ID_AAC, "aac_adif_smpte_381_4", 14 }, /* AAC SMPTE 381-4 */
+{ { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0d,0x0d,0x01,0x03,0x01,0x02,0x17,0x00,0x00 }, 14,   AV_CODEC_ID_AAC, "aac_adts_smpte_381_4", 14 }, /* AAC SMPTE 381-4 */
+{ { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0d,0x0d,0x01,0x03,0x01,0x02,0x18,0x00,0x00 }, 14,   AV_CODEC_ID_AAC, "aac_latm_loas_smpte_381_4", 14 }, /* AAC SMPTE 381-4 */
 { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 },  0,  AV_CODEC_ID_NONE },
 };
 
@@ -3029,6 +3032,8 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
 st->codecpar->codec_id = AV_CODEC_ID_PCM_S32BE;
 } else if (st->codecpar->codec_id == AV_CODEC_ID_MP2) {
 sti->need_parsing = AVSTREAM_PARSE_FULL;
+} else if (st->codecpar->codec_id == AV_CODEC_ID_AAC) {
+sti->need_parsing = AVSTREAM_PARSE_FULL;
 }
 st->codecpar->bits_per_coded_sample = av_get_bits_per_sample(st->codecpar->codec_id);
 
-- 
2.29.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".