[FFmpeg-devel] [PATCH 0/6] avformat/movenc: normalize on AC-3 parser usage

[FFmpeg-devel] [PATCH 0/6] avformat/movenc: normalize on AC-3 parser usage

[Jan Ekström]

The simplified parsing currently in `mov_write_ac3_tag` trusts the content
of the packets a bit too much (the AC-3 parser returns all data fed to it,
including any possible data before the start code), while the existing E-AC-3
logic does proper header validation by utilizing the (E-)AC-3 parser.

Thus, normalize on AC-3 parser usage for both AC-3 and E-AC-3. 

Jan Ekström (6):
  avcodec/ac3_parser{,_internal}: expose AC-3 bit_rate_code
  {configure,avformat/movenc}: enable AC-3 parser for movenc
  avformat/movenc: enable handle_eac3 to handle AC-3 tracks
  avformat/movenc: move eac3_info definition so that it can be used for
    AC-3
  avformat/movenc: utilize existing AC-3 parsing workflow for AC-3
  avformat/movenc: handle OOM situations when parsing AC-3 headers

 configure                        |   2 +-
 libavcodec/ac3_parser.c          |   3 +
 libavcodec/ac3_parser_internal.h |   1 +
 libavformat/movenc.c             | 123 ++++++++++++++++---------------
 4 files changed, 69 insertions(+), 60 deletions(-)

-- 
2.36.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 1/6] avcodec/ac3_parser{, _internal}: expose AC-3 bit_rate_code

[Jan Ekström]

From: Jan Ekström <jan.ekstrom@24i.com>

Required by MP4's AC3SpecificBox and MPEG-TS AC-3 audio_descriptor,
of which the former is implemented in our MP4 writer.

Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
---
 libavcodec/ac3_parser.c          | 3 +++
 libavcodec/ac3_parser_internal.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
index 119b1598c5..852c9e6b0d 100644
--- a/libavcodec/ac3_parser.c
+++ b/libavcodec/ac3_parser.c
@@ -70,6 +70,7 @@ int ff_ac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr)
         return AAC_AC3_PARSE_ERROR_BSID;
 
     hdr->num_blocks = 6;
+    hdr->ac3_bit_rate_code = -1;
 
     /* set default mix levels */
     hdr->center_mix_level   = 5;  // -4.5dB
@@ -89,6 +90,8 @@ int ff_ac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr)
         if(frame_size_code > 37)
             return AAC_AC3_PARSE_ERROR_FRAME_SIZE;
 
+        hdr->ac3_bit_rate_code = (frame_size_code >> 1);
+
         skip_bits(gbc, 5); // skip bsid, already got it
 
         hdr->bitstream_mode = get_bits(gbc, 3);
diff --git a/libavcodec/ac3_parser_internal.h b/libavcodec/ac3_parser_internal.h
index dd57dc95a6..bd4e1bbffb 100644
--- a/libavcodec/ac3_parser_internal.h
+++ b/libavcodec/ac3_parser_internal.h
@@ -60,6 +60,7 @@ typedef struct AC3HeaderInfo {
     uint8_t channels;
     uint16_t frame_size;
     uint64_t channel_layout;
+    int8_t ac3_bit_rate_code;
     /** @} */
 } AC3HeaderInfo;
 
-- 
2.36.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/6] {configure, avformat/movenc}: enable AC-3 parser for movenc

[Jan Ekström]

From: Jan Ekström <jan.ekstrom@24i.com>

This simplifies the code to no longer have #ifs in a manner which
does not require handling avpriv_ac3_parse_header returning ENOSYS.

