[FFmpeg-devel,02/35] lavu/fifo: make the contents of AVFifoBuffer private on next major bump

There should be no good reason for the callers to access any of its
contents.

Define a new type for the internal struct that currently matches
AVFifoBuffer. This will allow adding new private fields without waiting
for the major bump and will be useful in the following commits.

Unfortunately AVFifoBuffer fields cannot be marked as deprecated because
it would trigger a warning wherever fifo.h is #included, due to
inlined av_fifo_peek2().
---
 doc/APIchanges         |  4 ++++
 libavutil/fifo.c       | 23 +++++++++++++++++++----
 libavutil/fifo.h       | 14 ++++++++++++--
 libavutil/tests/fifo.c |  2 +-
 libavutil/version.h    |  1 +
 5 files changed, 37 insertions(+), 7 deletions(-)

Anton Khirnov:
> There should be no good reason for the callers to access any of its
> contents.
> 
> Define a new type for the internal struct that currently matches
> AVFifoBuffer. This will allow adding new private fields without waiting
> for the major bump and will be useful in the following commits.
> 
> Unfortunately AVFifoBuffer fields cannot be marked as deprecated because
> it would trigger a warning wherever fifo.h is #included, due to
> inlined av_fifo_peek2().
> ---
>  doc/APIchanges         |  4 ++++
>  libavutil/fifo.c       | 23 +++++++++++++++++++----
>  libavutil/fifo.h       | 14 ++++++++++++--
>  libavutil/tests/fifo.c |  2 +-
>  libavutil/version.h    |  1 +
>  5 files changed, 37 insertions(+), 7 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 8df0364e4c..21fa02ae9d 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,10 @@ libavutil:     2021-04-27
>  
>  API changes, most recent first:
>  
> +2022-01-xx - xxxxxxxxxx - lavu fifo.h
> +  Access to all AVFifoBuffer members is deprecated. The struct will
> +  become an incomplete type in a future major libavutil version.
> +
>  2022-01-04 - 78dc21b123e - lavu 57.16.100 - frame.h
>    Add AV_FRAME_DATA_DOVI_METADATA.
>  
> diff --git a/libavutil/fifo.c b/libavutil/fifo.c
> index f2f046b1f3..aaade01333 100644
> --- a/libavutil/fifo.c
> +++ b/libavutil/fifo.c
> @@ -28,9 +28,24 @@
>  
>  #define FIFO_SIZE_MAX FFMIN3((uint64_t)INT_MAX, (uint64_t)UINT32_MAX, (uint64_t)SIZE_MAX)
>  
> +#if FF_API_FIFO_PUBLIC
> +# define CTX_STRUCT_NAME FifoBuffer
> +#else
> +# define CTX_STRUCT_NAME AVFifoBuffer

This is invalid in pre-C11 (and will lead to compilation failures on old
GCC versions): Pre-C11, typedefs were subject to the ODR, yet you
already typedef AVFifoBuffer in fifo.h.

> +#endif
> +
> +typedef struct CTX_STRUCT_NAME {
> +    // These fields must match then contents of AVFifoBuffer in fifo.h

the contents

> +    // until FF_API_FIFO_PUBLIC is removed

The actual spec-compliant way for this is to add an AVFifoBuffer at the
start of this struct; it will also avoid the casts from FifoBuffer to
AVFifoBuffer. This will make the accesses to it a bit more cumbersome
and will mean more changes when FF_API_FIFO_PUBLIC is removed. (This
would not be an issue if we required support for anonymous structs
(mandatory in C11, supported by GCC and others long before that).)

> +    uint8_t *buffer;
> +    uint8_t *rptr, *wptr, *end;
> +    uint32_t rndx, wndx;
> +    /////////////////////////////////////////
> +} FifoBuffer;
> +
>  AVFifoBuffer *av_fifo_alloc_array(size_t nmemb, size_t size)
>  {
> -    AVFifoBuffer *f;
> +    FifoBuffer *f;
>      void *buffer;
>  
>      if (nmemb > FIFO_SIZE_MAX / size)
> @@ -39,15 +54,15 @@ AVFifoBuffer *av_fifo_alloc_array(size_t nmemb, size_t size)
>      buffer = av_realloc_array(NULL, nmemb, size);
>      if (!buffer)
>          return NULL;
> -    f = av_mallocz(sizeof(AVFifoBuffer));
> +    f = av_mallocz(sizeof(*f));
>      if (!f) {
>          av_free(buffer);
>          return NULL;
>      }
>      f->buffer = buffer;
>      f->end    = f->buffer + nmemb * size;
> -    av_fifo_reset(f);
> -    return f;
> +    av_fifo_reset((AVFifoBuffer*)f);
> +    return (AVFifoBuffer*)f;
>  }
>  
>  AVFifoBuffer *av_fifo_alloc(unsigned int size)
> diff --git a/libavutil/fifo.h b/libavutil/fifo.h
> index f4fd291e59..ca4e7fe060 100644
> --- a/libavutil/fifo.h
> +++ b/libavutil/fifo.h
> @@ -28,11 +28,21 @@
>  #include "avutil.h"
>  #include "attributes.h"
>  
> -typedef struct AVFifoBuffer {
> +#if FF_API_FIFO_PUBLIC
> +/**
> + * The contents of the struct are private and should not be accessed by the
> + * callers in any way.
> + */
> +#endif
> +typedef struct AVFifoBuffer
> +#if FF_API_FIFO_PUBLIC
> +{
>      uint8_t *buffer;
>      uint8_t *rptr, *wptr, *end;
>      uint32_t rndx, wndx;
> -} AVFifoBuffer;
> +}
> +#endif
> +AVFifoBuffer;
>  
>  /**
>   * Initialize an AVFifoBuffer.
> diff --git a/libavutil/tests/fifo.c b/libavutil/tests/fifo.c
> index a17d913233..e5aa88d252 100644
> --- a/libavutil/tests/fifo.c
> +++ b/libavutil/tests/fifo.c
> @@ -18,7 +18,7 @@
>  
>  #include <stdio.h>
>  #include <stdlib.h>
> -#include "libavutil/fifo.h"
> +#include "libavutil/fifo.c"
>  
>  int main(void)
>  {
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 953aac9d94..7c031f547e 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -110,6 +110,7 @@
>  #define FF_API_COLORSPACE_NAME          (LIBAVUTIL_VERSION_MAJOR < 58)
>  #define FF_API_AV_MALLOCZ_ARRAY         (LIBAVUTIL_VERSION_MAJOR < 58)
>  #define FF_API_FIFO_PEEK2               (LIBAVUTIL_VERSION_MAJOR < 58)
> +#define FF_API_FIFO_PUBLIC              (LIBAVUTIL_VERSION_MAJOR < 58)
>  
>  /**
>   * @}
>