[FFmpeg-devel] [PATCH 1/4] avcodec/ac3_parser: use a padded buffer in av_ac3_parse_header()

[FFmpeg-devel] [PATCH 1/4] avcodec/ac3_parser: use a padded buffer in av_ac3_parse_header()

[James Almer]

The GetBitContext API requires the buffer to be padded, and the documentation for
av_ac3_parse_header() does not specify it, so use a temporary local buffer.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/ac3_parser.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
index 69989690dd..9065d700e2 100644
--- a/libavcodec/ac3_parser.c
+++ b/libavcodec/ac3_parser.c
@@ -202,14 +202,24 @@ int av_ac3_parse_header(const uint8_t *buf, size_t size,
 {
     GetBitContext gb;
     AC3HeaderInfo hdr;
+    uint8_t *tmp = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
     int err;
 
-    err = init_get_bits8(&gb, buf, size);
-    if (err < 0)
+    if (!tmp)
+        return AVERROR(ENOMEM);
+
+    memcpy(tmp, buf, size);
+    memset(tmp + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
+    err = init_get_bits8(&gb, tmp, size);
+    if (err < 0) {
+        av_free(tmp);
         return AVERROR_INVALIDDATA;
+    }
     err = ff_ac3_parse_header(&gb, &hdr);
-    if (err < 0)
+    if (err < 0) {
+        av_free(tmp);
         return AVERROR_INVALIDDATA;
+    }
 
     *bitstream_id = hdr.bitstream_id;
     *frame_size   = hdr.frame_size;
-- 
2.49.0

_______________________________________________
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 v2 2/4] avcodec/ac3_parser: handle more header bits in ff_ac3_parse_header()

[James Almer]

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/ac3_parser.c          | 219 +++++++++++++++++++++++++++++++
 libavcodec/ac3_parser_internal.h |  19 +++
 libavcodec/ac3dec.c              |  88 +++----------
 libavcodec/ac3dec.h              |   4 +-
 libavcodec/ac3defs.h             |   2 +
 libavcodec/eac3dec.c             | 154 +++-------------------
 libavformat/movenc.c             |  17 +--
 7 files changed, 282 insertions(+), 221 deletions(-)

diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
index 9065d700e2..8545f18e14 100644
--- a/libavcodec/ac3_parser.c
+++ b/libavcodec/ac3_parser.c
@@ -73,6 +73,217 @@ int ff_ac3_find_syncword(const uint8_t *buf, int buf_size)
     return i;
 }
 
+/**
+ * Parse the 'sync info' and 'bit stream info' from the AC-3 bitstream.
+ * GetBitContext within AC3DecodeContext must point to
+ * the start of the synchronized AC-3 bitstream.
+ */
+static int ac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr)
+{
+    /* read the rest of the bsi. read twice for dual mono mode. */
+    for (int i = 0; i < (hdr->channel_mode ? 1 : 2); i++) {
+        hdr->dialog_normalization[i] = -get_bits(gbc, 5);
+        hdr->compression_exists[i] = get_bits1(gbc);
+        if (hdr->compression_exists[i])
+            hdr->heavy_dynamic_range[i] = get_bits(gbc, 8);
+        if (get_bits1(gbc))
+            skip_bits(gbc, 8); //skip language code
+        if (get_bits1(gbc))
+            skip_bits(gbc, 7); //skip audio production information
+    }
+
+    skip_bits(gbc, 2); //skip copyright bit and original bitstream bit
+
+    /* skip the timecodes or parse the Alternate Bit Stream Syntax */
+    if (hdr->bitstream_id != 6) {
+        if (get_bits1(gbc))
+            skip_bits(gbc, 14); //skip timecode1
+        if (get_bits1(gbc))
+            skip_bits(gbc, 14); //skip timecode2
+    } else {
+        if (get_bits1(gbc)) {
+            hdr->preferred_downmix       = get_bits(gbc, 2);
+            hdr->center_mix_level_ltrt   = get_bits(gbc, 3);
+            hdr->surround_mix_level_ltrt = av_clip(get_bits(gbc, 3), 3, 7);
+            hdr->center_mix_level        = get_bits(gbc, 3);
+            hdr->surround_mix_level      = av_clip(get_bits(gbc, 3), 3, 7);
+        }
+        if (get_bits1(gbc)) {
+            hdr->dolby_surround_ex_mode = get_bits(gbc, 2);
+            hdr->dolby_headphone_mode   = get_bits(gbc, 2);
+            skip_bits(gbc, 10); // skip adconvtyp (1), xbsi2 (8), encinfo (1)
+        }
+    }
+
+    /* skip additional bitstream info */
+    if (get_bits1(gbc)) {
+        int i = get_bits(gbc, 6);
+        do {
+            skip_bits(gbc, 8);
+        } while (i--);
+    }
+
+    return 0;
+}
+
+static int eac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr)
+{
+    if (hdr->frame_type == EAC3_FRAME_TYPE_RESERVED)
+        return AC3_PARSE_ERROR_FRAME_TYPE;
+    if (hdr->substreamid)
+        return AC3_PARSE_ERROR_FRAME_TYPE;
+
+    skip_bits(gbc, 5); // skip bitstream id
+
+    /* volume control params */
+    for (int i = 0; i < (hdr->channel_mode ? 1 : 2); i++) {
+        hdr->dialog_normalization[i] = -get_bits(gbc, 5);
+        hdr->compression_exists[i] = get_bits1(gbc);
+        if (hdr->compression_exists[i])
+            hdr->heavy_dynamic_range[i] = get_bits(gbc, 8);
+    }
+
+    /* dependent stream channel map */
+    if (hdr->frame_type == EAC3_FRAME_TYPE_DEPENDENT) {
+        hdr->channel_map_present = get_bits1(gbc);
+        if (hdr->channel_map_present) {
+            int64_t channel_layout = 0;
+            int channel_map = get_bits(gbc, 16);
+
+            for (int i = 0; i < 16; i++)
+                if (channel_map & (1 << (EAC3_MAX_CHANNELS - i - 1)))
+                    channel_layout |= ff_eac3_custom_channel_map_locations[i][1];
+
+            if (av_popcount64(channel_layout) > EAC3_MAX_CHANNELS) {
+                return AC3_PARSE_ERROR_CHANNEL_MAP;
+            }
+            hdr->channel_map = channel_map;
+        }
+    }
+
+    /* mixing metadata */
+    if (get_bits1(gbc)) {
+        /* center and surround mix levels */
+        if (hdr->channel_mode > AC3_CHMODE_STEREO) {
+            hdr->preferred_downmix = get_bits(gbc, 2);
+            if (hdr->channel_mode & 1) {
+                /* if three front channels exist */
+                hdr->center_mix_level_ltrt = get_bits(gbc, 3);
+                hdr->center_mix_level      = get_bits(gbc, 3);
+            }
+            if (hdr->channel_mode & 4) {
+                /* if a surround channel exists */
+                hdr->surround_mix_level_ltrt = av_clip(get_bits(gbc, 3), 3, 7);
+                hdr->surround_mix_level      = av_clip(get_bits(gbc, 3), 3, 7);
+            }
+        }
+
+        /* lfe mix level */
+        if (hdr->lfe_on && (hdr->lfe_mix_level_exists = get_bits1(gbc))) {
+            hdr->lfe_mix_level = get_bits(gbc, 5);
+        }
+
+        /* info for mixing with other streams and substreams */
+        if (hdr->frame_type == EAC3_FRAME_TYPE_INDEPENDENT) {
+            for (int i = 0; i < (hdr->channel_mode ? 1 : 2); i++) {
+                // TODO: apply program scale factor
+                if (get_bits1(gbc)) {
+                    skip_bits(gbc, 6);  // skip program scale factor
+                }
+            }
+            if (get_bits1(gbc)) {
+                skip_bits(gbc, 6);  // skip external program scale factor
+            }
+            /* skip mixing parameter data */
+            switch(get_bits(gbc, 2)) {
+                case 1: skip_bits(gbc, 5);  break;
+                case 2: skip_bits(gbc, 12); break;
+                case 3: {
+                    int mix_data_size = (get_bits(gbc, 5) + 2) << 3;
+                    skip_bits_long(gbc, mix_data_size);
+                    break;
+                }
+            }
+            /* skip pan information for mono or dual mono source */
+            if (hdr->channel_mode < AC3_CHMODE_STEREO) {
+                for (int i = 0; i < (hdr->channel_mode ? 1 : 2); i++) {
+                    if (get_bits1(gbc)) {
+                        /* note: this is not in the ATSC A/52B specification
+                           reference: ETSI TS 102 366 V1.1.1
+                                      section: E.1.3.1.25 */
+                        skip_bits(gbc, 8);  // skip pan mean direction index
+                        skip_bits(gbc, 6);  // skip reserved paninfo bits
+                    }
+                }
+            }
+            /* skip mixing configuration information */
+            if (get_bits1(gbc)) {
+                for (int i = 0; i < hdr->num_blocks; i++) {
+                    if (hdr->num_blocks == 1 || get_bits1(gbc)) {
+                        skip_bits(gbc, 5);
+                    }
+                }
+            }
+        }
+    }
+
+    /* informational metadata */
+    if (get_bits1(gbc)) {
+        hdr->bitstream_mode = get_bits(gbc, 3);
+        skip_bits(gbc, 2); // skip copyright bit and original bitstream bit
+        if (hdr->channel_mode == AC3_CHMODE_STEREO) {
+            hdr->dolby_surround_mode  = get_bits(gbc, 2);
+            hdr->dolby_headphone_mode = get_bits(gbc, 2);
+        }
+        if (hdr->channel_mode >= AC3_CHMODE_2F2R) {
+            hdr->dolby_surround_ex_mode = get_bits(gbc, 2);
+        }
+        for (int i = 0; i < (hdr->channel_mode ? 1 : 2); i++) {
+            if (get_bits1(gbc)) {
+                skip_bits(gbc, 8); // skip mix level, room type, and A/D converter type
+            }
+        }
+        if (hdr->sr_code != EAC3_SR_CODE_REDUCED) {
+            skip_bits1(gbc); // skip source sample rate code
+        }
+    }
+
+    /* converter synchronization flag
+       If frames are less than six blocks, this bit should be turned on
+       once every 6 blocks to indicate the start of a frame set.
+       reference: RFC 4598, Section 2.1.3  Frame Sets */
+    if (hdr->frame_type == EAC3_FRAME_TYPE_INDEPENDENT && hdr->num_blocks != 6) {
+        skip_bits1(gbc); // skip converter synchronization flag
+    }
+
+    /* original frame size code if this stream was converted from AC-3 */
+    if (hdr->frame_type == EAC3_FRAME_TYPE_AC3_CONVERT &&
+            (hdr->num_blocks == 6 || get_bits1(gbc))) {
+        skip_bits(gbc, 6); // skip frame size code
+    }
+
+    /* additional bitstream info */
+    if (get_bits1(gbc)) {
+        int addbsil = get_bits(gbc, 6);
+        for (int i = 0; i < addbsil + 1; i++) {
+            if (i == 0) {
+                /* In this 8 bit chunk, the LSB is equal to flag_ec3_extension_type_a
+                   which can be used to detect Atmos presence */
+                skip_bits(gbc, 7);
+                hdr->eac3_extension_type_a = get_bits1(gbc);
+                if (hdr->eac3_extension_type_a) {
+                    hdr->complexity_index_type_a = get_bits(gbc, 8);
+                    i++;
+                }
+            } else {
+                skip_bits(gbc, 8); // skip additional bit stream info
+            }
+        }
+    }
+
+    return 0;
+}
+
 int ff_ac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr)
 {
     int frame_size_code;
@@ -133,6 +344,10 @@ int ff_ac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr)
         hdr->frame_size = ff_ac3_frame_size_tab[frame_size_code][hdr->sr_code] * 2;
         hdr->frame_type = EAC3_FRAME_TYPE_AC3_CONVERT; //EAC3_FRAME_TYPE_INDEPENDENT;
         hdr->substreamid = 0;
