On 8/29/2022 12:00 PM, James Almer wrote:
On 8/29/2022 11:07 AM, Anton Khirnov wrote:
---
libavutil/fifo.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/libavutil/fifo.h b/libavutil/fifo.h
index 6c6bd78842..89872d0972 100644
--- a/libavutil/fifo.h
+++ b/libavutil/fifo.h
@@ -97,7 +97,13 @@ void av_fifo_auto_grow_limit(AVFifo *f, size_t
max_elems);
size_t av_fifo_can_read(const AVFifo *f);
/**
- * @return number of elements that can be written into the given FIFO.
+ * @return Number of elements that can be written into the given FIFO
without
+ * growing it.
+ *
+ * In other words, this number of elements or less is
guaranteed to fit
+ * into the FIFO. More data may be written when the
+ * AV_FIFO_FLAG_AUTO_GROW flag was specified at FIFO
creation, but this
+ * may involve memory allocation, which can fail.
This patch is an API break, because before it i was told
av_fifo_can_write() would tell me the amount of elements i could write
into the FIFO, regardless of how it was created, but now it legitimates
the one scenario where it was not reliable. An scenario i stumbled upon
in my code by following the documentation, which is in at least one
release, the LTS one.
Instead of changing the documentation to fit the behavior, the behavior
should match the documentation. This means that if a call to
av_fifo_write() can succeed, then av_fifo_can_write() should reflect that.
That said, it would be great if making av_fifo_can_write() tell the real
amount of elements one can write into the FIFO was possible without
breaking anything, but the doxy for av_fifo_grow2() says "On success,
the FIFO will be large enough to hold exactly inc + av_fifo_can_read() +
av_fifo_can_write()", a line that was obviously aware of the fact
av_fifo_can_write() ignored the autogrow feature, and would no longer be
true if said function is fixed.
This could have been avoided if we added an av_fifo_size2() function
that returned nb_elems, so the line above may have been replaced by one
simply referring the user to it. But as is, we're breaking the API no
matter what we do.
Something like the following is the alternative. It's going to be a
break in one way or another no matter what we do.
diff --git a/libavutil/fifo.c b/libavutil/fifo.c
index 51a5af6f39..3fc76b4247 100644
--- a/libavutil/fifo.c
+++ b/libavutil/fifo.c
@@ -79,6 +79,11 @@ void av_fifo_auto_grow_limit(AVFifo *f, size_t max_elems)
f->auto_grow_limit = max_elems;
}
+size_t av_fifo_size2(const AVFifo *f)
+{
+ return f->nb_elems;
+}
+
size_t av_fifo_elem_size(const AVFifo *f)
{
return f->elem_size;
@@ -93,7 +98,14 @@ size_t av_fifo_can_read(const AVFifo *f)
size_t av_fifo_can_write(const AVFifo *f)
{
- return f->nb_elems - av_fifo_can_read(f);
+ size_t nb_elems = f->nb_elems;
+
+ if (f->flags & AV_FIFO_FLAG_AUTO_GROW) {
+ size_t autogrow = f->auto_grow_limit > nb_elems ?
+ f->auto_grow_limit - nb_elems : 0;
+ nb_elems += autogrow;
+ }
+ return nb_elems - av_fifo_can_read(f);
}
int av_fifo_grow2(AVFifo *f, size_t inc)
diff --git a/libavutil/fifo.h b/libavutil/fifo.h
index 4eed364afc..0f909aac55 100644
--- a/libavutil/fifo.h
+++ b/libavutil/fifo.h
@@ -70,6 +70,11 @@ typedef int AVFifoCB(void *opaque, void *buf, size_t
*nb_elems);
AVFifo *av_fifo_alloc2(size_t elems, size_t elem_size,
unsigned int flags);
+/**
+ * @return Total number of elements the given FIFO can currently hold.
+ */
+size_t av_fifo_size2(const AVFifo *f);
+
/**
* @return Element size for FIFO operations. This element size is set at
* FIFO allocation and remains constant during its lifetime
@@ -89,20 +94,22 @@ size_t av_fifo_can_read(const AVFifo *f);
/**
* @return number of elements that can be written into the given FIFO.
+ * @note If the given FIFO was allocated with AV_FIFO_FLAG_AUTO_GROW, the
+ * result of av_fifo_size2(f) - av_fifo_can_read(f) is the amount
+ * of elements that can be written into it without the chance of
+ * failure.
*/
size_t av_fifo_can_write(const AVFifo *f);
/**
* Enlarge an AVFifo.
*
- * 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 AVFifo 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
+ * @return a non-negative number on success, a negative error code on failure.
+ * In case of failure, the old FIFO is kept unchanged.
+ * @see av_fifo_size2()
*/
int av_fifo_grow2(AVFifo *f, size_t inc);
@@ -112,6 +119,9 @@ int av_fifo_grow2(AVFifo *f, size_t inc);
* In case nb_elems > av_fifo_can_write(f), nothing is written and an error
* is returned.
*
+ * Calling this function is guaranteed to succeed if
+ * nb_elems <= av_fifo_size2(f) - av_fifo_can_read(f).
+ *
* @param f the FIFO buffer
* @param buf Data to be written. nb_elems * av_fifo_elem_size(f) bytes will be
* read from buf 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-requ...@ffmpeg.org with subject "unsubscribe".
