[FFmpeg-devel] [PATCH 1/4] avcodec/adts_parser: Don't presume buffer to be padded

[FFmpeg-devel] [PATCH 1/4] avcodec/adts_parser: Don't presume buffer to be padded

[Andreas Rheinhardt]

The documentation of av_adts_header_parse() does not require
the buffer to be padded at all.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/adts_parser.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libavcodec/adts_parser.c b/libavcodec/adts_parser.c
index 6c22c86ef2..81e2669149 100644
--- a/libavcodec/adts_parser.c
+++ b/libavcodec/adts_parser.c
@@ -28,9 +28,14 @@
 int av_adts_header_parse(const uint8_t *buf, uint32_t *samples, uint8_t *frames)
 {
 #if CONFIG_ADTS_HEADER
+    uint8_t tmpbuf[AV_AAC_ADTS_HEADER_SIZE + AV_INPUT_BUFFER_PADDING_SIZE];
     GetBitContext gb;
     AACADTSHeaderInfo hdr;
-    int err = init_get_bits8(&gb, buf, AV_AAC_ADTS_HEADER_SIZE);
+    int err;
+    if (!buf)
+        return AVERROR(EINVAL);
+    memcpy(tmpbuf, buf, AV_AAC_ADTS_HEADER_SIZE);
+    err = init_get_bits8(&gb, tmpbuf, AV_AAC_ADTS_HEADER_SIZE);
     if (err < 0)
         return err;
     err = ff_adts_header_parse(&gb, &hdr);
-- 
2.40.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/4] avcodec/aac_ac3_parser: Untangle AAC and AC3 parsing error codes

[Andreas Rheinhardt]

Also remove the (unused) AAC_AC3_PARSE_ERROR_CHANNEL_CFG while at it;
furthermore, fix the documentation of ff_ac3_parse_header()
and (ff|avpriv)_adts_header_parse().

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/aac_ac3_parser.h      | 10 ----------
 libavcodec/ac3_parser.c          | 14 +++++++-------
 libavcodec/ac3_parser_internal.h | 13 ++++++++++---
 libavcodec/ac3dec.c              | 18 ++++++++----------
 libavcodec/adts_header.c         |  7 +++----
 libavcodec/adts_header.h         | 16 ++++++++++------
 libavcodec/eac3dec.c             |  6 +++---
 7 files changed, 41 insertions(+), 43 deletions(-)

diff --git a/libavcodec/aac_ac3_parser.h b/libavcodec/aac_ac3_parser.h
index bc16181a19..e3259d1841 100644
--- a/libavcodec/aac_ac3_parser.h
+++ b/libavcodec/aac_ac3_parser.h
@@ -28,16 +28,6 @@
 #include "avcodec.h"
 #include "parser.h"
 
-typedef enum {
-    AAC_AC3_PARSE_ERROR_SYNC        = -0x1030c0a,
-    AAC_AC3_PARSE_ERROR_BSID        = -0x2030c0a,
-    AAC_AC3_PARSE_ERROR_SAMPLE_RATE = -0x3030c0a,
-    AAC_AC3_PARSE_ERROR_FRAME_SIZE  = -0x4030c0a,
-    AAC_AC3_PARSE_ERROR_FRAME_TYPE  = -0x5030c0a,
-    AAC_AC3_PARSE_ERROR_CRC         = -0x6030c0a,
-    AAC_AC3_PARSE_ERROR_CHANNEL_CFG = -0x7030c0a,
-} AACAC3ParseError;
-
 typedef struct AACAC3ParseContext {
     ParseContext pc;
     int header_size;
diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
index 4e0ba73481..69989690dd 100644
--- a/libavcodec/ac3_parser.c
+++ b/libavcodec/ac3_parser.c
@@ -81,12 +81,12 @@ int ff_ac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr)
 
     hdr->sync_word = get_bits(gbc, 16);
     if(hdr->sync_word != 0x0B77)
-        return AAC_AC3_PARSE_ERROR_SYNC;
+        return AC3_PARSE_ERROR_SYNC;
 
     /* read ahead to bsid to distinguish between AC-3 and E-AC-3 */
     hdr->bitstream_id = show_bits_long(gbc, 29) & 0x1F;
     if(hdr->bitstream_id > 16)
-        return AAC_AC3_PARSE_ERROR_BSID;
+        return AC3_PARSE_ERROR_BSID;
 
     hdr->num_blocks = 6;
     hdr->ac3_bit_rate_code = -1;
@@ -103,11 +103,11 @@ int ff_ac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr)
         hdr->crc1 = get_bits(gbc, 16);
         hdr->sr_code = get_bits(gbc, 2);
         if(hdr->sr_code == 3)
-            return AAC_AC3_PARSE_ERROR_SAMPLE_RATE;
+            return AC3_PARSE_ERROR_SAMPLE_RATE;
 
         frame_size_code = get_bits(gbc, 6);
         if(frame_size_code > 37)
-            return AAC_AC3_PARSE_ERROR_FRAME_SIZE;
+            return AC3_PARSE_ERROR_FRAME_SIZE;
 
         hdr->ac3_bit_rate_code = (frame_size_code >> 1);
 
@@ -138,19 +138,19 @@ int ff_ac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr)
         hdr->crc1 = 0;
         hdr->frame_type = get_bits(gbc, 2);
         if(hdr->frame_type == EAC3_FRAME_TYPE_RESERVED)