As an existing example, the MPEG-TS muxer already requires the AC-3
parser, and in order to fix existing issues with the current AC-3
movenc code, switching to use the AC-3 parser is required, so this
is an enabling change for that.

Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
---
 configure            | 2 +-
 libavformat/movenc.c | 4 ----
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/configure b/configure
index 5a167613a4..a4e8e33b83 100755
--- a/configure
+++ b/configure
@@ -3440,7 +3440,7 @@ mlp_demuxer_select="mlp_parser"
 mmf_muxer_select="riffenc"
 mov_demuxer_select="iso_media riffdec"
 mov_demuxer_suggest="zlib"
-mov_muxer_select="iso_media riffenc rtpenc_chain vp9_superframe_bsf aac_adtstoasc_bsf"
+mov_muxer_select="iso_media riffenc rtpenc_chain vp9_superframe_bsf aac_adtstoasc_bsf ac3_parser"
 mp3_demuxer_select="mpegaudio_parser"
 mp3_muxer_select="mpegaudioheader"
 mp4_muxer_select="mov_muxer"
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index de971f94e8..63713e5640 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -407,7 +407,6 @@ struct eac3_info {
     } substream[1]; /* TODO: support 8 independent substreams */
 };
 
-#if CONFIG_AC3_PARSER
 static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
 {
     AC3HeaderInfo *hdr = NULL;
@@ -548,7 +547,6 @@ end:
 
     return ret;
 }
-#endif
 
 static int mov_write_eac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track)
 {
@@ -6045,7 +6043,6 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
             }
         }
 
-#if CONFIG_AC3_PARSER
     } else if (par->codec_id == AV_CODEC_ID_EAC3) {
         size = handle_eac3(mov, pkt, trk);
         if (size < 0)
@@ -6053,7 +6050,6 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
         else if (!size)
             goto end;
         avio_write(pb, pkt->data, size);
-#endif
     } else if (par->codec_id == AV_CODEC_ID_EIA_608) {
         size = 8;
 
-- 
2.36.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 3/6] avformat/movenc: enable handle_eac3 to handle AC-3 tracks

[Jan Ekström]

From: Jan Ekström <jan.ekstrom@24i.com>

Add the AC-3 frame type, as well as early exit from additional packet
parsing in case of AC-3, as only a single packet is required to get
the required information.