+
+        int ret = ac3_parse_header(gbc, hdr);
+        if (ret < 0)
+            return ret;
     } else {
         /* Enhanced AC-3 */
         hdr->crc1 = 0;
@@ -165,6 +380,10 @@ int ff_ac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr)
         hdr->bit_rate = 8LL * hdr->frame_size * hdr->sample_rate /
                         (hdr->num_blocks * 256);
         hdr->channels = ff_ac3_channels_tab[hdr->channel_mode] + hdr->lfe_on;
+
+        int ret = eac3_parse_header(gbc, hdr);
+        if (ret < 0)
+            return ret;
     }
     hdr->channel_layout = ff_ac3_channel_layout_tab[hdr->channel_mode];
     if (hdr->lfe_on)
diff --git a/libavcodec/ac3_parser_internal.h b/libavcodec/ac3_parser_internal.h
index 46814bfb1f..ab5df34003 100644
--- a/libavcodec/ac3_parser_internal.h
+++ b/libavcodec/ac3_parser_internal.h
@@ -46,6 +46,7 @@ typedef struct AC3HeaderInfo {
     int substreamid;                        ///< substream identification
     int center_mix_level;                   ///< Center mix level index
     int surround_mix_level;                 ///< Surround mix level index
+    uint8_t channel_map_present;
     uint16_t channel_map;
     int num_blocks;                         ///< number of audio blocks
     int dolby_surround_mode;
@@ -62,6 +63,23 @@ typedef struct AC3HeaderInfo {
     uint64_t channel_layout;
     int8_t ac3_bit_rate_code;
     /** @} */
+
+    /** @name enhanced eac3 extension coded elements
+     * @{
+     */
+    int8_t dialog_normalization[2];
+    uint8_t compression_exists[2];
+    uint8_t heavy_dynamic_range[2];
+    uint8_t center_mix_level_ltrt;              ///< Center mix level index
+    uint8_t surround_mix_level_ltrt;            ///< Surround mix level index
+    uint8_t dolby_headphone_mode;
+    uint8_t dolby_surround_ex_mode;
+    uint8_t lfe_mix_level_exists;
+    uint8_t lfe_mix_level;
+    uint8_t preferred_downmix;
+    uint8_t eac3_extension_type_a;
+    uint8_t complexity_index_type_a;
+    /** @} */
 } AC3HeaderInfo;
 
 typedef enum {
@@ -71,6 +89,7 @@ typedef enum {
     AC3_PARSE_ERROR_FRAME_SIZE  = -0x4030c0a,
     AC3_PARSE_ERROR_FRAME_TYPE  = -0x5030c0a,
     AC3_PARSE_ERROR_CRC         = -0x6030c0a,
+    AC3_PARSE_ERROR_CHANNEL_MAP = -0x7030c0a,
 } AC3ParseError;
 
 /**
diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
index 5eacab4475..2f0e7f5344 100644
--- a/libavcodec/ac3dec.c
+++ b/libavcodec/ac3dec.c
@@ -156,72 +156,6 @@ static av_cold void ac3_decode_flush(AVCodecContext *avctx)
     av_lfg_init(&s->dith_state, 0);
 }
 
-/**
- * Parse the 'sync info' and 'bit stream info' from the AC-3 bitstream.
- * GetBitContext within AC3DecodeContext must point to
- * the start of the synchronized AC-3 bitstream.
- */
-static int ac3_parse_header(AC3DecodeContext *s)
-{
-    GetBitContext *gbc = &s->gbc;
-    int i;
-
-    /* read the rest of the bsi. read twice for dual mono mode. */
-    i = !s->channel_mode;
-    do {
-        s->dialog_normalization[(!s->channel_mode)-i] = -get_bits(gbc, 5);
-        if (s->dialog_normalization[(!s->channel_mode)-i] == 0) {
-            s->dialog_normalization[(!s->channel_mode)-i] = -31;
-        }
-        if (s->target_level != 0) {
-            s->level_gain[(!s->channel_mode)-i] = powf(2.0f,
-                (float)(s->target_level -
-                s->dialog_normalization[(!s->channel_mode)-i])/6.0f);
-        }
-        if (s->compression_exists[(!s->channel_mode)-i] = get_bits1(gbc)) {
-            s->heavy_dynamic_range[(!s->channel_mode)-i] =
-                AC3_HEAVY_RANGE(get_bits(gbc, 8));
-        }
-        if (get_bits1(gbc))
-            skip_bits(gbc, 8); //skip language code
-        if (get_bits1(gbc))
-            skip_bits(gbc, 7); //skip audio production information
-    } while (i--);
-
-    skip_bits(gbc, 2); //skip copyright bit and original bitstream bit
-
-    /* skip the timecodes or parse the Alternate Bit Stream Syntax */
-    if (s->bitstream_id != 6) {
-        if (get_bits1(gbc))
-            skip_bits(gbc, 14); //skip timecode1
-        if (get_bits1(gbc))
-            skip_bits(gbc, 14); //skip timecode2
-    } else {
-        if (get_bits1(gbc)) {
-            s->preferred_downmix       = get_bits(gbc, 2);
-            s->center_mix_level_ltrt   = get_bits(gbc, 3);
-            s->surround_mix_level_ltrt = av_clip(get_bits(gbc, 3), 3, 7);
-            s->center_mix_level        = get_bits(gbc, 3);
-            s->surround_mix_level      = av_clip(get_bits(gbc, 3), 3, 7);
-        }
-        if (get_bits1(gbc)) {
-            s->dolby_surround_ex_mode = get_bits(gbc, 2);
-            s->dolby_headphone_mode   = get_bits(gbc, 2);
-            skip_bits(gbc, 10); // skip adconvtyp (1), xbsi2 (8), encinfo (1)
-        }
-    }
-
-    /* skip additional bitstream info */
-    if (get_bits1(gbc)) {
-        i = get_bits(gbc, 6);
-        do {
-            skip_bits(gbc, 8);
-        } while (i--);
-    }
-
-    return 0;
-}
-
 /**
  * Common function to parse AC-3 or E-AC-3 frame header
  */
@@ -281,10 +215,25 @@ static int parse_frame_header(AC3DecodeContext *s)
         s->dba_syntax            = 1;
         s->skip_syntax           = 1;
         memset(s->channel_uses_aht, 0, sizeof(s->channel_uses_aht));
-        return ac3_parse_header(s);
+        /* volume control params */
+        for (int i = 0; i < (s->channel_mode ? 1 : 2); i++) {
+            s->dialog_normalization[i] = hdr.dialog_normalization[i];
+            if (s->dialog_normalization[i] == 0) {
+                s->dialog_normalization[i] = -31;
+            }
+            if (s->target_level != 0) {
+                s->level_gain[i] = powf(2.0f,
+                    (float)(s->target_level - s->dialog_normalization[i])/6.0f);
+            }
+            s->compression_exists[i] = hdr.compression_exists[i];
+            if (s->compression_exists[i]) {
+                s->heavy_dynamic_range[i] = AC3_HEAVY_RANGE(hdr.heavy_dynamic_range[i]);
+            }
+        }
+        return 0;
     } else if (CONFIG_EAC3_DECODER) {
         s->eac3 = 1;
-        return ff_eac3_parse_header(s);
+        return ff_eac3_parse_header(s, &hdr);
     } else {
         av_log(s->avctx, AV_LOG_ERROR, "E-AC-3 support not compiled in\n");
         return AVERROR(ENOSYS);
@@ -1469,6 +1418,9 @@ dependent_frame:
                 av_log(avctx, AV_LOG_ERROR, "invalid frame type\n");
             }
             break;
+        case AC3_PARSE_ERROR_CHANNEL_MAP:
+            av_log(avctx, AV_LOG_ERROR, "invalid channel map\n");
+            return AVERROR_INVALIDDATA;
         case AC3_PARSE_ERROR_CRC:
             break;
         default: // Normal AVERROR do not try to recover.
diff --git a/libavcodec/ac3dec.h b/libavcodec/ac3dec.h
index 4042a99b80..a099264475 100644
--- a/libavcodec/ac3dec.h
+++ b/libavcodec/ac3dec.h
@@ -260,11 +260,13 @@ typedef struct AC3DecodeContext {
 ///@}
 } AC3DecodeContext;
 
+struct AC3HeaderInfo;
+
 /**
  * Parse the E-AC-3 frame header.
  * This parses both the bit stream info and audio frame header.
  */
-static int ff_eac3_parse_header(AC3DecodeContext *s);
+static int ff_eac3_parse_header(AC3DecodeContext *s, const struct AC3HeaderInfo *hdr);
 
 /**
  * Decode mantissas in a single channel for the entire frame.
diff --git a/libavcodec/ac3defs.h b/libavcodec/ac3defs.h
index f9b1be059f..8d6a58d21f 100644
--- a/libavcodec/ac3defs.h
+++ b/libavcodec/ac3defs.h
@@ -34,6 +34,8 @@
 #define AC3_CRITICAL_BANDS 50
 #define AC3_MAX_CPL_BANDS  18
 
+#define EAC3_SR_CODE_REDUCED 3
+
 /* pre-defined gain values */
 #define LEVEL_PLUS_3DB          M_SQRT2
 #define LEVEL_PLUS_1POINT5DB    1.1892071150027209
diff --git a/libavcodec/eac3dec.c b/libavcodec/eac3dec.c
index 2b3bffda6e..c5095b1917 100644
--- a/libavcodec/eac3dec.c
+++ b/libavcodec/eac3dec.c
@@ -53,8 +53,6 @@ typedef enum {
     EAC3_GAQ_124
 } EAC3GaqMode;
 
-#define EAC3_SR_CODE_REDUCED  3
-
 static void ff_eac3_apply_spectral_extension(AC3DecodeContext *s)
 {
     int bin, bnd, ch, i;
@@ -287,7 +285,7 @@ static void ff_eac3_decode_transform_coeffs_aht_ch(AC3DecodeContext *s, int ch)
     }
 }
 