-            return AAC_AC3_PARSE_ERROR_FRAME_TYPE;
+            return AC3_PARSE_ERROR_FRAME_TYPE;
 
         hdr->substreamid = get_bits(gbc, 3);
 
         hdr->frame_size = (get_bits(gbc, 11) + 1) << 1;
         if(hdr->frame_size < AC3_HEADER_SIZE)
-            return AAC_AC3_PARSE_ERROR_FRAME_SIZE;
+            return AC3_PARSE_ERROR_FRAME_SIZE;
 
         hdr->sr_code = get_bits(gbc, 2);
         if (hdr->sr_code == 3) {
             int sr_code2 = get_bits(gbc, 2);
             if(sr_code2 == 3)
-                return AAC_AC3_PARSE_ERROR_SAMPLE_RATE;
+                return AC3_PARSE_ERROR_SAMPLE_RATE;
             hdr->sample_rate = ff_ac3_sample_rate_tab[sr_code2] / 2;
             hdr->sr_shift = 1;
         } else {
diff --git a/libavcodec/ac3_parser_internal.h b/libavcodec/ac3_parser_internal.h
index 2ac0e67ec2..2c4eb546e5 100644
--- a/libavcodec/ac3_parser_internal.h
+++ b/libavcodec/ac3_parser_internal.h
@@ -64,15 +64,22 @@ typedef struct AC3HeaderInfo {
     /** @} */
 } AC3HeaderInfo;
 
+typedef enum {
+    AC3_PARSE_ERROR_SYNC        = -0x1030c0a,
+    AC3_PARSE_ERROR_BSID        = -0x2030c0a,
+    AC3_PARSE_ERROR_SAMPLE_RATE = -0x3030c0a,
+    AC3_PARSE_ERROR_FRAME_SIZE  = -0x4030c0a,
+    AC3_PARSE_ERROR_FRAME_TYPE  = -0x5030c0a,
+    AC3_PARSE_ERROR_CRC         = -0x6030c0a,
+} AC3ParseError;
+
 /**
  * Parse AC-3 frame header.
  * Parse the header up to the lfeon element, which is the first 52 or 54 bits
  * depending on the audio coding mode.
  * @param[in]  gbc BitContext containing the first 54 bits of the frame.
  * @param[out] hdr Pointer to struct where header info is written.
- * @return Returns 0 on success, -1 if there is a sync word mismatch,
- * -2 if the bsid (version) element is invalid, -3 if the fscod (sample rate)
- * element is invalid, or -4 if the frmsizecod (bit rate) element is invalid.
+ * @return Returns 0 on success and AC3_PARSE_ERROR_* values otherwise.
  */
 int ff_ac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr);
 
diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
index 2d7e11c5b8..0a4d3375ee 100644
--- a/libavcodec/ac3dec.c
+++ b/libavcodec/ac3dec.c
@@ -39,7 +39,6 @@
 #include "libavutil/opt.h"
 #include "libavutil/thread.h"
 #include "bswapdsp.h"
-#include "aac_ac3_parser.h"
 #include "ac3_parser_internal.h"
 #include "ac3dec.h"
 #include "ac3dec_data.h"
@@ -1538,19 +1537,19 @@ dependent_frame:
 
     if (err) {
         switch (err) {
-        case AAC_AC3_PARSE_ERROR_SYNC:
+        case AC3_PARSE_ERROR_SYNC:
             av_log(avctx, AV_LOG_ERROR, "frame sync error\n");
             return AVERROR_INVALIDDATA;
-        case AAC_AC3_PARSE_ERROR_BSID:
+        case AC3_PARSE_ERROR_BSID:
             av_log(avctx, AV_LOG_ERROR, "invalid bitstream id\n");
             break;
-        case AAC_AC3_PARSE_ERROR_SAMPLE_RATE:
+        case AC3_PARSE_ERROR_SAMPLE_RATE:
             av_log(avctx, AV_LOG_ERROR, "invalid sample rate\n");
             break;
-        case AAC_AC3_PARSE_ERROR_FRAME_SIZE:
+        case AC3_PARSE_ERROR_FRAME_SIZE:
             av_log(avctx, AV_LOG_ERROR, "invalid frame size\n");
             break;
-        case AAC_AC3_PARSE_ERROR_FRAME_TYPE:
+        case AC3_PARSE_ERROR_FRAME_TYPE:
             /* skip frame if CRC is ok. otherwise use error concealment. */
             /* TODO: add support for substreams */
             if (s->substreamid) {
@@ -1563,8 +1562,7 @@ dependent_frame:
                 av_log(avctx, AV_LOG_ERROR, "invalid frame type\n");
             }
             break;
-        case AAC_AC3_PARSE_ERROR_CRC:
-        case AAC_AC3_PARSE_ERROR_CHANNEL_CFG:
+        case AC3_PARSE_ERROR_CRC:
             break;
         default: // Normal AVERROR do not try to recover.
             *got_frame_ptr = 0;
@@ -1574,7 +1572,7 @@ dependent_frame:
         /* check that reported frame size fits in input buffer */
         if (s->frame_size > buf_size) {
             av_log(avctx, AV_LOG_ERROR, "incomplete frame\n");
-            err = AAC_AC3_PARSE_ERROR_FRAME_SIZE;
+            err = AC3_PARSE_ERROR_FRAME_SIZE;
         } else if (avctx->err_recognition & (AV_EF_CRCCHECK|AV_EF_CAREFUL)) {
             /* check for crc mismatch */
             if (av_crc(av_crc_get_table(AV_CRC_16_ANSI), 0, &buf[2],
@@ -1582,7 +1580,7 @@ dependent_frame:
                 av_log(avctx, AV_LOG_ERROR, "frame CRC mismatch\n");
                 if (avctx->err_recognition & AV_EF_EXPLODE)
                     return AVERROR_INVALIDDATA;
-                err = AAC_AC3_PARSE_ERROR_CRC;
+                err = AC3_PARSE_ERROR_CRC;
             }
         }
     }
diff --git a/libavcodec/adts_header.c b/libavcodec/adts_header.c
index 00fa0a5a99..8663d00f4c 100644
--- a/libavcodec/adts_header.c
+++ b/libavcodec/adts_header.c
@@ -21,7 +21,6 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-#include "aac_ac3_parser.h"
 #include "adts_header.h"
 #include "adts_parser.h"
 #include "get_bits.h"
@@ -35,7 +34,7 @@ int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr)
     memset(hdr, 0, sizeof(*hdr));
 
     if (get_bits(gbc, 12) != 0xfff)
-        return AAC_AC3_PARSE_ERROR_SYNC;
+        return AAC_PARSE_ERROR_SYNC;
 
     skip_bits1(gbc);             /* id */
     skip_bits(gbc, 2);           /* layer */
@@ -43,7 +42,7 @@ int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr)
     aot     = get_bits(gbc, 2);  /* profile_objecttype */
     sr      = get_bits(gbc, 4);  /* sample_frequency_index */
     if (!ff_mpeg4audio_sample_rates[sr])