Additionally, expose ac3_bit_rate_code via the eac3_info struct as
it is required for AC3SpecificBox.

Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
---
 libavformat/movenc.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 63713e5640..2589f2f4bb 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -383,6 +383,7 @@ struct eac3_info {
     /* Layout of the EC3SpecificBox */
     /* maximum bitrate */
     uint16_t data_rate;
+    int8_t   ac3_bit_rate_code;
     /* number of independent substreams */
     uint8_t  num_ind_sub;
     struct {
@@ -413,8 +414,12 @@ static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
     struct eac3_info *info;
     int num_blocks, ret;
 
-    if (!track->eac3_priv && !(track->eac3_priv = av_mallocz(sizeof(*info))))
-        return AVERROR(ENOMEM);
+    if (!track->eac3_priv) {
+        if (!(track->eac3_priv = av_mallocz(sizeof(*info))))
+            return AVERROR(ENOMEM);
+
+        ((struct eac3_info *)track->eac3_priv)->ac3_bit_rate_code = -1;
+    }
     info = track->eac3_priv;
 
     if (!info->pkt && !(info->pkt = av_packet_alloc()))
@@ -431,6 +436,8 @@ static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
     }
 
     info->data_rate = FFMAX(info->data_rate, hdr->bit_rate / 1000);
+    info->ac3_bit_rate_code = FFMAX(info->ac3_bit_rate_code,
+                                    hdr->ac3_bit_rate_code);
     num_blocks = hdr->num_blocks;
 
     if (!info->ec3_done) {
@@ -442,7 +449,8 @@ static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
 
         /* this should always be the case, given that our AC-3 parser
          * concatenates dependent frames to their independent parent */
-        if (hdr->frame_type == EAC3_FRAME_TYPE_INDEPENDENT) {
+        if (hdr->frame_type == EAC3_FRAME_TYPE_INDEPENDENT ||
+            hdr->frame_type == EAC3_FRAME_TYPE_AC3_CONVERT) {
             /* substream ids must be incremental */
             if (hdr->substreamid > info->num_ind_sub + 1) {
                 ret = AVERROR(EINVAL);
@@ -474,6 +482,14 @@ static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
         info->substream[hdr->substreamid].acmod = hdr->channel_mode;
         info->substream[hdr->substreamid].lfeon = hdr->lfe_on;
 
+        if (track->par->codec_id == AV_CODEC_ID_AC3) {
+            // with AC-3 we only require the information of a single packet,
+            // so we can finish as soon as the basic values of the bit stream
+            // have been set to the track's informational structure.
+            info->ec3_done = 1;
+            goto concatenate;
+        }
+
         /* Parse dependent substream(s), if any */
         if (pkt->size != hdr->frame_size) {
             int cumul_size = hdr->frame_size;
-- 
2.36.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 4/6] avformat/movenc: move eac3_info definition so that it can be used for AC-3

[Jan Ekström]

From: Jan Ekström <jan.ekstrom@24i.com>

Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
---
 libavformat/movenc.c | 66 ++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 2589f2f4bb..5bc1f4c7a4 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -327,6 +327,39 @@ static int mov_write_amr_tag(AVIOContext *pb, MOVTrack *track)
     return 0x11;
 }
 
+struct eac3_info {
+    AVPacket *pkt;
+    uint8_t ec3_done;
+    uint8_t num_blocks;
+
+    /* Layout of the EC3SpecificBox */
+    /* maximum bitrate */
+    uint16_t data_rate;
+    int8_t   ac3_bit_rate_code;
+    /* number of independent substreams */
+    uint8_t  num_ind_sub;
+    struct {
+        /* sample rate code (see ff_ac3_sample_rate_tab) 2 bits */
+        uint8_t fscod;
+        /* bit stream identification 5 bits */
+        uint8_t bsid;
+        /* one bit reserved */
+        /* audio service mixing (not supported yet) 1 bit */
+        /* bit stream mode 3 bits */
+        uint8_t bsmod;
+        /* audio coding mode 3 bits */
+        uint8_t acmod;
+        /* sub woofer on 1 bit */
+        uint8_t lfeon;
+        /* 3 bits reserved */
+        /* number of dependent substreams associated with this substream 4 bits */
+        uint8_t num_dep_sub;
+        /* channel locations of the dependent substream(s), if any, 9 bits */
+        uint16_t chan_loc;
+        /* if there is no dependent substream, then one bit reserved instead */
+    } substream[1]; /* TODO: support 8 independent substreams */
+};
+
 static int mov_write_ac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track)
 {
     GetBitContext gbc;
@@ -375,39 +408,6 @@ static int mov_write_ac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *trac
     return 11;
 }
 
-struct eac3_info {
-    AVPacket *pkt;
-    uint8_t ec3_done;
-    uint8_t num_blocks;
-
-    /* Layout of the EC3SpecificBox */
-    /* maximum bitrate */
-    uint16_t data_rate;
-    int8_t   ac3_bit_rate_code;
-    /* number of independent substreams */
-    uint8_t  num_ind_sub;
-    struct {
-        /* sample rate code (see ff_ac3_sample_rate_tab) 2 bits */
-        uint8_t fscod;
-        /* bit stream identification 5 bits */
-        uint8_t bsid;
-        /* one bit reserved */
-        /* audio service mixing (not supported yet) 1 bit */
-        /* bit stream mode 3 bits */
-        uint8_t bsmod;
-        /* audio coding mode 3 bits */
-        uint8_t acmod;
-        /* sub woofer on 1 bit */
-        uint8_t lfeon;
-        /* 3 bits reserved */
-        /* number of dependent substreams associated with this substream 4 bits */
-        uint8_t num_dep_sub;
-        /* channel locations of the dependent substream(s), if any, 9 bits */
-        uint16_t chan_loc;
-        /* if there is no dependent substream, then one bit reserved instead */
-    } substream[1]; /* TODO: support 8 independent substreams */
-};
-
 static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
 {
     AC3HeaderInfo *hdr = NULL;
-- 
2.36.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 5/6] avformat/movenc: utilize existing AC-3 parsing workflow for AC-3

[Jan Ekström]

From: Jan Ekström <jan.ekstrom@24i.com>

Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
---
 libavformat/movenc.c | 46 +++++++++++++++++---------------------------
 1 file changed, 18 insertions(+), 28 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 5bc1f4c7a4..a1daf3598b 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -362,44 +362,34 @@ struct eac3_info {
 
 static int mov_write_ac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track)
 {
-    GetBitContext gbc;
+    struct eac3_info *info = track->eac3_priv;
     PutBitContext pbc;
     uint8_t buf[3];
-    int fscod, bsid, bsmod, acmod, lfeon, frmsizecod;
 
-    if (track->vos_len < 7) {
+    if (!info || !info->ec3_done) {
         av_log(s, AV_LOG_ERROR,
                "Cannot write moov atom before AC3 packets."
                " Set the delay_moov flag to fix this.\n");
         return AVERROR(EINVAL);
     }
 
+    if (info->ac3_bit_rate_code < 0) {
+        av_log(s, AV_LOG_ERROR,
+               "No valid AC3 bit rate code for data rate of %d!\n",
+               info->data_rate);
+        return AVERROR(EINVAL);
+    }
+
     avio_wb32(pb, 11);
     ffio_wfourcc(pb, "dac3");
 
-    init_get_bits(&gbc, track->vos_data + 4, (track->vos_len - 4) * 8);
-    fscod      = get_bits(&gbc, 2);
-    frmsizecod = get_bits(&gbc, 6);
-    bsid       = get_bits(&gbc, 5);
-    bsmod      = get_bits(&gbc, 3);
-    acmod      = get_bits(&gbc, 3);
-    if (acmod == 2) {
-        skip_bits(&gbc, 2); // dsurmod
-    } else {
-        if ((acmod & 1) && acmod != 1)
-            skip_bits(&gbc, 2); // cmixlev
-        if (acmod & 4)
-            skip_bits(&gbc, 2); // surmixlev
-    }
-    lfeon = get_bits1(&gbc);
-
     init_put_bits(&pbc, buf, sizeof(buf));
-    put_bits(&pbc, 2, fscod);
-    put_bits(&pbc, 5, bsid);
-    put_bits(&pbc, 3, bsmod);
-    put_bits(&pbc, 3, acmod);
-    put_bits(&pbc, 1, lfeon);
-    put_bits(&pbc, 5, frmsizecod >> 1); // bit_rate_code
+    put_bits(&pbc, 2, info->substream[0].fscod);
+    put_bits(&pbc, 5, info->substream[0].bsid);
+    put_bits(&pbc, 3, info->substream[0].bsmod);
+    put_bits(&pbc, 3, info->substream[0].acmod);
+    put_bits(&pbc, 1, info->substream[0].lfeon);
+    put_bits(&pbc, 5, info->ac3_bit_rate_code); // bit_rate_code
     put_bits(&pbc, 5, 0); // reserved
 
     flush_put_bits(&pbc);
@@ -5981,8 +5971,7 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
     if ((par->codec_id == AV_CODEC_ID_DNXHD ||
          par->codec_id == AV_CODEC_ID_H264 ||
          par->codec_id == AV_CODEC_ID_HEVC ||
-         par->codec_id == AV_CODEC_ID_TRUEHD ||
-         par->codec_id == AV_CODEC_ID_AC3) && !trk->vos_len &&
+         par->codec_id == AV_CODEC_ID_TRUEHD) && !trk->vos_len &&
          !TAG_IS_AVCI(trk->tag)) {
         /* copy frame to create needed atoms */
         trk->vos_len  = size;
@@ -6059,7 +6048,8 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
             }
         }
 
-    } else if (par->codec_id == AV_CODEC_ID_EAC3) {
+    } else if (par->codec_id == AV_CODEC_ID_EAC3 ||
+               par->codec_id == AV_CODEC_ID_AC3) {
         size = handle_eac3(mov, pkt, trk);
         if (size < 0)
             return size;
-- 
2.36.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 6/6] avformat/movenc: handle OOM situations when parsing AC-3 headers

[Jan Ekström]

From: Jan Ekström <jan.ekstrom@24i.com>

Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
---
 libavformat/movenc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index a1daf3598b..0f92f0e04e 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -415,7 +415,10 @@ static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
     if (!info->pkt && !(info->pkt = av_packet_alloc()))
         return AVERROR(ENOMEM);
 
-    if (avpriv_ac3_parse_header(&hdr, pkt->data, pkt->size) < 0) {
+    if ((ret = avpriv_ac3_parse_header(&hdr, pkt->data, pkt->size) < 0)) {
+        if (ret == AVERROR(ENOMEM))
+            goto end;
+
         /* drop the packets until we see a good one */
         if (!track->entry) {
             av_log(mov->fc, AV_LOG_WARNING, "Dropping invalid packet from start of the stream\n");
-- 
2.36.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 0/6] avformat/movenc: normalize on AC-3 parser usage

[Jan Ekström]

On Fri, Jun 3, 2022 at 3:46 PM Jan Ekström <jeebjp@gmail.com> wrote:
>
> The simplified parsing currently in `mov_write_ac3_tag` trusts the content
> of the packets a bit too much (the AC-3 parser returns all data fed to it,
> including any possible data before the start code), while the existing E-AC-3
> logic does proper header validation by utilizing the (E-)AC-3 parser.
>
> Thus, normalize on AC-3 parser usage for both AC-3 and E-AC-3.

Ping on this patch set, as this seems to improve the state of affairs
with the input being MPEG-TS muxes where PES packets do not start with
a start code (the previous ad-hoc parsing was way too trusting and
would write bogus values based on the last N bytes of a previous
packet into the AC-3 sample description).

When applying, `avcodec/ac3_parser{,_internal}: expose AC-3
bit_rate_code` will have a minor bump as technically a
private-yet-utilized-from-lavf structure has something appended to it.

Jan
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 0/6] avformat/movenc: normalize on AC-3 parser usage

[Jan Ekström]

On Tue, Jun 7, 2022 at 9:58 AM Jan Ekström <jeebjp@gmail.com> wrote:
>
> On Fri, Jun 3, 2022 at 3:46 PM Jan Ekström <jeebjp@gmail.com> wrote:
> >
> > The simplified parsing currently in `mov_write_ac3_tag` trusts the content
> > of the packets a bit too much (the AC-3 parser returns all data fed to it,
> > including any possible data before the start code), while the existing E-AC-3
> > logic does proper header validation by utilizing the (E-)AC-3 parser.
> >
> > Thus, normalize on AC-3 parser usage for both AC-3 and E-AC-3.
>
> Ping on this patch set, as this seems to improve the state of affairs
> with the input being MPEG-TS muxes where PES packets do not start with
> a start code (the previous ad-hoc parsing was way too trusting and
> would write bogus values based on the last N bytes of a previous
> packet into the AC-3 sample description).
>
> When applying, `avcodec/ac3_parser{,_internal}: expose AC-3
> bit_rate_code` will have a minor bump as technically a
> private-yet-utilized-from-lavf structure has something appended to it.

If there are no further comments, I will pull this in tomorrow, as
this has been generating valid sample descriptions for a piece of
software which is picky that the data is correct.

Jan
_______________________________________________
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".