Message ID | 20220111204610.14262-8-anton@khirnov.net |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,01/35] lavu/fifo: disallow overly large fifo sizes | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
andriy/make_ppc | success | Make finished |
andriy/make_fate_ppc | success | Make fate finished |
andriy/make_aarch64_jetson | success | Make finished |
andriy/make_fate_aarch64_jetson | success | Make fate finished |
Anton Khirnov: > As for writing, use separate functions for reading to a buffer and a > callback. Allow the callbacks to limit the amount of data read, > similarly to what is done for writing. > > Consistently use size_t for sizes. > --- > doc/APIchanges | 3 ++- > libavutil/fifo.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++ > libavutil/fifo.h | 60 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 130 insertions(+), 1 deletion(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index 0b179c30e5..f64759d69d 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -19,7 +19,8 @@ API changes, most recent first: > 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(), > - av_fifo_grow2(), av_fifo_drain2(), av_fifo_write(), av_fifo_write_from_cb(). > + av_fifo_grow2(), av_fifo_drain2(), av_fifo_write(), av_fifo_write_from_cb(), > + av_fifo_read(), av_fifo_read_to_cb(), av_fifo_peek(), av_fifo_peek_to_cb(). > > 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 1d94fff457..ea944bc936 100644 > --- a/libavutil/fifo.c > +++ b/libavutil/fifo.c > @@ -265,6 +265,74 @@ int av_fifo_write_from_cb(AVFifoBuffer *f, AVFifoCB read_cb, > return fifo_write_common(f, NULL, nb_elems, read_cb, opaque); > } > > +static int fifo_peek_common(const AVFifoBuffer *f, uint8_t *buf, size_t *nb_elems, > + size_t offset, AVFifoCB write_cb, void *opaque) > +{ > + const FifoBuffer *fb = (FifoBuffer*)f; > + size_t to_read = *nb_elems; > + size_t offset_r = fb->offset_r; > + int ret = 0; > + > + if (offset > av_fifo_can_read(f) || > + to_read > av_fifo_can_read(f) - offset) { You are calling av_fifo_can_read() multiple times. > + *nb_elems = 0; > + return AVERROR(EINVAL); > + } > + > + if (offset_r >= fb->nb_elems - offset) > + offset_r -= fb->nb_elems - offset; > + else > + offset_r += offset; > + > + do { Shouldn't this be a while loop? > + size_t len = FFMIN(fb->nb_elems - offset_r, to_read); > + uint8_t *rptr = f->buffer + offset_r * fb->elem_size; > + > + if (write_cb) { > + ret = write_cb(opaque, rptr, &len); > + if (ret < 0 || len == 0) > + break; > + } else { > + memcpy(buf, rptr, len * fb->elem_size); > + buf += len * fb->elem_size; > + } > + offset_r += len; > + if (offset_r >= fb->nb_elems) > + offset_r = 0; > + to_read -= len; > + } while (to_read > 0); > + > + *nb_elems -= to_read; > + > + return ret; > +} > + > +int av_fifo_read(AVFifoBuffer *f, void *buf, size_t nb_elems) > +{ > + int ret = fifo_peek_common(f, buf, &nb_elems, 0, NULL, NULL); > + av_fifo_drain2(f, nb_elems); In contrast to the current av_fifo_generic_read() the callback will still see the non-drained FIFO if the callback is called multiple times. I don't know whether this slight behaviour change can cause problems when updating. > + return ret; > +} > + > +int av_fifo_read_to_cb(AVFifoBuffer *f, AVFifoCB write_cb, > + void *opaque, size_t *nb_elems) > +{ > + int ret = fifo_peek_common(f, NULL, nb_elems, 0, write_cb, opaque); > + av_fifo_drain2(f, *nb_elems); > + return ret; > +} > + > +int av_fifo_peek(AVFifoBuffer *f, void *buf, size_t nb_elems, size_t offset) > +{ > + return fifo_peek_common(f, buf, &nb_elems, offset, NULL, NULL); > +} > + > +int av_fifo_peek_to_cb(AVFifoBuffer *f, AVFifoCB write_cb, void *opaque, > + size_t *nb_elems, size_t offset) > +{ > + return fifo_peek_common(f, NULL, nb_elems, offset, write_cb, opaque); > +} > + > int av_fifo_realloc2(AVFifoBuffer *f, unsigned int new_size) > { > FifoBuffer *fb = (FifoBuffer*)f; > diff --git a/libavutil/fifo.h b/libavutil/fifo.h > index ac1245ff39..c7be5e8f7d 100644 > --- a/libavutil/fifo.h > +++ b/libavutil/fifo.h > @@ -189,6 +189,66 @@ int av_fifo_write(AVFifoBuffer *f, const void *buf, size_t nb_elems); > int av_fifo_write_from_cb(AVFifoBuffer *f, AVFifoCB read_cb, > void *opaque, size_t *nb_elems); > > +/** > + * Read data from a FIFO. > + * > + * @param f the FIFO buffer > + * @param buf Buffer to store the data. nb_elems * av_fifo_elem_size(f) bytes > + * will be written into buf. > + * @param nb_elems number of elements to read from FIFO > + * > + * @return a non-negative number on success, a negative error code on failure > + */ > +int av_fifo_read(AVFifoBuffer *f, void *buf, size_t nb_elems); > + > +/** > + * Feed data from a FIFO into a user-provided callback. > + * > + * @param f the FIFO buffer > + * @param write_cb Callback the data will be supplied to. May be called > + * multiple times. > + * @param opaque opaque user data to be provided to write_cb > + * @param nb_elems Should point to the maximum number of elements that can be > + * read. Will be updated to contain the total number of elements > + * actually sent to the callback. > + * > + * @return non-negative number on success, a negative error code on failure > + */ > +int av_fifo_read_to_cb(AVFifoBuffer *f, AVFifoCB write_cb, > + void *opaque, size_t *nb_elems); > + > +/** > + * Read data from a FIFO without modifying FIFO state. > + * > + * @param f the FIFO buffer > + * @param buf Buffer to store the data. nb_elems * av_fifo_elem_size(f) bytes > + * will be written into buf. > + * @param nb_elems number of elements to read from FIFO > + * @param offset number of initial elements to skip; offset + nb_elems must not > + * be larger than av_fifo_can_read(f). > + * > + * @return a non-negative number on success, a negative error code on failure > + */ > +int av_fifo_peek(AVFifoBuffer *f, void *buf, size_t nb_elems, size_t offset); > + > +/** > + * Feed data from a FIFO into a user-provided callback. > + * > + * @param f the FIFO buffer > + * @param write_cb Callback the data will be supplied to. May be called > + * multiple times. > + * @param opaque opaque user data to be provided to write_cb > + * @param nb_elems Should point to the maximum number of elements that can be > + * read. Will be updated to contain the total number of elements > + * actually sent to the callback. > + * @param offset number of initial elements to skip; offset + *nb_elems must not > + * be larger than av_fifo_can_read(f). > + * > + * @return a non-negative number on success, a negative error code on failure > + */ > +int av_fifo_peek_to_cb(AVFifoBuffer *f, AVFifoCB write_cb, void *opaque, Ok, seems like we differ on our naming: I'd call this a read_cb, because it reads from the fifo, but it of course also writes, so it seems fine given that you are consistent. > + size_t *nb_elems, size_t offset); > + > /** > * Discard the specified amount of data from an AVFifoBuffer. > * @param size number of elements to discard, MUST NOT be larger than >
diff --git a/doc/APIchanges b/doc/APIchanges index 0b179c30e5..f64759d69d 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -19,7 +19,8 @@ API changes, most recent first: 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(), - av_fifo_grow2(), av_fifo_drain2(), av_fifo_write(), av_fifo_write_from_cb(). + av_fifo_grow2(), av_fifo_drain2(), av_fifo_write(), av_fifo_write_from_cb(), + av_fifo_read(), av_fifo_read_to_cb(), av_fifo_peek(), av_fifo_peek_to_cb(). 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 1d94fff457..ea944bc936 100644 --- a/libavutil/fifo.c +++ b/libavutil/fifo.c @@ -265,6 +265,74 @@ int av_fifo_write_from_cb(AVFifoBuffer *f, AVFifoCB read_cb, return fifo_write_common(f, NULL, nb_elems, read_cb, opaque); } +static int fifo_peek_common(const AVFifoBuffer *f, uint8_t *buf, size_t *nb_elems, + size_t offset, AVFifoCB write_cb, void *opaque) +{ + const FifoBuffer *fb = (FifoBuffer*)f; + size_t to_read = *nb_elems; + size_t offset_r = fb->offset_r; + int ret = 0; + + if (offset > av_fifo_can_read(f) || + to_read > av_fifo_can_read(f) - offset) { + *nb_elems = 0; + return AVERROR(EINVAL); + } + + if (offset_r >= fb->nb_elems - offset) + offset_r -= fb->nb_elems - offset; + else + offset_r += offset; + + do { + size_t len = FFMIN(fb->nb_elems - offset_r, to_read); + uint8_t *rptr = f->buffer + offset_r * fb->elem_size; + + if (write_cb) { + ret = write_cb(opaque, rptr, &len); + if (ret < 0 || len == 0) + break; + } else { + memcpy(buf, rptr, len * fb->elem_size); + buf += len * fb->elem_size; + } + offset_r += len; + if (offset_r >= fb->nb_elems) + offset_r = 0; + to_read -= len; + } while (to_read > 0); + + *nb_elems -= to_read; + + return ret; +} + +int av_fifo_read(AVFifoBuffer *f, void *buf, size_t nb_elems) +{ + int ret = fifo_peek_common(f, buf, &nb_elems, 0, NULL, NULL); + av_fifo_drain2(f, nb_elems); + return ret; +} + +int av_fifo_read_to_cb(AVFifoBuffer *f, AVFifoCB write_cb, + void *opaque, size_t *nb_elems) +{ + int ret = fifo_peek_common(f, NULL, nb_elems, 0, write_cb, opaque); + av_fifo_drain2(f, *nb_elems); + return ret; +} + +int av_fifo_peek(AVFifoBuffer *f, void *buf, size_t nb_elems, size_t offset) +{ + return fifo_peek_common(f, buf, &nb_elems, offset, NULL, NULL); +} + +int av_fifo_peek_to_cb(AVFifoBuffer *f, AVFifoCB write_cb, void *opaque, + size_t *nb_elems, size_t offset) +{ + return fifo_peek_common(f, NULL, nb_elems, offset, write_cb, opaque); +} + int av_fifo_realloc2(AVFifoBuffer *f, unsigned int new_size) { FifoBuffer *fb = (FifoBuffer*)f; diff --git a/libavutil/fifo.h b/libavutil/fifo.h index ac1245ff39..c7be5e8f7d 100644 --- a/libavutil/fifo.h +++ b/libavutil/fifo.h @@ -189,6 +189,66 @@ int av_fifo_write(AVFifoBuffer *f, const void *buf, size_t nb_elems); int av_fifo_write_from_cb(AVFifoBuffer *f, AVFifoCB read_cb, void *opaque, size_t *nb_elems); +/** + * Read data from a FIFO. + * + * @param f the FIFO buffer + * @param buf Buffer to store the data. nb_elems * av_fifo_elem_size(f) bytes + * will be written into buf. + * @param nb_elems number of elements to read from FIFO + * + * @return a non-negative number on success, a negative error code on failure + */ +int av_fifo_read(AVFifoBuffer *f, void *buf, size_t nb_elems); + +/** + * Feed data from a FIFO into a user-provided callback. + * + * @param f the FIFO buffer + * @param write_cb Callback the data will be supplied to. May be called + * multiple times. + * @param opaque opaque user data to be provided to write_cb + * @param nb_elems Should point to the maximum number of elements that can be + * read. Will be updated to contain the total number of elements + * actually sent to the callback. + * + * @return non-negative number on success, a negative error code on failure + */ +int av_fifo_read_to_cb(AVFifoBuffer *f, AVFifoCB write_cb, + void *opaque, size_t *nb_elems); + +/** + * Read data from a FIFO without modifying FIFO state. + * + * @param f the FIFO buffer + * @param buf Buffer to store the data. nb_elems * av_fifo_elem_size(f) bytes + * will be written into buf. + * @param nb_elems number of elements to read from FIFO + * @param offset number of initial elements to skip; offset + nb_elems must not + * be larger than av_fifo_can_read(f). + * + * @return a non-negative number on success, a negative error code on failure + */ +int av_fifo_peek(AVFifoBuffer *f, void *buf, size_t nb_elems, size_t offset); + +/** + * Feed data from a FIFO into a user-provided callback. + * + * @param f the FIFO buffer + * @param write_cb Callback the data will be supplied to. May be called + * multiple times. + * @param opaque opaque user data to be provided to write_cb + * @param nb_elems Should point to the maximum number of elements that can be + * read. Will be updated to contain the total number of elements + * actually sent to the callback. + * @param offset number of initial elements to skip; offset + *nb_elems must not + * be larger than av_fifo_can_read(f). + * + * @return a non-negative number on success, a negative error code on failure + */ +int av_fifo_peek_to_cb(AVFifoBuffer *f, AVFifoCB write_cb, void *opaque, + size_t *nb_elems, size_t offset); + /** * Discard the specified amount of data from an AVFifoBuffer. * @param size number of elements to discard, MUST NOT be larger than