[FFmpeg-devel] [PATCH 1/7] lavc/flac_parser: use a custom FIFO implementation

* [FFmpeg-devel] [PATCH 1/7] lavc/flac_parser: use a custom FIFO implementation
@ 2021-12-31 10:53 Anton Khirnov
  2021-12-31 10:53 ` [FFmpeg-devel] [PATCH 2/7] lavf/dvenc: replace av_fifo_peek2() with av_fifo_generic_peek_at() Anton Khirnov
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Anton Khirnov @ 2021-12-31 10:53 UTC (permalink / raw)
  To: ffmpeg-devel

FLAC parser currently uses AVFifoBuffer in a highly non-trivial manner,
modifying its "internals" (the whole struct is currently public, but no
other code touches its contents directly). E.g. it does not use any
av_fifo functions for reading the FIFO contents, but implements its own.

Reimplement the needed parts of the AVFifoBuffer API in the FLAC parser,
making it completely self-contained. This will allow us to make
AVFifoBuffer private.
---
 libavcodec/flac_parser.c | 191 +++++++++++++++++++++++++++++++--------
 1 file changed, 153 insertions(+), 38 deletions(-)

diff --git a/libavcodec/flac_parser.c b/libavcodec/flac_parser.c
index 3b27b152fc..d6af5ce836 100644
--- a/libavcodec/flac_parser.c
+++ b/libavcodec/flac_parser.c
@@ -34,7 +34,6 @@
 
 #include "libavutil/attributes.h"
 #include "libavutil/crc.h"
-#include "libavutil/fifo.h"
 #include "bytestream.h"
 #include "parser.h"
 #include "flac.h"
@@ -57,6 +56,15 @@
 #define MAX_FRAME_HEADER_SIZE 16
 #define MAX_FRAME_VERIFY_SIZE (MAX_FRAME_HEADER_SIZE + 1)
 
+typedef struct FifoBuffer {
+    uint8_t *buffer;
+    uint8_t *end;
+    uint8_t *rptr;
+    uint8_t *wptr;
+    size_t rndx;
+    size_t wndx;
+} FifoBuffer;
+
 typedef struct FLACHeaderMarker {
     int offset;       /**< byte offset from start of FLACParseContext->buffer */
     int link_penalty[FLAC_MAX_SEQUENTIAL_HEADERS]; /**< array of local scores
@@ -84,7 +92,7 @@ typedef struct FLACParseContext {
     int nb_headers_buffered;       /**< number of headers that are buffered   */
     int best_header_valid;         /**< flag set when the parser returns junk;
                                         if set return best_header next time   */
-    AVFifoBuffer *fifo_buf;        /**< buffer to store all data until headers
+    FifoBuffer fifo_buf;           /**< buffer to store all data until headers
                                         can be verified                       */
     int end_padded;                /**< specifies if fifo_buf's end is padded */
     uint8_t *wrap_buf;             /**< general fifo read buffer when wrapped */
@@ -127,6 +135,17 @@ static int frame_header_is_valid(AVCodecContext *avctx, const uint8_t *buf,
     return 1;
 }
 
+static size_t flac_fifo_size(FifoBuffer *f)
+{
+    av_assert0(f->wndx >= f->rndx);
+    return f->wndx - f->rndx;
+}
+
+static size_t flac_fifo_space(FifoBuffer *f)
+{
+    return f->end - f->buffer - flac_fifo_size(f);
+}
+
 /**
  * Non-destructive fast fifo pointer fetching
  * Returns a pointer from the specified offset.
@@ -143,7 +162,7 @@ static int frame_header_is_valid(AVCodecContext *avctx, const uint8_t *buf,
 static uint8_t *flac_fifo_read_wrap(FLACParseContext *fpc, int offset, int len,
                                     uint8_t **wrap_buf, int *allocated_size)
 {
-    AVFifoBuffer *f   = fpc->fifo_buf;
+    FifoBuffer   *f   = &fpc->fifo_buf;
     uint8_t *start    = f->rptr + offset;
     uint8_t *tmp_buf;
 
@@ -180,9 +199,8 @@ static uint8_t *flac_fifo_read_wrap(FLACParseContext *fpc, int offset, int len,
  * A second call to flac_fifo_read (with new offset and len) should be called
  * to get the post-wrap buf if the returned len is less than the requested.
  **/
-static uint8_t *flac_fifo_read(FLACParseContext *fpc, int offset, int *len)
+static uint8_t *flac_fifo_read(FifoBuffer *f, int offset, int *len)
 {
-    AVFifoBuffer *f   = fpc->fifo_buf;
     uint8_t *start    = f->rptr + offset;
 
     if (start >= f->end)
@@ -191,6 +209,106 @@ static uint8_t *flac_fifo_read(FLACParseContext *fpc, int offset, int *len)
     return start;
 }
 
+static int flac_fifo_grow(FifoBuffer *f, size_t inc)
+{
+    size_t size_old = f->end - f->buffer;
+    size_t offset_r = f->rptr - f->buffer;
+    size_t offset_w = f->wptr - f->buffer;
+    size_t size_new;
+
+    uint8_t *tmp;
+
+    if (size_old > SIZE_MAX - inc)
+        return AVERROR(EINVAL);
+    size_new = size_old + inc;
+
+    tmp = av_realloc(f->buffer, size_new);
+    if (!tmp)
+        return AVERROR(ENOMEM);
+
+    // move the data from the beginning of the ring buffer
+    // to the newly allocated space
+    // the second condition distinguishes full vs empty fifo
+    if (offset_w <= offset_r && flac_fifo_size(f)) {
+        const size_t copy = FFMIN(inc, offset_w);
+        memcpy(tmp + size_old, tmp, copy);
+        if (copy < offset_w) {
+            memmove(tmp, tmp + copy, offset_w - copy);
+            offset_w -= copy;
+        } else
+            offset_w = size_old + copy;
+    }
+
+    f->buffer = tmp;
+    f->end    = f->buffer + size_new;
+    f->rptr   = f->buffer + offset_r;
+    f->wptr   = f->buffer + offset_w;
+
+    return 0;
+}
+
+static int flac_fifo_write(FifoBuffer *f, const uint8_t *src, size_t size)
+{
+    uint32_t wndx = f->wndx;
+    uint8_t *wptr;
+
+    if (flac_fifo_space(f) < size) {
+        int ret = flac_fifo_grow(f, FFMAX(flac_fifo_size(f), size));
+        if (ret < 0)
+            return ret;
+    }
+
+    wptr = f->wptr;
+    do {
+        size_t len = FFMIN(f->end - wptr, size);
+        memcpy(wptr, src, len);
+        src  += len;
+        wptr += len;
+        if (wptr >= f->end)
+            wptr = f->buffer;
+        wndx    += len;
+        size    -= len;
+    } while (size > 0);
+
+    f->wndx = wndx;
+    f->wptr = wptr;
+
+    return 0;
+}
+
+static void flac_fifo_drain(FifoBuffer *f, size_t size)
+{
+    av_assert0(flac_fifo_size(f) >= size);
+    f->rptr += size;
+    if (f->rptr >= f->end)
+        f->rptr -= f->end - f->buffer;
+    f->rndx += size;
+}
+
+static int flac_fifo_alloc(FifoBuffer *f, size_t size)
+{
+    memset(f, 0, sizeof(*f));
+
+    f->buffer = av_realloc(NULL, size);
+    if (!f->buffer)
+        return AVERROR(ENOMEM);
+
+    f->wptr = f->buffer;
+    f->rptr = f->buffer;
+    f->end  = f->buffer + size;
+
+    f->rndx = 0;
+    f->wndx = 0;
+
+    return 0;
+}
+
+static void flac_fifo_free(FifoBuffer *f)
+{
+    av_freep(&f->buffer);
+    memset(f, 0, sizeof(*f));
+}
+
 static int find_headers_search_validate(FLACParseContext *fpc, int offset)
 {
     FLACFrameInfo fi;
@@ -263,9 +381,9 @@ static int find_new_headers(FLACParseContext *fpc, int search_start)
     fpc->nb_headers_found = 0;
 
     /* Search for a new header of at most 16 bytes. */
-    search_end = av_fifo_size(fpc->fifo_buf) - (MAX_FRAME_HEADER_SIZE - 1);
+    search_end = flac_fifo_size(&fpc->fifo_buf) - (MAX_FRAME_HEADER_SIZE - 1);
     read_len   = search_end - search_start + 1;
-    buf        = flac_fifo_read(fpc, search_start, &read_len);
+    buf        = flac_fifo_read(&fpc->fifo_buf, search_start, &read_len);
     size       = find_headers_search(fpc, buf, read_len, search_start);
     search_start += read_len - 1;
 
@@ -277,7 +395,7 @@ static int find_new_headers(FLACParseContext *fpc, int search_start)
         /* search_start + 1 is the post-wrap offset in the fifo. */
         read_len = search_end - (search_start + 1) + 1;
 
-        buf      = flac_fifo_read(fpc, search_start + 1, &read_len);
+        buf      = flac_fifo_read(&fpc->fifo_buf, search_start + 1, &read_len);
         wrap[1]  = buf[0];
 
         if ((AV_RB16(wrap) & 0xFFFE) == 0xFFF8) {
@@ -406,12 +524,12 @@ static int check_header_mismatch(FLACParseContext  *fpc,
             }
 
             read_len = end->offset - start->offset;
-            buf      = flac_fifo_read(fpc, start->offset, &read_len);
+            buf      = flac_fifo_read(&fpc->fifo_buf, start->offset, &read_len);
             crc      = av_crc(av_crc_get_table(AV_CRC_16_ANSI), 0, buf, read_len);
             read_len = (end->offset - start->offset) - read_len;
 
             if (read_len) {
-                buf = flac_fifo_read(fpc, end->offset - read_len, &read_len);
+                buf = flac_fifo_read(&fpc->fifo_buf, end->offset - read_len, &read_len);
                 crc = av_crc(av_crc_get_table(AV_CRC_16_ANSI), crc, buf, read_len);
             }
         }
@@ -500,7 +618,7 @@ static int get_best_header(FLACParseContext *fpc, const uint8_t **poutbuf,
     FLACHeaderMarker *header = fpc->best_header;
     FLACHeaderMarker *child  = header->best_child;
     if (!child) {
-        *poutbuf_size = av_fifo_size(fpc->fifo_buf) - header->offset;
+        *poutbuf_size = flac_fifo_size(&fpc->fifo_buf) - header->offset;
     } else {
         *poutbuf_size = child->offset - header->offset;
 
@@ -519,7 +637,6 @@ static int get_best_header(FLACParseContext *fpc, const uint8_t **poutbuf,
                                         &fpc->wrap_buf,
                                         &fpc->wrap_buf_allocated_size);
 
-
     if (fpc->pc->flags & PARSER_FLAG_USE_CODEC_TS) {
         if (header->fi.is_var_size)
           fpc->pc->pts = header->fi.frame_or_sample_num;
@@ -534,7 +651,7 @@ static int get_best_header(FLACParseContext *fpc, const uint8_t **poutbuf,
     /* Return the negative overread index so the client can compute pos.
        This should be the amount overread to the beginning of the child */
     if (child)
-        return child->offset - av_fifo_size(fpc->fifo_buf);
+        return child->offset - flac_fifo_size(&fpc->fifo_buf);
     return 0;
 }
 
@@ -586,7 +703,7 @@ static int flac_parse(AVCodecParserContext *s, AVCodecContext *avctx,
             fpc->nb_headers_buffered--;
         }
         /* Release returned data from ring buffer. */
-        av_fifo_drain(fpc->fifo_buf, best_child->offset);
+        flac_fifo_drain(&fpc->fifo_buf, best_child->offset);
 
         /* Fix the offset for the headers remaining to match the new buffer. */
         for (curr = best_child->next; curr; curr = curr->next)
@@ -620,7 +737,7 @@ static int flac_parse(AVCodecParserContext *s, AVCodecContext *avctx,
     while ((buf_size && read_end < buf + buf_size &&
             fpc->nb_headers_buffered < FLAC_MIN_HEADERS)
            || (!buf_size && !fpc->end_padded)) {
-        int start_offset;
+        int start_offset, ret;
 
         /* Pad the end once if EOF, to check the final region for headers. */
         if (!buf_size) {
@@ -634,8 +751,8 @@ static int flac_parse(AVCodecParserContext *s, AVCodecContext *avctx,
                                               nb_desired * FLAC_AVG_FRAME_SIZE);
         }
 
-        if (!av_fifo_space(fpc->fifo_buf) &&
-            av_fifo_size(fpc->fifo_buf) / FLAC_AVG_FRAME_SIZE >
+        if (!flac_fifo_space(&fpc->fifo_buf) &&
+            flac_fifo_size(&fpc->fifo_buf) / FLAC_AVG_FRAME_SIZE >
             fpc->nb_headers_buffered * 20) {
             /* There is less than one valid flac header buffered for 20 headers
              * buffered. Therefore the fifo is most likely filled with invalid
@@ -644,24 +761,20 @@ static int flac_parse(AVCodecParserContext *s, AVCodecContext *avctx,
         }
 
         /* Fill the buffer. */
-        if (   av_fifo_space(fpc->fifo_buf) < read_end - read_start
-            && av_fifo_realloc2(fpc->fifo_buf, (read_end - read_start) + 2*av_fifo_size(fpc->fifo_buf)) < 0) {
-            av_log(avctx, AV_LOG_ERROR,
-                   "couldn't reallocate buffer of size %"PTRDIFF_SPECIFIER"\n",
-                   (read_end - read_start) + av_fifo_size(fpc->fifo_buf));
-            goto handle_error;
-        }
-
         if (buf_size) {
-            av_fifo_generic_write(fpc->fifo_buf, (void*) read_start,
-                                  read_end - read_start, NULL);
+            ret = flac_fifo_write(&fpc->fifo_buf, read_start,
+                                  read_end - read_start);
         } else {
             int8_t pad[MAX_FRAME_HEADER_SIZE] = { 0 };
-            av_fifo_generic_write(fpc->fifo_buf, pad, sizeof(pad), NULL);
+            ret = flac_fifo_write(&fpc->fifo_buf, pad, sizeof(pad));
+        }
+        if (ret < 0) {
+            av_log(avctx, AV_LOG_ERROR, "Error buffering data\n");
+            goto handle_error;
         }
 
         /* Tag headers and update sequences. */
-        start_offset = av_fifo_size(fpc->fifo_buf) -
+        start_offset = flac_fifo_size(&fpc->fifo_buf) -
                        ((read_end - read_start) + (MAX_FRAME_HEADER_SIZE - 1));
         start_offset = FFMAX(0, start_offset);
         nb_headers   = find_new_headers(fpc, start_offset);
@@ -689,13 +802,13 @@ static int flac_parse(AVCodecParserContext *s, AVCodecContext *avctx,
 
         /* restore the state pre-padding */
         if (fpc->end_padded) {
-            int warp = fpc->fifo_buf->wptr - fpc->fifo_buf->buffer < MAX_FRAME_HEADER_SIZE;
+            int warp = fpc->fifo_buf.wptr - fpc->fifo_buf.buffer < MAX_FRAME_HEADER_SIZE;
             /* HACK: drain the tail of the fifo */
-            fpc->fifo_buf->wptr -= MAX_FRAME_HEADER_SIZE;
-            fpc->fifo_buf->wndx -= MAX_FRAME_HEADER_SIZE;
+            fpc->fifo_buf.wptr -= MAX_FRAME_HEADER_SIZE;
+            fpc->fifo_buf.wndx -= MAX_FRAME_HEADER_SIZE;
             if (warp) {
-                fpc->fifo_buf->wptr += fpc->fifo_buf->end -
-                    fpc->fifo_buf->buffer;
+                fpc->fifo_buf.wptr += fpc->fifo_buf.end -
+                    fpc->fifo_buf.buffer;
             }
             read_start = read_end = NULL;
         }
@@ -727,7 +840,7 @@ static int flac_parse(AVCodecParserContext *s, AVCodecContext *avctx,
                                                 &fpc->wrap_buf,
                                                 &fpc->wrap_buf_allocated_size);
             return buf_size ? (read_end - buf) : (fpc->best_header->offset -
-                                                  av_fifo_size(fpc->fifo_buf));
+                                                  flac_fifo_size(&fpc->fifo_buf));
         }
         if (!buf_size)
             return get_best_header(fpc, poutbuf, poutbuf_size);
@@ -742,11 +855,13 @@ handle_error:
 static av_cold int flac_parse_init(AVCodecParserContext *c)
 {
     FLACParseContext *fpc = c->priv_data;
+    int ret;
+
     fpc->pc = c;
     /* There will generally be FLAC_MIN_HEADERS buffered in the fifo before
        it drains.  This is allocated early to avoid slow reallocation. */
-    fpc->fifo_buf = av_fifo_alloc_array(FLAC_MIN_HEADERS + 3, FLAC_AVG_FRAME_SIZE);
-    if (!fpc->fifo_buf) {
+    ret = flac_fifo_alloc(&fpc->fifo_buf, (FLAC_MIN_HEADERS + 3) * FLAC_AVG_FRAME_SIZE);
+    if (ret < 0) {
         av_log(fpc->avctx, AV_LOG_ERROR,
                 "couldn't allocate fifo_buf\n");
         return AVERROR(ENOMEM);
@@ -765,7 +880,7 @@ static void flac_parse_close(AVCodecParserContext *c)
         curr = temp;
     }
     fpc->headers = NULL;
-    av_fifo_freep(&fpc->fifo_buf);
+    flac_fifo_free(&fpc->fifo_buf);
     av_freep(&fpc->wrap_buf);
 }
 
-- 
2.33.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".

^ permalink raw reply	[flat|nested] 10+ messages in thread