diff mbox series

[FFmpeg-devel,1/3] avutil/buffer: change public function and struct size parameter types to size_t

Message ID 20200531163839.561-1-jamrial@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,1/3] avutil/buffer: change public function and struct size parameter types to size_t | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

James Almer May 31, 2020, 4:38 p.m. UTC 
Signed-off-by: James Almer <jamrial@gmail.com>
---
Aside from being more correct, it's also needed now that the video_enc_params
API, which may allocate arrays > INT_MAX, is used as frame side data.

 doc/APIchanges              |  4 ++++
 libavutil/buffer.c          | 25 +++++++++++++++++++++++++
 libavutil/buffer.h          | 31 +++++++++++++++++++++++++++++++
 libavutil/buffer_internal.h | 13 +++++++++++++
 libavutil/version.h         |  3 +++
 5 files changed, 76 insertions(+)

Comments

James Almer May 31, 2020, 8:13 p.m. UTC | #1 
On 5/31/2020 1:38 PM, James Almer wrote:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> Aside from being more correct, it's also needed now that the video_enc_params
> API, which may allocate arrays > INT_MAX, is used as frame side data.
> 
>  doc/APIchanges              |  4 ++++
>  libavutil/buffer.c          | 25 +++++++++++++++++++++++++
>  libavutil/buffer.h          | 31 +++++++++++++++++++++++++++++++
>  libavutil/buffer_internal.h | 13 +++++++++++++
>  libavutil/version.h         |  3 +++
>  5 files changed, 76 insertions(+)

[...]