-static int ff_eac3_parse_header(AC3DecodeContext *s)
+static int ff_eac3_parse_header(AC3DecodeContext *s, const AC3HeaderInfo *hdr)
 {
     int i, blk, ch;
     int ac3_exponent_strategy, parse_aht_info, parse_spx_atten_data;
@@ -323,11 +321,10 @@ static int ff_eac3_parse_header(AC3DecodeContext *s)
         avpriv_request_sample(s->avctx, "Reduced sampling rate");
         return AVERROR_PATCHWELCOME;
     }
-    skip_bits(gbc, 5); // skip bitstream id
 
     /* volume control params */
     for (i = 0; i < (s->channel_mode ? 1 : 2); i++) {
-        s->dialog_normalization[i] = -get_bits(gbc, 5);
+        s->dialog_normalization[i] = hdr->dialog_normalization[i];
         if (s->dialog_normalization[i] == 0) {
             s->dialog_normalization[i] = -31;
         }
@@ -335,147 +332,30 @@ static int ff_eac3_parse_header(AC3DecodeContext *s)
             s->level_gain[i] = powf(2.0f,
                 (float)(s->target_level - s->dialog_normalization[i])/6.0f);
         }
-        s->compression_exists[i] = get_bits1(gbc);
-        if (s->compression_exists[i]) {
-            s->heavy_dynamic_range[i] = AC3_HEAVY_RANGE(get_bits(gbc, 8));
+        if (hdr->compression_exists[i]) {
+            s->heavy_dynamic_range[i] = AC3_HEAVY_RANGE(hdr->heavy_dynamic_range[i]);
         }
     }
 