-        return AAC_AC3_PARSE_ERROR_SAMPLE_RATE;
+        return AAC_PARSE_ERROR_SAMPLE_RATE;
     skip_bits1(gbc);             /* private_bit */
     ch = get_bits(gbc, 3);       /* channel_configuration */
 
@@ -55,7 +54,7 @@ int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr)
     skip_bits1(gbc);             /* copyright_identification_start */
     size = get_bits(gbc, 13);    /* aac_frame_length */
     if (size < AV_AAC_ADTS_HEADER_SIZE)
-        return AAC_AC3_PARSE_ERROR_FRAME_SIZE;
+        return AAC_PARSE_ERROR_FRAME_SIZE;
 
     skip_bits(gbc, 11);          /* adts_buffer_fullness */
     rdb = get_bits(gbc, 2);      /* number_of_raw_data_blocks_in_frame */
diff --git a/libavcodec/adts_header.h b/libavcodec/adts_header.h
index 354d07e1f8..d89d487025 100644
--- a/libavcodec/adts_header.h
+++ b/libavcodec/adts_header.h
@@ -25,6 +25,12 @@
 
 #include "get_bits.h"
 
+typedef enum {
+    AAC_PARSE_ERROR_SYNC        = -0x1030c0a,
+    AAC_PARSE_ERROR_SAMPLE_RATE = -0x3030c0a,
+    AAC_PARSE_ERROR_FRAME_SIZE  = -0x4030c0a,
+} AACParseError;
+
 typedef struct AACADTSHeaderInfo {
     uint32_t sample_rate;
     uint32_t samples;
@@ -42,9 +48,8 @@ typedef struct AACADTSHeaderInfo {
  * the first 54 bits.
  * @param[in]  gbc BitContext containing the first 54 bits of the frame.
  * @param[out] hdr Pointer to struct where header info is written.
- * @return Returns 0 on success, -1 if there is a sync word mismatch,
- * -2 if the version element is invalid, -3 if the sample rate
- * element is invalid, or -4 if the bit rate element is invalid.
+ * @return Returns the size in bytes of the header parsed on success
+ * and AAC_PARSE_ERROR_* values otherwise.
  */
 int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr);
 
@@ -56,9 +61,8 @@ int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr);
  * @param[out] phdr Pointer to pointer to struct AACADTSHeaderInfo for which
  * memory is allocated and header info is written into it. After using the header
  * information, the allocated memory must be freed by using av_free.
- * @return Returns 0 on success, -1 if there is a sync word mismatch,
- * -2 if the version element is invalid, -3 if the sample rate
- * element is invalid, or -4 if the bit rate element is invalid.
+ * @return Returns 0 on success, AAC_PARSE_ERROR_* values on invalid input
+ *         and ordinary AVERROR codes otherwise.
  */
 int avpriv_adts_header_parse(AACADTSHeaderInfo **phdr, const uint8_t *buf, size_t size);
 
diff --git a/libavcodec/eac3dec.c b/libavcodec/eac3dec.c
index 5c71751a0c..2b3bffda6e 100644
--- a/libavcodec/eac3dec.c
+++ b/libavcodec/eac3dec.c
@@ -39,8 +39,8 @@
 
 
 #include "avcodec.h"
-#include "aac_ac3_parser.h"
 #include "ac3.h"
+#include "ac3_parser_internal.h"
 #include "ac3dec.h"
 #include "ac3dec_data.h"
 #include "eac3_data.h"
