Message ID | 20220111204610.14262-5-anton@khirnov.net |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,01/35] lavu/fifo: disallow overly large fifo sizes | expand |
On 1/11/2022 5:45 PM, Anton Khirnov wrote: > Consistently use size_t for sizes. > > Unlike av_fifo_grow(), which addds to the currently used size, this > function adds to the allocated size. > > No new function is provided for a generic realloc, since the current code > only supports increasing the FIFO size. > --- > doc/APIchanges | 3 ++- > libavutil/fifo.c | 61 +++++++++++++++++++++++++++--------------------- > libavutil/fifo.h | 14 +++++++++++ > 3 files changed, 50 insertions(+), 28 deletions(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index 9400c5147a..98eae55719 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -18,7 +18,8 @@ API changes, most recent first: > Add av_fifo_alloc2(), which allows setting a FIFO element size. > Operations on FIFOs created with this function on these elements > rather than bytes. > - Add av_fifo_elem_size(), av_fifo_can_read(), av_fifo_can_write(). > + Add av_fifo_elem_size(), av_fifo_can_read(), av_fifo_can_write(), > + av_fifo_grow2(). > > 2022-01-xx - xxxxxxxxxx - lavu fifo.h > Access to all AVFifoBuffer members is deprecated. The struct will > diff --git a/libavutil/fifo.c b/libavutil/fifo.c > index 8cde2c20e1..28d6d1b2e6 100644 > --- a/libavutil/fifo.c > +++ b/libavutil/fifo.c > @@ -150,41 +150,48 @@ int av_fifo_space(const AVFifoBuffer *f) > return av_fifo_can_write(f); > } > > -int av_fifo_realloc2(AVFifoBuffer *f, unsigned int new_size) > +int av_fifo_grow2(AVFifoBuffer *f, size_t inc) > { > FifoBuffer *fb = (FifoBuffer*)f; > + uint8_t *tmp; > > - if (new_size > FIFO_SIZE_MAX) > + if (inc > FIFO_SIZE_MAX - fb->nb_elems) > return AVERROR(EINVAL); > > - if (fb->nb_elems < new_size) { > - uint8_t *tmp; > - > - tmp = av_realloc_array(f->buffer, new_size, fb->elem_size); > - if (!tmp) > - return AVERROR(ENOMEM); > - > - // move the data from the beginning of the ring buffer > - // to the newly allocated space > - if (fb->offset_w <= fb->offset_r && !fb->is_empty) { > - const size_t copy = FFMIN(new_size - fb->nb_elems, fb->offset_w); > - memcpy(tmp + fb->nb_elems * fb->elem_size, tmp, copy * fb->elem_size); > - if (copy < fb->offset_w) { > - memmove(tmp, tmp + copy * fb->elem_size, > - (fb->offset_w - copy) * fb->elem_size); > - fb->offset_w -= copy; > - } else > - fb->offset_w = fb->nb_elems + copy; > - } > + tmp = av_realloc_array(f->buffer, fb->nb_elems + inc, fb->elem_size); This sounds like a good opportunity to introduce av_fast_realloc_array() in mem.h, with FifoBuffer being a private struct where you can safely store the required buffer size field. av_realloc_array() can be slow in an auto-grow scenario if there's a lot of buffering before stuff starts being drained. > + if (!tmp) > + return AVERROR(ENOMEM); > + > + // move the data from the beginning of the ring buffer > + // to the newly allocated space > + if (fb->offset_w <= fb->offset_r && !fb->is_empty) { > + const size_t copy = FFMIN(inc, fb->offset_w); > + memcpy(tmp + fb->nb_elems * fb->elem_size, tmp, copy * fb->elem_size); > + if (copy < fb->offset_w) { > + memmove(tmp, tmp + copy * fb->elem_size, > + (fb->offset_w - copy) * fb->elem_size); > + fb->offset_w -= copy; > + } else > + fb->offset_w = fb->nb_elems + copy; > + } > + > + f->buffer = tmp; > + fb->nb_elems += inc; > > - f->buffer = tmp; > #if FF_API_FIFO_PUBLIC > - f->end = f->buffer + new_size; > - f->rptr = f->buffer + fb->offset_r * fb->elem_size; > - f->wptr = f->buffer + fb->offset_w * fb->elem_size; > + f->end = f->buffer + fb->nb_elems * fb->elem_size; > + f->rptr = f->buffer + fb->offset_r * fb->elem_size; > + f->wptr = f->buffer + fb->offset_w * fb->elem_size; > #endif > - fb->nb_elems = new_size; > - } > + > + return 0; > +} > + > +int av_fifo_realloc2(AVFifoBuffer *f, unsigned int new_size) > +{ > + FifoBuffer *fb = (FifoBuffer*)f; > + if (fb->nb_elems < new_size) > + return av_fifo_grow2(f, new_size - fb->nb_elems); > return 0; > } > > diff --git a/libavutil/fifo.h b/libavutil/fifo.h > index 9e78082b3b..375d0d133b 100644 > --- a/libavutil/fifo.h > +++ b/libavutil/fifo.h > @@ -129,6 +129,20 @@ size_t av_fifo_can_read(const AVFifoBuffer *f); > */ > size_t av_fifo_can_write(const AVFifoBuffer *f); > > +/** > + * Enlarge an AVFifoBuffer. > + * > + * On success, the FIFO will be large enough to hold exactly > + * inc + av_fifo_can_read() + av_fifo_can_write() > + * elements. In case of failure, the old FIFO is kept unchanged. > + * > + * @param f AVFifoBuffer to resize > + * @param inc number of elements to allocate for, in addition to the current > + * allocated size > + * @return a non-negative number on success, a negative error code on failure > + */ > +int av_fifo_grow2(AVFifoBuffer *f, size_t inc); > + > /** > * Feed data at specific position from an AVFifoBuffer to a user-supplied callback. > * Similar as av_fifo_gereric_read but without discarding data.
diff --git a/doc/APIchanges b/doc/APIchanges index 9400c5147a..98eae55719 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -18,7 +18,8 @@ API changes, most recent first: Add av_fifo_alloc2(), which allows setting a FIFO element size. Operations on FIFOs created with this function on these elements rather than bytes. - Add av_fifo_elem_size(), av_fifo_can_read(), av_fifo_can_write(). + Add av_fifo_elem_size(), av_fifo_can_read(), av_fifo_can_write(), + av_fifo_grow2(). 2022-01-xx - xxxxxxxxxx - lavu fifo.h Access to all AVFifoBuffer members is deprecated. The struct will diff --git a/libavutil/fifo.c b/libavutil/fifo.c index 8cde2c20e1..28d6d1b2e6 100644 --- a/libavutil/fifo.c +++ b/libavutil/fifo.c @@ -150,41 +150,48 @@ int av_fifo_space(const AVFifoBuffer *f) return av_fifo_can_write(f); } -int av_fifo_realloc2(AVFifoBuffer *f, unsigned int new_size) +int av_fifo_grow2(AVFifoBuffer *f, size_t inc) { FifoBuffer *fb = (FifoBuffer*)f; + uint8_t *tmp; - if (new_size > FIFO_SIZE_MAX) + if (inc > FIFO_SIZE_MAX - fb->nb_elems) return AVERROR(EINVAL); - if (fb->nb_elems < new_size) { - uint8_t *tmp; - - tmp = av_realloc_array(f->buffer, new_size, fb->elem_size); - if (!tmp) - return AVERROR(ENOMEM); - - // move the data from the beginning of the ring buffer - // to the newly allocated space - if (fb->offset_w <= fb->offset_r && !fb->is_empty) { - const size_t copy = FFMIN(new_size - fb->nb_elems, fb->offset_w); - memcpy(tmp + fb->nb_elems * fb->elem_size, tmp, copy * fb->elem_size); - if (copy < fb->offset_w) { - memmove(tmp, tmp + copy * fb->elem_size, - (fb->offset_w - copy) * fb->elem_size); - fb->offset_w -= copy; - } else - fb->offset_w = fb->nb_elems + copy; - } + tmp = av_realloc_array(f->buffer, fb->nb_elems + inc, fb->elem_size); + if (!tmp) + return AVERROR(ENOMEM); + + // move the data from the beginning of the ring buffer + // to the newly allocated space + if (fb->offset_w <= fb->offset_r && !fb->is_empty) { + const size_t copy = FFMIN(inc, fb->offset_w); + memcpy(tmp + fb->nb_elems * fb->elem_size, tmp, copy * fb->elem_size); + if (copy < fb->offset_w) { + memmove(tmp, tmp + copy * fb->elem_size, + (fb->offset_w - copy) * fb->elem_size); + fb->offset_w -= copy; + } else + fb->offset_w = fb->nb_elems + copy; + } + + f->buffer = tmp; + fb->nb_elems += inc; - f->buffer = tmp; #if FF_API_FIFO_PUBLIC - f->end = f->buffer + new_size; - f->rptr = f->buffer + fb->offset_r * fb->elem_size; - f->wptr = f->buffer + fb->offset_w * fb->elem_size; + f->end = f->buffer + fb->nb_elems * fb->elem_size; + f->rptr = f->buffer + fb->offset_r * fb->elem_size; + f->wptr = f->buffer + fb->offset_w * fb->elem_size; #endif - fb->nb_elems = new_size; - } + + return 0; +} + +int av_fifo_realloc2(AVFifoBuffer *f, unsigned int new_size) +{ + FifoBuffer *fb = (FifoBuffer*)f; + if (fb->nb_elems < new_size) + return av_fifo_grow2(f, new_size - fb->nb_elems); return 0; } diff --git a/libavutil/fifo.h b/libavutil/fifo.h index 9e78082b3b..375d0d133b 100644 --- a/libavutil/fifo.h +++ b/libavutil/fifo.h @@ -129,6 +129,20 @@ size_t av_fifo_can_read(const AVFifoBuffer *f); */ size_t av_fifo_can_write(const AVFifoBuffer *f); +/** + * Enlarge an AVFifoBuffer. + * + * On success, the FIFO will be large enough to hold exactly + * inc + av_fifo_can_read() + av_fifo_can_write() + * elements. In case of failure, the old FIFO is kept unchanged. + * + * @param f AVFifoBuffer to resize + * @param inc number of elements to allocate for, in addition to the current + * allocated size + * @return a non-negative number on success, a negative error code on failure + */ +int av_fifo_grow2(AVFifoBuffer *f, size_t inc); + /** * Feed data at specific position from an AVFifoBuffer to a user-supplied callback. * Similar as av_fifo_gereric_read but without discarding data.