-    /* dependent stream channel map */
-    if (s->frame_type == EAC3_FRAME_TYPE_DEPENDENT) {
-        if (get_bits1(gbc)) {
-            int64_t channel_layout = 0;
-            int channel_map = get_bits(gbc, 16);
-            av_log(s->avctx, AV_LOG_DEBUG, "channel_map: %0X\n", channel_map);
-
-            for (i = 0; i < 16; i++)
-                if (channel_map & (1 << (EAC3_MAX_CHANNELS - i - 1)))
-                    channel_layout |= ff_eac3_custom_channel_map_locations[i][1];
-
-            if (av_popcount64(channel_layout) > EAC3_MAX_CHANNELS) {
-                return AVERROR_INVALIDDATA;
-            }
-            s->channel_map = channel_map;
-        }
-    }
+    s->channel_map = hdr->channel_map;
 
     /* mixing metadata */
-    if (get_bits1(gbc)) {
-        /* center and surround mix levels */
-        if (s->channel_mode > AC3_CHMODE_STEREO) {
-            s->preferred_downmix = get_bits(gbc, 2);
-            if (s->channel_mode & 1) {
-                /* if three front channels exist */
-                s->center_mix_level_ltrt = get_bits(gbc, 3);
-                s->center_mix_level      = get_bits(gbc, 3);
-            }
-            if (s->channel_mode & 4) {
-                /* if a surround channel exists */
-                s->surround_mix_level_ltrt = av_clip(get_bits(gbc, 3), 3, 7);
-                s->surround_mix_level      = av_clip(get_bits(gbc, 3), 3, 7);
-            }
-        }
-
-        /* lfe mix level */
-        if (s->lfe_on && (s->lfe_mix_level_exists = get_bits1(gbc))) {
-            s->lfe_mix_level = get_bits(gbc, 5);
-        }
-
-        /* info for mixing with other streams and substreams */
-        if (s->frame_type == EAC3_FRAME_TYPE_INDEPENDENT) {
-            for (i = 0; i < (s->channel_mode ? 1 : 2); i++) {
-                // TODO: apply program scale factor
-                if (get_bits1(gbc)) {
-                    skip_bits(gbc, 6);  // skip program scale factor
-                }
-            }
-            if (get_bits1(gbc)) {
-                skip_bits(gbc, 6);  // skip external program scale factor
-            }
-            /* skip mixing parameter data */
-            switch(get_bits(gbc, 2)) {
-                case 1: skip_bits(gbc, 5);  break;
-                case 2: skip_bits(gbc, 12); break;
-                case 3: {
-                    int mix_data_size = (get_bits(gbc, 5) + 2) << 3;
-                    skip_bits_long(gbc, mix_data_size);
-                    break;
-                }
-            }
-            /* skip pan information for mono or dual mono source */
-            if (s->channel_mode < AC3_CHMODE_STEREO) {
-                for (i = 0; i < (s->channel_mode ? 1 : 2); i++) {
-                    if (get_bits1(gbc)) {
-                        /* note: this is not in the ATSC A/52B specification
-                           reference: ETSI TS 102 366 V1.1.1
-                                      section: E.1.3.1.25 */
-                        skip_bits(gbc, 8);  // skip pan mean direction index
-                        skip_bits(gbc, 6);  // skip reserved paninfo bits
-                    }
-                }
-            }
-            /* skip mixing configuration information */
-            if (get_bits1(gbc)) {
-                for (blk = 0; blk < s->num_blocks; blk++) {
-                    if (s->num_blocks == 1 || get_bits1(gbc)) {
-                        skip_bits(gbc, 5);
-                    }
-                }
-            }
-        }
-    }
+    s->preferred_downmix       = hdr->preferred_downmix;
+    s->center_mix_level_ltrt   = hdr->center_mix_level_ltrt;
+    s->center_mix_level        = hdr->center_mix_level;
+    s->surround_mix_level_ltrt = hdr->surround_mix_level_ltrt;
+    s->surround_mix_level      = hdr->surround_mix_level;
+    s->lfe_mix_level_exists    = hdr->lfe_mix_level_exists;
+    s->lfe_mix_level           = hdr->lfe_mix_level;
+    s->dolby_surround_mode     = hdr->dolby_surround_mode;
+    s->dolby_headphone_mode    = hdr->dolby_headphone_mode;
+    s->dolby_surround_ex_mode  = hdr->dolby_surround_ex_mode;
 
     /* informational metadata */