@@ -300,7 +300,7 @@ static int ff_eac3_parse_header(AC3DecodeContext *s)
        dependent streams which are used to add or replace channels. */
     if (s->frame_type == EAC3_FRAME_TYPE_RESERVED) {
         av_log(s->avctx, AV_LOG_ERROR, "Reserved frame type\n");
-        return AAC_AC3_PARSE_ERROR_FRAME_TYPE;
+        return AC3_PARSE_ERROR_FRAME_TYPE;
     }
 
     /* The substream id indicates which substream this frame belongs to. each
@@ -312,7 +312,7 @@ static int ff_eac3_parse_header(AC3DecodeContext *s)
             s->eac3_subsbtreamid_found = 1;
             avpriv_request_sample(s->avctx, "Additional substreams");
         }
-        return AAC_AC3_PARSE_ERROR_FRAME_TYPE;
+        return AC3_PARSE_ERROR_FRAME_TYPE;
     }
 
     if (s->bit_alloc_params.sr_code == EAC3_SR_CODE_REDUCED) {
-- 
2.40.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/4] avcodec/adts_header: Add ff_adts_header_parse_buf()

[Andreas Rheinhardt]

Most users of ff_adts_header_parse() don't already have
an opened GetBitContext for the header, so add a convenience
function for them.
Also use a forward declaration of GetBitContext in adts_header.h
as this avoids (implicit) inclusion of get_bits.h in some of
the users that now no longer use a GetBitContext of their own.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/aac_parser.c        | 16 +++++-----------
 libavcodec/adts_header.c       | 10 ++++++++++
 libavcodec/adts_header.h       | 14 ++++++++++++--
 libavcodec/adts_parser.c       | 18 ++++--------------
 libavcodec/bsf/aac_adtstoasc.c |  6 ++----
 libavcodec/ftr_parser.c        |  7 ++-----
 6 files changed, 35 insertions(+), 36 deletions(-)

diff --git a/libavcodec/aac_parser.c b/libavcodec/aac_parser.c
index f295dfccdd..186fcd887a 100644
--- a/libavcodec/aac_parser.c
+++ b/libavcodec/aac_parser.c
@@ -24,24 +24,18 @@
 #include "aac_ac3_parser.h"
 #include "adts_header.h"
 #include "adts_parser.h"
-#include "get_bits.h"
-#include "mpeg4audio.h"
+#include "libavutil/intreadwrite.h"
 
 static int aac_sync(uint64_t state, int *need_next_header, int *new_frame_start)
 {
-    GetBitContext bits;
+    uint8_t tmp[8 + AV_INPUT_BUFFER_PADDING_SIZE];
     AACADTSHeaderInfo hdr;
     int size;
-    union {
-        uint64_t u64;
-        uint8_t  u8[8 + AV_INPUT_BUFFER_PADDING_SIZE];
-    } tmp;
 
-    tmp.u64 = av_be2ne64(state);
-    init_get_bits(&bits, tmp.u8 + 8 - AV_AAC_ADTS_HEADER_SIZE,
-                  AV_AAC_ADTS_HEADER_SIZE * 8);
+    AV_WB64(tmp, state);
 
-    if ((size = ff_adts_header_parse(&bits, &hdr)) < 0)
+    size = ff_adts_header_parse_buf(tmp + 8 - AV_AAC_ADTS_HEADER_SIZE, &hdr);
+    if (size < 0)
         return 0;
     *need_next_header = 0;
     *new_frame_start  = 1;
diff --git a/libavcodec/adts_header.c b/libavcodec/adts_header.c
index 8663d00f4c..0d6be1aa0e 100644
--- a/libavcodec/adts_header.c
+++ b/libavcodec/adts_header.c
@@ -25,6 +25,7 @@
 #include "adts_parser.h"
 #include "get_bits.h"
 #include "mpeg4audio.h"
+#include "libavutil/avassert.h"
 
 int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr)
 {
@@ -71,3 +72,12 @@ int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr)
 
     return size;
 }
+
+int ff_adts_header_parse_buf(const uint8_t buf[AV_AAC_ADTS_HEADER_SIZE + AV_INPUT_BUFFER_PADDING_SIZE],
+                             AACADTSHeaderInfo *hdr)
+{
+    GetBitContext gb;
+    av_unused int ret = init_get_bits8(&gb, buf, AV_AAC_ADTS_HEADER_SIZE);
+    av_assert1(ret >= 0);
+    return ff_adts_header_parse(&gb, hdr);
+}
diff --git a/libavcodec/adts_header.h b/libavcodec/adts_header.h
index d89d487025..4f62edcbac 100644
--- a/libavcodec/adts_header.h
+++ b/libavcodec/adts_header.h
@@ -23,7 +23,8 @@
 #ifndef AVCODEC_ADTS_HEADER_H
 #define AVCODEC_ADTS_HEADER_H
 
-#include "get_bits.h"
+#include "adts_parser.h"
+#include "defs.h"
 
 typedef enum {
     AAC_PARSE_ERROR_SYNC        = -0x1030c0a,
@@ -43,6 +44,8 @@ typedef struct AACADTSHeaderInfo {
     uint32_t frame_length;
 } AACADTSHeaderInfo;
 
+struct GetBitContext;
+
 /**
  * Parse the ADTS frame header to the end of the variable header, which is
  * the first 54 bits.
@@ -51,7 +54,14 @@ typedef struct AACADTSHeaderInfo {
  * @return Returns the size in bytes of the header parsed on success
  * and AAC_PARSE_ERROR_* values otherwise.
  */
