| Message ID | 20210306194243.14931-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/9,v3] avutil/buffer: change public function and struct size parameter types to size_t | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On Sat, Mar 06, 2021 at 04:42:35PM -0300, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > Changes since v2 is the addition of the buffer_size_t typedef to reduce the > amount of ifdeffery required to adapt our code. > > doc/APIchanges | 4 ++++ > libavutil/buffer.c | 14 +++++++------- > libavutil/buffer.h | 32 ++++++++++++++++++++++++++++++++ > libavutil/buffer_internal.h | 8 ++++---- > libavutil/internal.h | 7 +++++++ > libavutil/version.h | 5 ++++- > 6 files changed, 58 insertions(+), 12 deletions(-) The patchset prior to the bump LGTM the patchset after the bump (thus enabled size_t) is probably ok but i did not review this for downstream effects which could introduce bugs when theres any code that expected the variables to be int code prior bump tested, code post bump not tested Thanks [...]
On 3/10/2021 4:38 PM, Michael Niedermayer wrote: > On Sat, Mar 06, 2021 at 04:42:35PM -0300, James Almer wrote: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> Changes since v2 is the addition of the buffer_size_t typedef to reduce the >> amount of ifdeffery required to adapt our code. >> >> doc/APIchanges | 4 ++++ >> libavutil/buffer.c | 14 +++++++------- >> libavutil/buffer.h | 32 ++++++++++++++++++++++++++++++++ >> libavutil/buffer_internal.h | 8 ++++---- >> libavutil/internal.h | 7 +++++++ >> libavutil/version.h | 5 ++++- >> 6 files changed, 58 insertions(+), 12 deletions(-) > > The patchset prior to the bump LGTM > the patchset after the bump (thus enabled size_t) is probably ok but > i did not review this for downstream effects which could introduce bugs > when theres any code that expected the variables to be int That's what 2+ years of advanced warning are for. Same as we did with the Crypto modules that will be switching to size_t in the upcoming bump. > > code prior bump tested, code post bump not tested > > Thanks > > [...] > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
James Almer: > On 3/10/2021 4:38 PM, Michael Niedermayer wrote: >> On Sat, Mar 06, 2021 at 04:42:35PM -0300, James Almer wrote: >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> Changes since v2 is the addition of the buffer_size_t typedef to >>> reduce the >>> amount of ifdeffery required to adapt our code. >>> >>> doc/APIchanges | 4 ++++ >>> libavutil/buffer.c | 14 +++++++------- >>> libavutil/buffer.h | 32 ++++++++++++++++++++++++++++++++ >>> libavutil/buffer_internal.h | 8 ++++---- >>> libavutil/internal.h | 7 +++++++ >>> libavutil/version.h | 5 ++++- >>> 6 files changed, 58 insertions(+), 12 deletions(-) >> >> The patchset prior to the bump LGTM >> the patchset after the bump (thus enabled size_t) is probably ok but >> i did not review this for downstream effects which could introduce bugs >> when theres any code that expected the variables to be int > > That's what 2+ years of advanced warning are for. Same as we did with > the Crypto modules that will be switching to size_t in the upcoming bump. > What 2+ years of advanced warning are you talking about? This patchset as-is wants to switch the type at the next major bump, i.e. fairly soon. (And fyi: Our crypto functions are not compatible with size_t right now; see https://github.com/mkver/FFmpeg/commit/1d830b0495399bb59a296d7bb3e2b2ab88a32fc1.) - Andreas >> >> code prior bump tested, code post bump not tested >> >> Thanks >> >> [...] >> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 3/10/2021 6:28 PM, Andreas Rheinhardt wrote: > James Almer: >> On 3/10/2021 4:38 PM, Michael Niedermayer wrote: >>> On Sat, Mar 06, 2021 at 04:42:35PM -0300, James Almer wrote: >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> Changes since v2 is the addition of the buffer_size_t typedef to >>>> reduce the >>>> amount of ifdeffery required to adapt our code. >>>> >>>> doc/APIchanges | 4 ++++ >>>> libavutil/buffer.c | 14 +++++++------- >>>> libavutil/buffer.h | 32 ++++++++++++++++++++++++++++++++ >>>> libavutil/buffer_internal.h | 8 ++++---- >>>> libavutil/internal.h | 7 +++++++ >>>> libavutil/version.h | 5 ++++- >>>> 6 files changed, 58 insertions(+), 12 deletions(-) >>> >>> The patchset prior to the bump LGTM >>> the patchset after the bump (thus enabled size_t) is probably ok but >>> i did not review this for downstream effects which could introduce bugs >>> when theres any code that expected the variables to be int >> >> That's what 2+ years of advanced warning are for. Same as we did with >> the Crypto modules that will be switching to size_t in the upcoming bump. >> > > What 2+ years of advanced warning are you talking about? My bad, was thinking about the AVPacket set. This patchset > as-is wants to switch the type at the next major bump, i.e. fairly soon. > (And fyi: Our crypto functions are not compatible with size_t right now; > see > https://github.com/mkver/FFmpeg/commit/1d830b0495399bb59a296d7bb3e2b2ab88a32fc1.) > > - Andreas > >>> >>> code prior bump tested, code post bump not tested >>> >>> Thanks >>> >>> [...] >>> >>> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >>> To unsubscribe, visit link above, or email >>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >>> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
diff --git a/doc/APIchanges b/doc/APIchanges index 4027d599e7..7abc320db2 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,10 @@ libavutil: 2017-10-21 API changes, most recent first: +2021-03-xx - xxxxxxxxxx - lavu 56.68.100 - buffer.h + Change AVBufferRef related function and struct size parameter and fields + type to size_t at next major bump. + 2021-03-04 - xxxxxxxxxx - lavc 58.128.101 - avcodec.h Enable err_recognition to be set for encoders. diff --git a/libavutil/buffer.c b/libavutil/buffer.c index 3204f11b68..858633e8c7 100644 --- a/libavutil/buffer.c +++ b/libavutil/buffer.c @@ -26,7 +26,7 @@ #include "mem.h" #include "thread.h" -AVBufferRef *av_buffer_create(uint8_t *data, int size, +AVBufferRef *av_buffer_create(uint8_t *data, buffer_size_t size, void (*free)(void *opaque, uint8_t *data), void *opaque, int flags) { @@ -64,7 +64,7 @@ void av_buffer_default_free(void *opaque, uint8_t *data) av_free(data); } -AVBufferRef *av_buffer_alloc(int size) +AVBufferRef *av_buffer_alloc(buffer_size_t size) { AVBufferRef *ret = NULL; uint8_t *data = NULL; @@ -80,7 +80,7 @@ AVBufferRef *av_buffer_alloc(int size) return ret; } -AVBufferRef *av_buffer_allocz(int size) +AVBufferRef *av_buffer_allocz(buffer_size_t size) { AVBufferRef *ret = av_buffer_alloc(size); if (!ret) @@ -166,7 +166,7 @@ int av_buffer_make_writable(AVBufferRef **pbuf) return 0; } -int av_buffer_realloc(AVBufferRef **pbuf, int size) +int av_buffer_realloc(AVBufferRef **pbuf, buffer_size_t size) { AVBufferRef *buf = *pbuf; uint8_t *tmp; @@ -242,8 +242,8 @@ int av_buffer_replace(AVBufferRef **pdst, AVBufferRef *src) return 0; } -AVBufferPool *av_buffer_pool_init2(int size, void *opaque, - AVBufferRef* (*alloc)(void *opaque, int size), +AVBufferPool *av_buffer_pool_init2(buffer_size_t size, void *opaque, + AVBufferRef* (*alloc)(void *opaque, buffer_size_t size), void (*pool_free)(void *opaque)) { AVBufferPool *pool = av_mallocz(sizeof(*pool)); @@ -263,7 +263,7 @@ AVBufferPool *av_buffer_pool_init2(int size, void *opaque, return pool; } -AVBufferPool *av_buffer_pool_init(int size, AVBufferRef* (*alloc)(int size)) +AVBufferPool *av_buffer_pool_init(buffer_size_t size, AVBufferRef* (*alloc)(buffer_size_t size)) { AVBufferPool *pool = av_mallocz(sizeof(*pool)); if (!pool) diff --git a/libavutil/buffer.h b/libavutil/buffer.h index fd4e381efa..241a80ed67 100644 --- a/libavutil/buffer.h +++ b/libavutil/buffer.h @@ -25,8 +25,11 @@ #ifndef AVUTIL_BUFFER_H #define AVUTIL_BUFFER_H +#include <stddef.h> #include <stdint.h> +#include "version.h" + /** * @defgroup lavu_buffer AVBuffer * @ingroup lavu_data @@ -90,7 +93,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 +105,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_allocz(size_t size); +#endif /** * Always treat the buffer as read-only, even when it has only one @@ -127,7 +142,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 +214,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 /** * Ensure dst refers to the same data as src. @@ -262,7 +285,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. @@ -279,8 +306,13 @@ AVBufferPool *av_buffer_pool_init(int size, AVBufferRef* (*alloc)(int size)); * data. May be NULL. * @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 70d2615a06..0b549e3a53 100644 --- a/libavutil/buffer_internal.h +++ b/libavutil/buffer_internal.h @@ -32,7 +32,7 @@ struct AVBuffer { uint8_t *data; /**< data described by this buffer */ - int size; /**< size of data in bytes */ + buffer_size_t size; /**< size of data in bytes */ /** * number of existing AVBufferRef instances referring to this buffer @@ -89,10 +89,10 @@ struct AVBufferPool { */ atomic_uint refcount; - int size; + buffer_size_t size; void *opaque; - AVBufferRef* (*alloc)(int size); - AVBufferRef* (*alloc2)(void *opaque, int size); + AVBufferRef* (*alloc)(buffer_size_t size); + AVBufferRef* (*alloc2)(void *opaque, buffer_size_t size); void (*pool_free)(void *opaque); }; diff --git a/libavutil/internal.h b/libavutil/internal.h index 93ea57c324..572daefec3 100644 --- a/libavutil/internal.h +++ b/libavutil/internal.h @@ -299,4 +299,11 @@ int avpriv_dict_set_timestamp(AVDictionary **dict, const char *key, int64_t time #define FF_PSEUDOPAL 0 #endif +// Temporary typedef to simplify porting all AVBufferRef users to size_t +#if FF_API_BUFFER_SIZE_T +typedef int buffer_size_t; +#else +typedef size_t buffer_size_t; +#endif + #endif /* AVUTIL_INTERNAL_H */ diff --git a/libavutil/version.h b/libavutil/version.h index 356c54d633..9a290d57e7 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,7 +79,7 @@ */ #define LIBAVUTIL_VERSION_MAJOR 56 -#define LIBAVUTIL_VERSION_MINOR 67 +#define LIBAVUTIL_VERSION_MINOR 68 #define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ @@ -132,6 +132,9 @@ #ifndef FF_API_CHILD_CLASS_NEXT #define FF_API_CHILD_CLASS_NEXT (LIBAVUTIL_VERSION_MAJOR < 57) #endif +#ifndef FF_API_BUFFER_SIZE_T +#define FF_API_BUFFER_SIZE_T (LIBAVUTIL_VERSION_MAJOR < 57) +#endif #ifndef FF_API_D2STR #define FF_API_D2STR (LIBAVUTIL_VERSION_MAJOR < 58) #endif
Signed-off-by: James Almer <jamrial@gmail.com> --- Changes since v2 is the addition of the buffer_size_t typedef to reduce the amount of ifdeffery required to adapt our code. doc/APIchanges | 4 ++++ libavutil/buffer.c | 14 +++++++------- libavutil/buffer.h | 32 ++++++++++++++++++++++++++++++++ libavutil/buffer_internal.h | 8 ++++---- libavutil/internal.h | 7 +++++++ libavutil/version.h | 5 ++++- 6 files changed, 58 insertions(+), 12 deletions(-)