> +#if FF_API_BUFFER_SIZE_T
>  AVBufferPool *av_buffer_pool_init2(int size, void *opaque,
>                                     AVBufferRef* (*alloc)(void *opaque, int size),
> +#else
> +AVBufferPool *av_buffer_pool_init2(size_t size, void *opaque,
> +                                   AVBufferRef* (*alloc)(void *opaque, size_t size),
> +#endif
>                                     void (*pool_free)(void *opaque))
>  {
>      AVBufferPool *pool = av_mallocz(sizeof(*pool));
> @@ -236,7 +257,11 @@ AVBufferPool *av_buffer_pool_init2(int size, void *opaque,
>      return pool;
>  }
>  
> +#if FF_API_BUFFER_SIZE_T
>  AVBufferPool *av_buffer_pool_init(int size, AVBufferRef* (*alloc)(int size))
> +#else
> +AVBufferPool *av_buffer_pool_init(size_t size, AVBufferRef* (*alloc)(size_t size))

These two pool alloc functions are the ones that make me think we may
need to let two years pass before making the switch. Library users
implementing their own callbacks will need to adapt them.

I'll for that matter resend the AVPacket patch later, changing only the
side data parts.
Hendrik Leppkes May 31, 2020, 10:30 p.m. UTC | #2 
On Sun, May 31, 2020 at 10:13 PM James Almer <jamrial@gmail.com> wrote:
>
> On 5/31/2020 1:38 PM, James Almer wrote:
> > Signed-off-by: James Almer <jamrial@gmail.com>
> > ---
> > Aside from being more correct, it's also needed now that the video_enc_params
> > API, which may allocate arrays > INT_MAX, is used as frame side data.
> >
> >  doc/APIchanges              |  4 ++++
> >  libavutil/buffer.c          | 25 +++++++++++++++++++++++++
> >  libavutil/buffer.h          | 31 +++++++++++++++++++++++++++++++
> >  libavutil/buffer_internal.h | 13 +++++++++++++
> >  libavutil/version.h         |  3 +++
> >  5 files changed, 76 insertions(+)
>
> [...]
>
> > +#if FF_API_BUFFER_SIZE_T
> >  AVBufferPool *av_buffer_pool_init2(int size, void *opaque,
> >                                     AVBufferRef* (*alloc)(void *opaque, int size),
> > +#else
> > +AVBufferPool *av_buffer_pool_init2(size_t size, void *opaque,
> > +                                   AVBufferRef* (*alloc)(void *opaque, size_t size),
> > +#endif
> >                                     void (*pool_free)(void *opaque))
> >  {
> >      AVBufferPool *pool = av_mallocz(sizeof(*pool));
> > @@ -236,7 +257,11 @@ AVBufferPool *av_buffer_pool_init2(int size, void *opaque,
> >      return pool;
> >  }
> >
> > +#if FF_API_BUFFER_SIZE_T
> >  AVBufferPool *av_buffer_pool_init(int size, AVBufferRef* (*alloc)(int size))
> > +#else
> > +AVBufferPool *av_buffer_pool_init(size_t size, AVBufferRef* (*alloc)(size_t size))
>
> These two pool alloc functions are the ones that make me think we may
> need to let two years pass before making the switch. Library users
> implementing their own callbacks will need to adapt them.
>
> I'll for that matter resend the AVPacket patch later, changing only the
> side data parts.

A deprecation period only makes sense if you plan to offer the
alternate API under a new (permanent) name. The point of such a period
is to deprecate the old name and give people time to swap to the new
one.
Announcing a future change without any pro-active migration does not
help, and thus the two year period would do nothing but delay the
change.

- Hendrik
diff mbox series

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index fb5534b5f5..9de23744c0 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,10 @@  libavutil:     2017-10-21
 
 API changes, most recent first:
 
+2020-06-xx - xxxxxxxxxx
+  Change AVBufferRef related function and struct size parameter and fields
+  type to size_t at next major bump.
+
 2020-xx-xx - xxxxxxxxxx - lavc 58.88.100 - avcodec.h codec.h
   Move AVCodec-related public API to new header codec.h.
 
diff --git a/libavutil/buffer.c b/libavutil/buffer.c
index 6d9cb7428e..0a29430381 100644
--- a/libavutil/buffer.c
+++ b/libavutil/buffer.c
@@ -26,7 +26,11 @@ 
 #include "mem.h"
 #include "thread.h"
 
+#if FF_API_BUFFER_SIZE_T
 AVBufferRef *av_buffer_create(uint8_t *data, int size,
+#else
+AVBufferRef *av_buffer_create(uint8_t *data, size_t size,
+#endif
                               void (*free)(void *opaque, uint8_t *data),
                               void *opaque, int flags)
 {
@@ -65,7 +69,11 @@  void av_buffer_default_free(void *opaque, uint8_t *data)
     av_free(data);
 }
 
+#if FF_API_BUFFER_SIZE_T
 AVBufferRef *av_buffer_alloc(int size)
+#else
+AVBufferRef *av_buffer_alloc(size_t size)
+#endif
 {
     AVBufferRef *ret = NULL;
     uint8_t    *data = NULL;
@@ -81,7 +89,11 @@  AVBufferRef *av_buffer_alloc(int size)
     return ret;
 }
 
+#if FF_API_BUFFER_SIZE_T
 AVBufferRef *av_buffer_allocz(int size)
+#else
+AVBufferRef *av_buffer_allocz(size_t size)
+#endif
 {
     AVBufferRef *ret = av_buffer_alloc(size);
     if (!ret)
@@ -167,7 +179,11 @@  int av_buffer_make_writable(AVBufferRef **pbuf)
     return 0;
 }
 
+#if FF_API_BUFFER_SIZE_T
 int av_buffer_realloc(AVBufferRef **pbuf, int size)
+#else
+int av_buffer_realloc(AVBufferRef **pbuf, size_t size)
+#endif
 {
     AVBufferRef *buf = *pbuf;
     uint8_t *tmp;
@@ -216,8 +232,13 @@  int av_buffer_realloc(AVBufferRef **pbuf, int size)
     return 0;
 }
 
+#if FF_API_BUFFER_SIZE_T
 AVBufferPool *av_buffer_pool_init2(int size, void *opaque,
                                    AVBufferRef* (*alloc)(void *opaque, int size),
+#else
+AVBufferPool *av_buffer_pool_init2(size_t size, void *opaque,
+                                   AVBufferRef* (*alloc)(void *opaque, size_t size),
+#endif
                                    void (*pool_free)(void *opaque))
 {
     AVBufferPool *pool = av_mallocz(sizeof(*pool));
@@ -236,7 +257,11 @@  AVBufferPool *av_buffer_pool_init2(int size, void *opaque,
     return pool;
 }
 
+#if FF_API_BUFFER_SIZE_T
 AVBufferPool *av_buffer_pool_init(int size, AVBufferRef* (*alloc)(int size))
+#else
+AVBufferPool *av_buffer_pool_init(size_t size, AVBufferRef* (*alloc)(size_t size))
+#endif
 {
     AVBufferPool *pool = av_mallocz(sizeof(*pool));
     if (!pool)
diff --git a/libavutil/buffer.h b/libavutil/buffer.h
index e0f94314f4..547c376439 100644
--- a/libavutil/buffer.h
+++ b/libavutil/buffer.h
@@ -27,6 +27,8 @@ 
 
 #include <stdint.h>
 
+#include "version.h"
+
 /**
  * @defgroup lavu_buffer AVBuffer
  * @ingroup lavu_data
@@ -90,7 +92,11 @@  typedef struct AVBufferRef {
     /**
      * Size of data in bytes.
      */
+#if FF_API_BUFFER_SIZE_T
     int      size;
+#else
+    size_t   size;
+#endif
 } AVBufferRef;
 
 /**
@@ -98,13 +104,21 @@  typedef struct AVBufferRef {
  *
  * @return an AVBufferRef of given size or NULL when out of memory
  */
+#if FF_API_BUFFER_SIZE_T
 AVBufferRef *av_buffer_alloc(int size);
+#else
+AVBufferRef *av_buffer_alloc(size_t size);
+#endif
 
 /**
  * Same as av_buffer_alloc(), except the returned buffer will be initialized
  * to zero.
  */
+#if FF_API_BUFFER_SIZE_T
 AVBufferRef *av_buffer_allocz(int size);
+#else
+AVBufferRef *av_buffer_alloc(size_t size);
+#endif
 
 /**
  * Always treat the buffer as read-only, even when it has only one
@@ -127,7 +141,11 @@  AVBufferRef *av_buffer_allocz(int size);
  *
  * @return an AVBufferRef referring to data on success, NULL on failure.
  */
+#if FF_API_BUFFER_SIZE_T
 AVBufferRef *av_buffer_create(uint8_t *data, int size,
+#else
+AVBufferRef *av_buffer_create(uint8_t *data, size_t size,
+#endif
                               void (*free)(void *opaque, uint8_t *data),
                               void *opaque, int flags);
 
@@ -195,7 +213,11 @@  int av_buffer_make_writable(AVBufferRef **buf);
  * reference to it (i.e. the one passed to this function). In all other cases
  * a new buffer is allocated and the data is copied.
  */
+#if FF_API_BUFFER_SIZE_T
 int av_buffer_realloc(AVBufferRef **buf, int size);
+#else
+int av_buffer_realloc(AVBufferRef **buf, size_t size);
+#endif
 
 /**
  * @}
@@ -246,7 +268,11 @@  typedef struct AVBufferPool AVBufferPool;
  * (av_buffer_alloc()).
  * @return newly created buffer pool on success, NULL on error.
  */
+#if FF_API_BUFFER_SIZE_T
 AVBufferPool *av_buffer_pool_init(int size, AVBufferRef* (*alloc)(int size));
+#else
+AVBufferPool *av_buffer_pool_init(size_t size, AVBufferRef* (*alloc)(size_t size));
+#endif
 
 /**
  * Allocate and initialize a buffer pool with a more complex allocator.
@@ -262,8 +288,13 @@  AVBufferPool *av_buffer_pool_init(int size, AVBufferRef* (*alloc)(int size));
  *                  data.
  * @return newly created buffer pool on success, NULL on error.
  */
+#if FF_API_BUFFER_SIZE_T
 AVBufferPool *av_buffer_pool_init2(int size, void *opaque,
                                    AVBufferRef* (*alloc)(void *opaque, int size),
+#else
+AVBufferPool *av_buffer_pool_init2(size_t size, void *opaque,
+                                   AVBufferRef* (*alloc)(void *opaque, size_t size),
+#endif
                                    void (*pool_free)(void *opaque));
 
 /**
diff --git a/libavutil/buffer_internal.h b/libavutil/buffer_internal.h
index 54b67047e5..c2f9342422 100644
--- a/libavutil/buffer_internal.h
+++ b/libavutil/buffer_internal.h
@@ -36,7 +36,11 @@ 
 
 struct AVBuffer {
     uint8_t *data; /**< data described by this buffer */
+#if FF_API_BUFFER_SIZE_T
     int      size; /**< size of data in bytes */
+#else
+    size_t   size; /**< size of data in bytes */
+#endif
 
     /**
      *  number of existing AVBufferRef instances referring to this buffer
@@ -88,10 +92,19 @@  struct AVBufferPool {
      */
     atomic_uint refcount;
 
+#if FF_API_BUFFER_SIZE_T
     int size;
+#else
+    size_t size;
+#endif
     void *opaque;
+#if FF_API_BUFFER_SIZE_T
     AVBufferRef* (*alloc)(int size);
     AVBufferRef* (*alloc2)(void *opaque, int size);
+#else
+    AVBufferRef* (*alloc)(size_t size);
+    AVBufferRef* (*alloc2)(void *opaque, size_t size);
+#endif
     void         (*pool_free)(void *opaque);
 };
 
diff --git a/libavutil/version.h b/libavutil/version.h
index 7acecf5a97..431eb1d571 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -129,6 +129,9 @@ 
 #ifndef FF_API_PSEUDOPAL
 #define FF_API_PSEUDOPAL                (LIBAVUTIL_VERSION_MAJOR < 57)
 #endif
+#ifndef FF_API_BUFFER_SIZE_T
+#define FF_API_BUFFER_SIZE_T            (LIBAVUTIL_VERSION_MAJOR < 57)
+#endif
 
 
 /**