-int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr);
+int ff_adts_header_parse(struct GetBitContext *gbc, AACADTSHeaderInfo *hdr);
+
+/**
+ * Wrapper around ff_adts_header_parse() for users that already have
+ * a proper GetBitContext.
+ */
+int ff_adts_header_parse_buf(const uint8_t buf[AV_AAC_ADTS_HEADER_SIZE + AV_INPUT_BUFFER_PADDING_SIZE],
+                             AACADTSHeaderInfo *hdr);
 
 /**
  * Parse the ADTS frame header contained in the buffer, which is
diff --git a/libavcodec/adts_parser.c b/libavcodec/adts_parser.c
index 81e2669149..66b988d6f6 100644
--- a/libavcodec/adts_parser.c
+++ b/libavcodec/adts_parser.c
@@ -20,7 +20,9 @@
 
 #include <stddef.h>
 #include <stdint.h>
+#include <string.h>
 
+#include "libavutil/error.h"
 #include "libavutil/mem.h"
 #include "adts_header.h"
 #include "adts_parser.h"
@@ -29,16 +31,12 @@ int av_adts_header_parse(const uint8_t *buf, uint32_t *samples, uint8_t *frames)
 {
 #if CONFIG_ADTS_HEADER
     uint8_t tmpbuf[AV_AAC_ADTS_HEADER_SIZE + AV_INPUT_BUFFER_PADDING_SIZE];
-    GetBitContext gb;
     AACADTSHeaderInfo hdr;
     int err;
     if (!buf)
         return AVERROR(EINVAL);
     memcpy(tmpbuf, buf, AV_AAC_ADTS_HEADER_SIZE);
-    err = init_get_bits8(&gb, tmpbuf, AV_AAC_ADTS_HEADER_SIZE);
-    if (err < 0)
-        return err;
-    err = ff_adts_header_parse(&gb, &hdr);
+    err = ff_adts_header_parse_buf(tmpbuf, &hdr);
     if (err < 0)
         return err;
     *samples = hdr.samples;
@@ -54,7 +52,6 @@ int avpriv_adts_header_parse(AACADTSHeaderInfo **phdr, const uint8_t *buf, size_
 #if CONFIG_ADTS_HEADER
     int ret = 0;
     int allocated = 0;
-    GetBitContext gb;
 
     if (!phdr || !buf || size < AV_AAC_ADTS_HEADER_SIZE)
         return AVERROR_INVALIDDATA;
@@ -66,14 +63,7 @@ int avpriv_adts_header_parse(AACADTSHeaderInfo **phdr, const uint8_t *buf, size_
     if (!*phdr)
         return AVERROR(ENOMEM);
 
-    ret = init_get_bits8(&gb, buf, AV_AAC_ADTS_HEADER_SIZE);
-    if (ret < 0) {
-        if (allocated)
-            av_freep(phdr);
-        return ret;
-    }
-
-    ret = ff_adts_header_parse(&gb, *phdr);
+    ret = ff_adts_header_parse_buf(buf, *phdr);
     if (ret < 0) {
         if (allocated)
             av_freep(phdr);
diff --git a/libavcodec/bsf/aac_adtstoasc.c b/libavcodec/bsf/aac_adtstoasc.c
index dd5e8b2a31..b821414f2a 100644
--- a/libavcodec/bsf/aac_adtstoasc.c
+++ b/libavcodec/bsf/aac_adtstoasc.c
@@ -40,7 +40,6 @@ static int aac_adtstoasc_filter(AVBSFContext *bsfc, AVPacket *pkt)
 {
     AACBSFContext *ctx = bsfc->priv_data;
 
-    GetBitContext gb;
     PutBitContext pb;
     AACADTSHeaderInfo hdr;
     int ret;
@@ -55,9 +54,7 @@ static int aac_adtstoasc_filter(AVBSFContext *bsfc, AVPacket *pkt)
     if (pkt->size < AV_AAC_ADTS_HEADER_SIZE)
         goto packet_too_small;
 
-    init_get_bits(&gb, pkt->data, AV_AAC_ADTS_HEADER_SIZE * 8);
-
-    if (ff_adts_header_parse(&gb, &hdr) < 0) {
+    if (ff_adts_header_parse_buf(pkt->data, &hdr) < 0) {
         av_log(bsfc, AV_LOG_ERROR, "Error parsing ADTS frame header!\n");
         ret = AVERROR_INVALIDDATA;
         goto fail;
@@ -81,6 +78,7 @@ static int aac_adtstoasc_filter(AVBSFContext *bsfc, AVPacket *pkt)
         uint8_t       *extradata;
 
         if (!hdr.chan_config) {
+            GetBitContext gb;
             init_get_bits(&gb, pkt->data, pkt->size * 8);
             if (get_bits(&gb, 3) != 5) {
                 avpriv_report_missing_feature(bsfc,
diff --git a/libavcodec/ftr_parser.c b/libavcodec/ftr_parser.c
index 05e6cfed98..656fd289f6 100644
--- a/libavcodec/ftr_parser.c
+++ b/libavcodec/ftr_parser.c
@@ -25,7 +25,6 @@
  */
 
 #include "parser.h"
-#include "get_bits.h"
 #include "adts_header.h"
 #include "adts_parser.h"
 #include "mpeg4audio.h"