-    if (get_bits1(gbc)) {
-        s->bitstream_mode = get_bits(gbc, 3);
-        skip_bits(gbc, 2); // skip copyright bit and original bitstream bit
-        if (s->channel_mode == AC3_CHMODE_STEREO) {
-            s->dolby_surround_mode  = get_bits(gbc, 2);
-            s->dolby_headphone_mode = get_bits(gbc, 2);
-        }
-        if (s->channel_mode >= AC3_CHMODE_2F2R) {
-            s->dolby_surround_ex_mode = get_bits(gbc, 2);
-        }
-        for (i = 0; i < (s->channel_mode ? 1 : 2); i++) {
-            if (get_bits1(gbc)) {
-                skip_bits(gbc, 8); // skip mix level, room type, and A/D converter type
-            }
-        }
-        if (s->bit_alloc_params.sr_code != EAC3_SR_CODE_REDUCED) {
-            skip_bits1(gbc); // skip source sample rate code
-        }
-    }
-
-    /* converter synchronization flag
-       If frames are less than six blocks, this bit should be turned on
-       once every 6 blocks to indicate the start of a frame set.
-       reference: RFC 4598, Section 2.1.3  Frame Sets */
-    if (s->frame_type == EAC3_FRAME_TYPE_INDEPENDENT && s->num_blocks != 6) {
-        skip_bits1(gbc); // skip converter synchronization flag
-    }
-
-    /* original frame size code if this stream was converted from AC-3 */
-    if (s->frame_type == EAC3_FRAME_TYPE_AC3_CONVERT &&
-            (s->num_blocks == 6 || get_bits1(gbc))) {
-        skip_bits(gbc, 6); // skip frame size code
-    }
+    s->bitstream_mode = hdr->bitstream_mode;
 
     /* additional bitstream info */
-    if (get_bits1(gbc)) {
-        int addbsil = get_bits(gbc, 6);
-        for (i = 0; i < addbsil + 1; i++) {
-            if (i == 0) {
-                /* In this 8 bit chunk, the LSB is equal to flag_ec3_extension_type_a
-                   which can be used to detect Atmos presence */
-                skip_bits(gbc, 7);
-                if (get_bits1(gbc)) {
-                    s->eac3_extension_type_a = 1;
-                }
-            } else {
-                skip_bits(gbc, 8); // skip additional bit stream info
-            }
-        }
-    }
+    s->eac3_extension_type_a = hdr->eac3_extension_type_a;
 
     /* audio frame syntax flags, strategy data, and per-frame data */
 
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 402611e81e..5941f82521 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -532,8 +532,6 @@ static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
             int parent = hdr->substreamid;
 
             while (cumul_size != pkt->size) {
-                GetBitContext gbc;
-                int i;
                 ret = avpriv_ac3_parse_header(&hdr, pkt->data + cumul_size, pkt->size - cumul_size);
                 if (ret < 0)
                     goto end;
@@ -544,20 +542,9 @@ static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
                 info->substream[parent].num_dep_sub++;
                 ret /= 8;
 
-                /* header is parsed up to lfeon, but custom channel map may be needed */
-                init_get_bits8(&gbc, pkt->data + cumul_size + ret, pkt->size - cumul_size - ret);
-                /* skip bsid */
-                skip_bits(&gbc, 5);
-                /* skip volume control params */
-                for (i = 0; i < (hdr->channel_mode ? 1 : 2); i++) {
-                    skip_bits(&gbc, 5); // skip dialog normalization
-                    if (get_bits1(&gbc)) {
-                        skip_bits(&gbc, 8); // skip compression gain word
-                    }
-                }
                 /* get the dependent stream channel map, if exists */
-                if (get_bits1(&gbc))
-                    info->substream[parent].chan_loc |= (get_bits(&gbc, 16) >> 5) & 0x1f;
+                if (hdr->channel_map_present)
+                    info->substream[parent].chan_loc |= (hdr->channel_map >> 5) & 0x1f;
                 else
                     info->substream[parent].chan_loc |= hdr->channel_mode;
                 cumul_size += hdr->frame_size;
-- 
2.49.0

_______________________________________________
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 v2 3/4] avformat/movenc: fix writing reserved bits in EC3SpecificBox

[James Almer]

As described in section F.6.1 from ETSI TS 102 366.

Found-by: nyanmisaka
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/movenc.c         | 2 +-
 tests/ref/fate/copy-trac3074 | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 5941f82521..7bcdc2463d 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -618,7 +618,7 @@ static int mov_write_eac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra
         put_bits(&pbc, 3, info->substream[i].bsmod);
         put_bits(&pbc, 3, info->substream[i].acmod);
         put_bits(&pbc, 1, info->substream[i].lfeon);
-        put_bits(&pbc, 5, 0); /* reserved */
+        put_bits(&pbc, 3, 0); /* reserved */
         put_bits(&pbc, 4, info->substream[i].num_dep_sub);
         if (!info->substream[i].num_dep_sub) {
             put_bits(&pbc, 1, 0); /* reserved */
diff --git a/tests/ref/fate/copy-trac3074 b/tests/ref/fate/copy-trac3074
index 53ba27e920..9ed42c8d8d 100644
--- a/tests/ref/fate/copy-trac3074
+++ b/tests/ref/fate/copy-trac3074
@@ -1,5 +1,5 @@
-36fcc0a62695bcf93068fcfe68283ee9 *tests/data/fate/copy-trac3074.mp4
-334016 tests/data/fate/copy-trac3074.mp4
+5b4a3ed9de3b2a92e5dcb127bca12e68 *tests/data/fate/copy-trac3074.mp4
+334015 tests/data/fate/copy-trac3074.mp4
 #tb 0: 1/48000
 #media_type 0: audio
 #codec_id 0: eac3
-- 
2.49.0

_______________________________________________
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 v2 3/4] avformat/movenc: fix writing reserved bits in EC3SpecificBox

