[FFmpeg-devel] [PATCH 2/3] lavu/fifo: clarify interaction of AV_FIFO_FLAG_AUTO_GROW with av_fifo_can_write()

[James Almer]

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.

>    */
>   size_t av_fifo_can_write(const AVFifo *f);
>