@@ -45,7 +44,6 @@ static int ftr_parse(AVCodecParserContext *s, AVCodecContext *avctx,
     FTRParseContext *ftr = s->priv_data;
     uint64_t state = ftr->pc.state64;
     int next = END_NOT_FOUND;
-    GetBitContext bits;
     AACADTSHeaderInfo hdr;
     int size;
 
@@ -71,10 +69,9 @@ static int ftr_parse(AVCodecParserContext *s, AVCodecContext *avctx,
 
             state = (state << 8) | buf[i];
             AV_WB64(tmp, state);
-            init_get_bits(&bits, tmp + 8 - AV_AAC_ADTS_HEADER_SIZE,
-                          AV_AAC_ADTS_HEADER_SIZE * 8);
+            size = ff_adts_header_parse_buf(tmp + 8 - AV_AAC_ADTS_HEADER_SIZE, &hdr);
 
-            if ((size = ff_adts_header_parse(&bits, &hdr)) > 0) {
+            if (size > 0) {
                 ftr->skip = size - 6;
                 ftr->frame_index += ff_mpeg4audio_channels[hdr.chan_config];
                 if (ftr->frame_index >= avctx->ch_layout.nb_channels) {
-- 
2.40.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/4] avcodec/aac_ac3_parser: Use ff_adts_header_parse_buf()

[Andreas Rheinhardt]

instead of avpriv_adts_header_parse(). Using the former avoids
an indirection.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/aac_ac3_parser.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/libavcodec/aac_ac3_parser.c b/libavcodec/aac_ac3_parser.c
index 9305b4c50f..f45631d09f 100644
--- a/libavcodec/aac_ac3_parser.c
+++ b/libavcodec/aac_ac3_parser.c
@@ -144,10 +144,9 @@ get_next:
 #endif
         } else {
 #if CONFIG_AAC_PARSER
-            AACADTSHeaderInfo hdr, *phrd = &hdr;
-            int ret = avpriv_adts_header_parse(&phrd, buf, buf_size);
-
-            if (ret < 0)
+            AACADTSHeaderInfo hdr;
+            if (buf_size < AV_AAC_ADTS_HEADER_SIZE ||
+                ff_adts_header_parse_buf(buf, &hdr) < 0)
                 return i;
 
             bit_rate = hdr.bit_rate;
-- 
2.40.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 3/4] avcodec/adts_header: Add ff_adts_header_parse_buf()

[James Almer]

On 5/10/2024 1:56 PM, Andreas Rheinhardt wrote:
> Most users of ff_adts_header_parse() don't already have
> an opened GetBitContext for the header, so add a convenience
> function for them.
> Also use a forward declaration of GetBitContext in adts_header.h
> as this avoids (implicit) inclusion of get_bits.h in some of
> the users that now no longer use a GetBitContext of their own.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>   libavcodec/aac_parser.c        | 16 +++++-----------
>   libavcodec/adts_header.c       | 10 ++++++++++
>   libavcodec/adts_header.h       | 14 ++++++++++++--
>   libavcodec/adts_parser.c       | 18 ++++--------------
>   libavcodec/bsf/aac_adtstoasc.c |  6 ++----
>   libavcodec/ftr_parser.c        |  7 ++-----
>   6 files changed, 35 insertions(+), 36 deletions(-)

> diff --git a/libavcodec/adts_header.h b/libavcodec/adts_header.h
> index d89d487025..4f62edcbac 100644
> --- a/libavcodec/adts_header.h
> +++ b/libavcodec/adts_header.h
> @@ -23,7 +23,8 @@
>   #ifndef AVCODEC_ADTS_HEADER_H
>   #define AVCODEC_ADTS_HEADER_H
>   
> -#include "get_bits.h"
> +#include "adts_parser.h"
> +#include "defs.h"
>   
>   typedef enum {
>       AAC_PARSE_ERROR_SYNC        = -0x1030c0a,
> @@ -43,6 +44,8 @@ typedef struct AACADTSHeaderInfo {
>       uint32_t frame_length;
>   } AACADTSHeaderInfo;
>   
> +struct GetBitContext;
> +
>   /**
>    * Parse the ADTS frame header to the end of the variable header, which is
>    * the first 54 bits.
> @@ -51,7 +54,14 @@ typedef struct AACADTSHeaderInfo {
>    * @return Returns the size in bytes of the header parsed on success
>    * and AAC_PARSE_ERROR_* values otherwise.
>    */
> -int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr);
> +int ff_adts_header_parse(struct GetBitContext *gbc, AACADTSHeaderInfo *hdr);
> +
> +/**
> + * Wrapper around ff_adts_header_parse() for users that already have
> + * a proper GetBitContext.

That have or don't have?

> + */
> +int ff_adts_header_parse_buf(const uint8_t buf[AV_AAC_ADTS_HEADER_SIZE + AV_INPUT_BUFFER_PADDING_SIZE],
> +                             AACADTSHeaderInfo *hdr);
>   
>   /**
>    * Parse the ADTS frame header contained in the buffer, which is
_______________________________________________
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 3/4] avcodec/adts_header: Add ff_adts_header_parse_buf()

[Andreas Rheinhardt]

James Almer:
> On 5/10/2024 1:56 PM, Andreas Rheinhardt wrote:
>> Most users of ff_adts_header_parse() don't already have
>> an opened GetBitContext for the header, so add a convenience
>> function for them.
>> Also use a forward declaration of GetBitContext in adts_header.h
>> as this avoids (implicit) inclusion of get_bits.h in some of
>> the users that now no longer use a GetBitContext of their own.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>   libavcodec/aac_parser.c        | 16 +++++-----------
>>   libavcodec/adts_header.c       | 10 ++++++++++
>>   libavcodec/adts_header.h       | 14 ++++++++++++--
>>   libavcodec/adts_parser.c       | 18 ++++--------------
>>   libavcodec/bsf/aac_adtstoasc.c |  6 ++----
>>   libavcodec/ftr_parser.c        |  7 ++-----
>>   6 files changed, 35 insertions(+), 36 deletions(-)
> 
>> diff --git a/libavcodec/adts_header.h b/libavcodec/adts_header.h
>> index d89d487025..4f62edcbac 100644
>> --- a/libavcodec/adts_header.h
>> +++ b/libavcodec/adts_header.h
>> @@ -23,7 +23,8 @@
>>   #ifndef AVCODEC_ADTS_HEADER_H
>>   #define AVCODEC_ADTS_HEADER_H
>>   -#include "get_bits.h"
>> +#include "adts_parser.h"
>> +#include "defs.h"
>>     typedef enum {
>>       AAC_PARSE_ERROR_SYNC        = -0x1030c0a,
>> @@ -43,6 +44,8 @@ typedef struct AACADTSHeaderInfo {
>>       uint32_t frame_length;
>>   } AACADTSHeaderInfo;
>>   +struct GetBitContext;
>> +
>>   /**
>>    * Parse the ADTS frame header to the end of the variable header,
>> which is
>>    * the first 54 bits.
>> @@ -51,7 +54,14 @@ typedef struct AACADTSHeaderInfo {
>>    * @return Returns the size in bytes of the header parsed on success
>>    * and AAC_PARSE_ERROR_* values otherwise.
>>    */
>> -int ff_adts_header_parse(GetBitContext *gbc, AACADTSHeaderInfo *hdr);
>> +int ff_adts_header_parse(struct GetBitContext *gbc, AACADTSHeaderInfo
>> *hdr);
>> +
>> +/**
>> + * Wrapper around ff_adts_header_parse() for users that already have
>> + * a proper GetBitContext.
> 
> That have or don't have?

that don't already have a proper GetBitContext

> 
>> + */
>> +int ff_adts_header_parse_buf(const uint8_t
>> buf[AV_AAC_ADTS_HEADER_SIZE + AV_INPUT_BUFFER_PADDING_SIZE],
>> +                             AACADTSHeaderInfo *hdr);
>>     /**
>>    * Parse the ADTS frame header contained in the buffer, which is
> _______________________________________________
> 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 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 2/4] avcodec/aac_ac3_parser: Untangle AAC and AC3 parsing error codes

[Andrew Sayers]

On Fri, May 10, 2024 at 06:56:25PM +0200, Andreas Rheinhardt wrote:
> Also remove the (unused) AAC_AC3_PARSE_ERROR_CHANNEL_CFG while at it;
> furthermore, fix the documentation of ff_ac3_parse_header()
> and (ff|avpriv)_adts_header_parse().
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/aac_ac3_parser.h      | 10 ----------
>  libavcodec/ac3_parser.c          | 14 +++++++-------
>  libavcodec/ac3_parser_internal.h | 13 ++++++++++---
>  libavcodec/ac3dec.c              | 18 ++++++++----------
>  libavcodec/adts_header.c         |  7 +++----
>  libavcodec/adts_header.h         | 16 ++++++++++------
>  libavcodec/eac3dec.c             |  6 +++---
>  7 files changed, 41 insertions(+), 43 deletions(-)
> 
> diff --git a/libavcodec/aac_ac3_parser.h b/libavcodec/aac_ac3_parser.h
> index bc16181a19..e3259d1841 100644
> --- a/libavcodec/aac_ac3_parser.h
> +++ b/libavcodec/aac_ac3_parser.h
> @@ -28,16 +28,6 @@
>  #include "avcodec.h"
>  #include "parser.h"
>  
> -typedef enum {
> -    AAC_AC3_PARSE_ERROR_SYNC        = -0x1030c0a,
> -    AAC_AC3_PARSE_ERROR_BSID        = -0x2030c0a,
> -    AAC_AC3_PARSE_ERROR_SAMPLE_RATE = -0x3030c0a,
> -    AAC_AC3_PARSE_ERROR_FRAME_SIZE  = -0x4030c0a,
> -    AAC_AC3_PARSE_ERROR_FRAME_TYPE  = -0x5030c0a,
> -    AAC_AC3_PARSE_ERROR_CRC         = -0x6030c0a,
> -    AAC_AC3_PARSE_ERROR_CHANNEL_CFG = -0x7030c0a,
> -} AACAC3ParseError;
> -
>  typedef struct AACAC3ParseContext {
>      ParseContext pc;
>      int header_size;
> diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
> index 4e0ba73481..69989690dd 100644
> --- a/libavcodec/ac3_parser.c
> +++ b/libavcodec/ac3_parser.c
> @@ -81,12 +81,12 @@ int ff_ac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr)
>  
>      hdr->sync_word = get_bits(gbc, 16);
>      if(hdr->sync_word != 0x0B77)
> -        return AAC_AC3_PARSE_ERROR_SYNC;
> +        return AC3_PARSE_ERROR_SYNC;
>  
>      /* read ahead to bsid to distinguish between AC-3 and E-AC-3 */
>      hdr->bitstream_id = show_bits_long(gbc, 29) & 0x1F;
>      if(hdr->bitstream_id > 16)
> -        return AAC_AC3_PARSE_ERROR_BSID;
> +        return AC3_PARSE_ERROR_BSID;
>  
>      hdr->num_blocks = 6;
>      hdr->ac3_bit_rate_code = -1;
> @@ -103,11 +103,11 @@ int ff_ac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr)
>          hdr->crc1 = get_bits(gbc, 16);
>          hdr->sr_code = get_bits(gbc, 2);
>          if(hdr->sr_code == 3)
> -            return AAC_AC3_PARSE_ERROR_SAMPLE_RATE;
> +            return AC3_PARSE_ERROR_SAMPLE_RATE;
>  
>          frame_size_code = get_bits(gbc, 6);
>          if(frame_size_code > 37)
> -            return AAC_AC3_PARSE_ERROR_FRAME_SIZE;
> +            return AC3_PARSE_ERROR_FRAME_SIZE;
>  
>          hdr->ac3_bit_rate_code = (frame_size_code >> 1);
>  
> @@ -138,19 +138,19 @@ int ff_ac3_parse_header(GetBitContext *gbc, AC3HeaderInfo *hdr)
>          hdr->crc1 = 0;
>          hdr->frame_type = get_bits(gbc, 2);
>          if(hdr->frame_type == EAC3_FRAME_TYPE_RESERVED)
> -            return AAC_AC3_PARSE_ERROR_FRAME_TYPE;
> +            return AC3_PARSE_ERROR_FRAME_TYPE;
>  
>          hdr->substreamid = get_bits(gbc, 3);
>  
>          hdr->frame_size = (get_bits(gbc, 11) + 1) << 1;
>          if(hdr->frame_size < AC3_HEADER_SIZE)
> -            return AAC_AC3_PARSE_ERROR_FRAME_SIZE;
> +            return AC3_PARSE_ERROR_FRAME_SIZE;
>  
>          hdr->sr_code = get_bits(gbc, 2);
>          if (hdr->sr_code == 3) {
>              int sr_code2 = get_bits(gbc, 2);
>              if(sr_code2 == 3)
> -                return AAC_AC3_PARSE_ERROR_SAMPLE_RATE;
> +                return AC3_PARSE_ERROR_SAMPLE_RATE;
>              hdr->sample_rate = ff_ac3_sample_rate_tab[sr_code2] / 2;
>              hdr->sr_shift = 1;
>          } else {
> diff --git a/libavcodec/ac3_parser_internal.h b/libavcodec/ac3_parser_internal.h
> index 2ac0e67ec2..2c4eb546e5 100644
> --- a/libavcodec/ac3_parser_internal.h
> +++ b/libavcodec/ac3_parser_internal.h
> @@ -64,15 +64,22 @@ typedef struct AC3HeaderInfo {
>      /** @} */
>  } AC3HeaderInfo;
>  
> +typedef enum {
> +    AC3_PARSE_ERROR_SYNC        = -0x1030c0a,
> +    AC3_PARSE_ERROR_BSID        = -0x2030c0a,
> +    AC3_PARSE_ERROR_SAMPLE_RATE = -0x3030c0a,
> +    AC3_PARSE_ERROR_FRAME_SIZE  = -0x4030c0a,
> +    AC3_PARSE_ERROR_FRAME_TYPE  = -0x5030c0a,
> +    AC3_PARSE_ERROR_CRC         = -0x6030c0a,
> +} AC3ParseError;
> +
>  /**
>   * Parse AC-3 frame header.
>   * Parse the header up to the lfeon element, which is the first 52 or 54 bits
>   * depending on the audio coding mode.
>   * @param[in]  gbc BitContext containing the first 54 bits of the frame.
>   * @param[out] hdr Pointer to struct where header info is written.
> - * @return Returns 0 on success, -1 if there is a sync word mismatch,
> - * -2 if the bsid (version) element is invalid, -3 if the fscod (sample rate)
> - * element is invalid, or -4 if the frmsizecod (bit rate) element is invalid.
> + * @return Returns 0 on success and AC3_PARSE_ERROR_* values otherwise.

"@return Returns" is redundant - can this (and similar below) just be
"@return 0 on success ..."?
_______________________________________________
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/adts_parser: Don't presume buffer to be padded

[Andreas Rheinhardt]

Andreas Rheinhardt:
> The documentation of av_adts_header_parse() does not require
> the buffer to be padded at all.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/adts_parser.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/adts_parser.c b/libavcodec/adts_parser.c
> index 6c22c86ef2..81e2669149 100644
> --- a/libavcodec/adts_parser.c
> +++ b/libavcodec/adts_parser.c
> @@ -28,9 +28,14 @@
>  int av_adts_header_parse(const uint8_t *buf, uint32_t *samples, uint8_t *frames)
>  {
>  #if CONFIG_ADTS_HEADER
> +    uint8_t tmpbuf[AV_AAC_ADTS_HEADER_SIZE + AV_INPUT_BUFFER_PADDING_SIZE];
>      GetBitContext gb;
>      AACADTSHeaderInfo hdr;
> -    int err = init_get_bits8(&gb, buf, AV_AAC_ADTS_HEADER_SIZE);
> +    int err;
> +    if (!buf)
> +        return AVERROR(EINVAL);
> +    memcpy(tmpbuf, buf, AV_AAC_ADTS_HEADER_SIZE);
> +    err = init_get_bits8(&gb, tmpbuf, AV_AAC_ADTS_HEADER_SIZE);
>      if (err < 0)
>          return err;
>      err = ff_adts_header_parse(&gb, &hdr);

Will apply this patchset with the remarks by Andrew and James addressed
tomorrow unless there are objections.

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