[Baptiste Coudurier]

> On Jun 5, 2025, at 5:14 PM, James Almer <jamrial@gmail.com> wrote:
> 
> As described in section F.6.1 from ETSI TS 102 366.
> 
> Found-by: nyanmisaka
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavformat/movenc.c         | 2 +-
> tests/ref/fate/copy-trac3074 | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 5941f82521..7bcdc2463d 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -618,7 +618,7 @@ static int mov_write_eac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra
>         put_bits(&pbc, 3, info->substream[i].bsmod);
>         put_bits(&pbc, 3, info->substream[i].acmod);
>         put_bits(&pbc, 1, info->substream[i].lfeon);
> -        put_bits(&pbc, 5, 0); /* reserved */
> +        put_bits(&pbc, 3, 0); /* reserved */
>         put_bits(&pbc, 4, info->substream[i].num_dep_sub);
>         if (!info->substream[i].num_dep_sub) {
>             put_bits(&pbc, 1, 0); /* reserved */
> diff --git a/tests/ref/fate/copy-trac3074 b/tests/ref/fate/copy-trac3074
> index 53ba27e920..9ed42c8d8d 100644
> --- a/tests/ref/fate/copy-trac3074
> +++ b/tests/ref/fate/copy-trac3074
> @@ -1,5 +1,5 @@
> -36fcc0a62695bcf93068fcfe68283ee9 *tests/data/fate/copy-trac3074.mp4
> -334016 tests/data/fate/copy-trac3074.mp4
> +5b4a3ed9de3b2a92e5dcb127bca12e68 *tests/data/fate/copy-trac3074.mp4
> +334015 tests/data/fate/copy-trac3074.mp4
> #tb 0: 1/48000
> #media_type 0: audio
> #codec_id 0: eac3

Looks good, you might wanna update the size of the buffer too in this commit, 
or do it in the atmos patch I guess

— 
Baptiste

_______________________________________________
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 v2 4/4] avformat/movenc: handle EAC-3 extension bits for Atmos

[James Almer]

From: nyanmisaka <nst799610810@gmail.com>

Fixes commit #9996.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/movenc.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 7bcdc2463d..cd5b45f6fe 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -393,6 +393,8 @@ struct eac3_info {
         uint16_t chan_loc;
         /* if there is no dependent substream, then one bit reserved instead */
     } substream[1]; /* TODO: support 8 independent substreams */
+    /* indicates the decoding complexity, 8 bits */
+    uint8_t complexity_index_type_a;
 };
 
 static int mov_write_ac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track)
@@ -474,6 +476,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);
+    info->complexity_index_type_a = hdr->complexity_index_type_a;
+
     num_blocks = hdr->num_blocks;
 
     if (!info->ec3_done) {
@@ -601,7 +605,7 @@ static int mov_write_eac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra
     }
 
     info = track->eac3_priv;
-    size = 2 + ((34 * (info->num_ind_sub + 1) + 7) >> 3);
+    size = 2 + (4 * (info->num_ind_sub + 1)) + (2 * !!info->complexity_index_type_a);
     buf = av_malloc(size);
     if (!buf) {
         return AVERROR(ENOMEM);
@@ -626,6 +630,11 @@ static int mov_write_eac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra
             put_bits(&pbc, 9, info->substream[i].chan_loc);
         }
     }
+    if (info->complexity_index_type_a) {
+        put_bits(&pbc, 7, 0); /* reserved */
+        put_bits(&pbc, 1, 1); // flag_eac3_extension_type_a
+        put_bits(&pbc, 8, info->complexity_index_type_a);
+    }
     flush_put_bits(&pbc);
     size = put_bytes_output(&pbc);
 
-- 
2.49.0

_______________________________________________
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 v2 4/4] avformat/movenc: handle EAC-3 extension bits for Atmos

[Baptiste Coudurier]

> On Jun 5, 2025, at 5:14 PM, James Almer <jamrial@gmail.com> wrote:
> 
> From: nyanmisaka <nst799610810@gmail.com>
> 
> Fixes commit #9996.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavformat/movenc.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 7bcdc2463d..cd5b45f6fe 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -393,6 +393,8 @@ struct eac3_info {
>         uint16_t chan_loc;
>         /* if there is no dependent substream, then one bit reserved instead */
>     } substream[1]; /* TODO: support 8 independent substreams */
> +    /* indicates the decoding complexity, 8 bits */
> +    uint8_t complexity_index_type_a;
> };
> 
> static int mov_write_ac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track)
> @@ -474,6 +476,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);
> +    info->complexity_index_type_a = hdr->complexity_index_type_a;
> +
>     num_blocks = hdr->num_blocks;
> 
>     if (!info->ec3_done) {
> @@ -601,7 +605,7 @@ static int mov_write_eac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra
>     }
> 
>     info = track->eac3_priv;
> -    size = 2 + ((34 * (info->num_ind_sub + 1) + 7) >> 3);
> +    size = 2 + (4 * (info->num_ind_sub + 1)) + (2 * !!info->complexity_index_type_a);
>     buf = av_malloc(size);
>     if (!buf) {
>         return AVERROR(ENOMEM);
> @@ -626,6 +630,11 @@ static int mov_write_eac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra
>             put_bits(&pbc, 9, info->substream[i].chan_loc);
>         }
>     }
> +    if (info->complexity_index_type_a) {
> +        put_bits(&pbc, 7, 0); /* reserved */
> +        put_bits(&pbc, 1, 1); // flag_eac3_extension_type_a
> +        put_bits(&pbc, 8, info->complexity_index_type_a);
> +    }
>     flush_put_bits(&pbc);
>     size = put_bytes_output(&pbc);

Looks good, confirmed that Quicktime recognizes Atmos after the changes

— 
Baptiste

_______________________________________________
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 1/4] avcodec/ac3_parser: use a padded buffer in av_ac3_parse_header()

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 1474 bytes --]

On 6/5/2025 9:14 PM, James Almer wrote:
> The GetBitContext API requires the buffer to be padded, and the documentation for
> av_ac3_parse_header() does not specify it, so use a temporary local buffer.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>   libavcodec/ac3_parser.c | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
> index 69989690dd..9065d700e2 100644
> --- a/libavcodec/ac3_parser.c
> +++ b/libavcodec/ac3_parser.c
> @@ -202,14 +202,24 @@ int av_ac3_parse_header(const uint8_t *buf, size_t size,
>   {
>       GetBitContext gb;
>       AC3HeaderInfo hdr;
> +    uint8_t *tmp = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
>       int err;
>   
> -    err = init_get_bits8(&gb, buf, size);
> -    if (err < 0)
> +    if (!tmp)
> +        return AVERROR(ENOMEM);
> +
> +    memcpy(tmp, buf, size);
> +    memset(tmp + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
> +    err = init_get_bits8(&gb, tmp, size);
> +    if (err < 0) {
> +        av_free(tmp);
>           return AVERROR_INVALIDDATA;
> +    }
>       err = ff_ac3_parse_header(&gb, &hdr);
> -    if (err < 0)
> +    if (err < 0) {
> +        av_free(tmp);
>           return AVERROR_INVALIDDATA;
> +    }
>   
>       *bitstream_id = hdr.bitstream_id;
>       *frame_size   = hdr.frame_size;

Added the missing av_free(tmp) in case of success locally...


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 1/4] avcodec/ac3_parser: use a padded buffer in av_ac3_parse_header()

[Andreas Rheinhardt]

James Almer:
> The GetBitContext API requires the buffer to be padded, and the documentation for
> av_ac3_parse_header() does not specify it, so use a temporary local buffer.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/ac3_parser.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
> index 69989690dd..9065d700e2 100644
> --- a/libavcodec/ac3_parser.c
> +++ b/libavcodec/ac3_parser.c
> @@ -202,14 +202,24 @@ int av_ac3_parse_header(const uint8_t *buf, size_t size,
>  {
>      GetBitContext gb;
>      AC3HeaderInfo hdr;
> +    uint8_t *tmp = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
>      int err;
>  
> -    err = init_get_bits8(&gb, buf, size);
> -    if (err < 0)
> +    if (!tmp)
> +        return AVERROR(ENOMEM);
> +
> +    memcpy(tmp, buf, size);
> +    memset(tmp + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
> +    err = init_get_bits8(&gb, tmp, size);
> +    if (err < 0) {
> +        av_free(tmp);
>          return AVERROR_INVALIDDATA;
> +    }
>      err = ff_ac3_parse_header(&gb, &hdr);
> -    if (err < 0)
> +    if (err < 0) {
> +        av_free(tmp);
>          return AVERROR_INVALIDDATA;
> +    }
>  
>      *bitstream_id = hdr.bitstream_id;
>      *frame_size   = hdr.frame_size;

There is no need for an allocation here; (E)AC-3 frames have a bounded
size and the number of bytes read by ff_ac3_parse_header() is even smaller.
Anyway: The buffer leaks on success.

- Andreas

_______________________________________________
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 1/4] avcodec/ac3_parser: use a padded buffer in av_ac3_parse_header()

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 2058 bytes --]

On 6/5/2025 9:29 PM, Andreas Rheinhardt wrote:
> James Almer:
>> The GetBitContext API requires the buffer to be padded, and the documentation for
>> av_ac3_parse_header() does not specify it, so use a temporary local buffer.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/ac3_parser.c | 16 +++++++++++++---
>>   1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
>> index 69989690dd..9065d700e2 100644
>> --- a/libavcodec/ac3_parser.c
>> +++ b/libavcodec/ac3_parser.c
>> @@ -202,14 +202,24 @@ int av_ac3_parse_header(const uint8_t *buf, size_t size,
>>   {
>>       GetBitContext gb;
>>       AC3HeaderInfo hdr;
>> +    uint8_t *tmp = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
>>       int err;
>>   
>> -    err = init_get_bits8(&gb, buf, size);
>> -    if (err < 0)
>> +    if (!tmp)
>> +        return AVERROR(ENOMEM);
>> +
>> +    memcpy(tmp, buf, size);
>> +    memset(tmp + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>> +    err = init_get_bits8(&gb, tmp, size);
>> +    if (err < 0) {
>> +        av_free(tmp);
>>           return AVERROR_INVALIDDATA;
>> +    }
>>       err = ff_ac3_parse_header(&gb, &hdr);
>> -    if (err < 0)
>> +    if (err < 0) {
>> +        av_free(tmp);
>>           return AVERROR_INVALIDDATA;
>> +    }
>>   
>>       *bitstream_id = hdr.bitstream_id;
>>       *frame_size   = hdr.frame_size;
> 
> There is no need for an allocation here; (E)AC-3 frames have a bounded
> size and the number of bytes read by ff_ac3_parse_header() is even smaller.

What's the max size? Is there a define for it?

> Anyway: The buffer leaks on success.

I'm aware, i said as much in a reply :p

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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 1/4] avcodec/ac3_parser: use a padded buffer in av_ac3_parse_header()

[Andreas Rheinhardt]

James Almer:
> On 6/5/2025 9:29 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> The GetBitContext API requires the buffer to be padded, and the
>>> documentation for
>>> av_ac3_parse_header() does not specify it, so use a temporary local
>>> buffer.
>>>
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>>   libavcodec/ac3_parser.c | 16 +++++++++++++---
>>>   1 file changed, 13 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
>>> index 69989690dd..9065d700e2 100644
>>> --- a/libavcodec/ac3_parser.c
>>> +++ b/libavcodec/ac3_parser.c
>>> @@ -202,14 +202,24 @@ int av_ac3_parse_header(const uint8_t *buf,
>>> size_t size,
>>>   {
>>>       GetBitContext gb;
>>>       AC3HeaderInfo hdr;
>>> +    uint8_t *tmp = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
>>>       int err;
>>>   -    err = init_get_bits8(&gb, buf, size);
>>> -    if (err < 0)
>>> +    if (!tmp)
>>> +        return AVERROR(ENOMEM);
>>> +
>>> +    memcpy(tmp, buf, size);
>>> +    memset(tmp + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>>> +    err = init_get_bits8(&gb, tmp, size);
>>> +    if (err < 0) {
>>> +        av_free(tmp);
>>>           return AVERROR_INVALIDDATA;
>>> +    }
>>>       err = ff_ac3_parse_header(&gb, &hdr);
>>> -    if (err < 0)
>>> +    if (err < 0) {
>>> +        av_free(tmp);
>>>           return AVERROR_INVALIDDATA;
>>> +    }
>>>         *bitstream_id = hdr.bitstream_id;
>>>       *frame_size   = hdr.frame_size;
>>
>> There is no need for an allocation here; (E)AC-3 frames have a bounded
>> size and the number of bytes read by ff_ac3_parse_header() is even
>> smaller.
> 
> What's the max size? Is there a define for it?

Not yet.

> 
>> Anyway: The buffer leaks on success.
> 
> I'm aware, i said as much in a reply :p
> 

Should have read all your mails.

- Andreas

_______________________________________________
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 1/4] avcodec/ac3_parser: use a padded buffer in av_ac3_parse_header()

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 2469 bytes --]

On 6/5/2025 9:32 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 6/5/2025 9:29 PM, Andreas Rheinhardt wrote:
>>> James Almer:
>>>> The GetBitContext API requires the buffer to be padded, and the
>>>> documentation for
>>>> av_ac3_parse_header() does not specify it, so use a temporary local
>>>> buffer.
>>>>
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>>    libavcodec/ac3_parser.c | 16 +++++++++++++---
>>>>    1 file changed, 13 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
>>>> index 69989690dd..9065d700e2 100644
>>>> --- a/libavcodec/ac3_parser.c
>>>> +++ b/libavcodec/ac3_parser.c
>>>> @@ -202,14 +202,24 @@ int av_ac3_parse_header(const uint8_t *buf,
>>>> size_t size,
>>>>    {
>>>>        GetBitContext gb;
>>>>        AC3HeaderInfo hdr;
>>>> +    uint8_t *tmp = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
>>>>        int err;
>>>>    -    err = init_get_bits8(&gb, buf, size);
>>>> -    if (err < 0)
>>>> +    if (!tmp)
>>>> +        return AVERROR(ENOMEM);
>>>> +
>>>> +    memcpy(tmp, buf, size);
>>>> +    memset(tmp + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>>>> +    err = init_get_bits8(&gb, tmp, size);
>>>> +    if (err < 0) {
>>>> +        av_free(tmp);
>>>>            return AVERROR_INVALIDDATA;
>>>> +    }
>>>>        err = ff_ac3_parse_header(&gb, &hdr);
>>>> -    if (err < 0)
>>>> +    if (err < 0) {
>>>> +        av_free(tmp);
>>>>            return AVERROR_INVALIDDATA;
>>>> +    }
>>>>          *bitstream_id = hdr.bitstream_id;
>>>>        *frame_size   = hdr.frame_size;
>>>
>>> There is no need for an allocation here; (E)AC-3 frames have a bounded
>>> size and the number of bytes read by ff_ac3_parse_header() is even
>>> smaller.
>>
>> What's the max size? Is there a define for it?
> 
> Not yet.

Ok, what size should the static buffer be?

> 
>>
>>> Anyway: The buffer leaks on success.
>>
>> I'm aware, i said as much in a reply :p
>>
> 
> Should have read all your mails.
> 
> - Andreas
> 
> _______________________________________________
> 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".


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 1/4] avcodec/ac3_parser: use a padded buffer in av_ac3_parse_header()

[Andreas Rheinhardt]

James Almer:
> On 6/5/2025 9:32 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 6/5/2025 9:29 PM, Andreas Rheinhardt wrote:
>>>> James Almer:
>>>>> The GetBitContext API requires the buffer to be padded, and the
>>>>> documentation for
>>>>> av_ac3_parse_header() does not specify it, so use a temporary local
>>>>> buffer.
>>>>>
>>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>>> ---
>>>>>    libavcodec/ac3_parser.c | 16 +++++++++++++---
>>>>>    1 file changed, 13 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
>>>>> index 69989690dd..9065d700e2 100644
>>>>> --- a/libavcodec/ac3_parser.c
>>>>> +++ b/libavcodec/ac3_parser.c
>>>>> @@ -202,14 +202,24 @@ int av_ac3_parse_header(const uint8_t *buf,
>>>>> size_t size,
>>>>>    {
>>>>>        GetBitContext gb;
>>>>>        AC3HeaderInfo hdr;
>>>>> +    uint8_t *tmp = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
>>>>>        int err;
>>>>>    -    err = init_get_bits8(&gb, buf, size);
>>>>> -    if (err < 0)
>>>>> +    if (!tmp)
>>>>> +        return AVERROR(ENOMEM);
>>>>> +
>>>>> +    memcpy(tmp, buf, size);
>>>>> +    memset(tmp + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>>>>> +    err = init_get_bits8(&gb, tmp, size);
>>>>> +    if (err < 0) {
>>>>> +        av_free(tmp);
>>>>>            return AVERROR_INVALIDDATA;
>>>>> +    }
>>>>>        err = ff_ac3_parse_header(&gb, &hdr);
>>>>> -    if (err < 0)
>>>>> +    if (err < 0) {
>>>>> +        av_free(tmp);
>>>>>            return AVERROR_INVALIDDATA;
>>>>> +    }
>>>>>          *bitstream_id = hdr.bitstream_id;
>>>>>        *frame_size   = hdr.frame_size;
>>>>
>>>> There is no need for an allocation here; (E)AC-3 frames have a bounded
>>>> size and the number of bytes read by ff_ac3_parse_header() is even
>>>> smaller.
>>>
>>> What's the max size? Is there a define for it?
>>
>> Not yet.
> 
> Ok, what size should the static buffer be?
> 

It should not be static. And I don't know. Just look at the code to get
the maximum.

- Andreas

_______________________________________________
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 v2 1/4] avcodec/ac3_parser: use a padded buffer in av_ac3_parse_header()

[James Almer]

The GetBitContext API requires the buffer to be padded, and the documentation for
av_ac3_parse_header() does not specify it, so use a temporary local buffer.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/ac3_parser.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
index 69989690dd..1d9a40a8d9 100644
--- a/libavcodec/ac3_parser.c
+++ b/libavcodec/ac3_parser.c
@@ -202,9 +202,13 @@ int av_ac3_parse_header(const uint8_t *buf, size_t size,
 {
     GetBitContext gb;
     AC3HeaderInfo hdr;
+    uint8_t tmp[32 + AV_INPUT_BUFFER_PADDING_SIZE];
     int err;
 
-    err = init_get_bits8(&gb, buf, size);
+    size = FFMIN(32, size);
+    memcpy(tmp, buf, size);
+    memset(tmp + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
+    err = init_get_bits8(&gb, tmp, size);
     if (err < 0)
         return AVERROR_INVALIDDATA;
     err = ff_ac3_parse_header(&gb, &hdr);
-- 
2.49.0

_______________________________________________
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 1/4] avcodec/ac3_parser: use a padded buffer in av_ac3_parse_header()

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 392 bytes --]

On 6/5/2025 9:14 PM, James Almer wrote:
> The GetBitContext API requires the buffer to be padded, and the documentation for
> av_ac3_parse_header() does not specify it, so use a temporary local buffer.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>   libavcodec/ac3_parser.c | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)

Will apply set.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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