diff mbox series

[FFmpeg-devel] avutil/frame: change the type of the crop fields to unsigned in the next major bump

Message ID 20230417153408.1225-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel] avutil/frame: change the type of the crop fields to unsigned in the next major bump | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

James Almer April 17, 2023, 3:34 p.m. UTC
size_t is unnecessarily big on 64bit arches when width and height are ints.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavutil/frame.h   | 7 +++++++
 libavutil/version.h | 1 +
 2 files changed, 8 insertions(+)

Comments

Anton Khirnov April 17, 2023, 5:07 p.m. UTC | #1
Quoting James Almer (2023-04-17 17:34:08)
> size_t is unnecessarily big on 64bit arches when width and height are ints.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavutil/frame.h   | 7 +++++++
>  libavutil/version.h | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index f85d630c5c..e10a1c65e3 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -722,10 +722,17 @@ typedef struct AVFrame {
>       * the frame intended for presentation.
>       * @{
>       */
> +#if FF_API_CROP_SIZE_T
>      size_t crop_top;
>      size_t crop_bottom;
>      size_t crop_left;
>      size_t crop_right;
> +#else
> +    unsigned int crop_top;
> +    unsigned int crop_bottom;
> +    unsigned int crop_left;
> +    unsigned int crop_right;
> +#endif

Ok, but should be mentioned in APIchanges
Marton Balint April 17, 2023, 5:20 p.m. UTC | #2
On Mon, 17 Apr 2023, Anton Khirnov wrote:

> Quoting James Almer (2023-04-17 17:34:08)
>> size_t is unnecessarily big on 64bit arches when width and height are ints.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  libavutil/frame.h   | 7 +++++++
>>  libavutil/version.h | 1 +
>>  2 files changed, 8 insertions(+)
>>
>> diff --git a/libavutil/frame.h b/libavutil/frame.h
>> index f85d630c5c..e10a1c65e3 100644
>> --- a/libavutil/frame.h
>> +++ b/libavutil/frame.h
>> @@ -722,10 +722,17 @@ typedef struct AVFrame {
>>       * the frame intended for presentation.
>>       * @{
>>       */
>> +#if FF_API_CROP_SIZE_T
>>      size_t crop_top;
>>      size_t crop_bottom;
>>      size_t crop_left;
>>      size_t crop_right;
>> +#else
>> +    unsigned int crop_top;
>> +    unsigned int crop_bottom;
>> +    unsigned int crop_left;
>> +    unsigned int crop_right;
>> +#endif
>
> Ok, but should be mentioned in APIchanges

I don't oppose this, but isn't this the same kind of change I wanted to do 
for frame_number which was rejected because of e.g. printf format string 
concerns? So based on that logic the proper way is to introduce new fields 
with different types and deprecate the old ones. If we want to be 
consistent with our API rules...

Regards,
Marton
diff mbox series

Patch

diff --git a/libavutil/frame.h b/libavutil/frame.h
index f85d630c5c..e10a1c65e3 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -722,10 +722,17 @@  typedef struct AVFrame {
      * the frame intended for presentation.
      * @{
      */
+#if FF_API_CROP_SIZE_T
     size_t crop_top;
     size_t crop_bottom;
     size_t crop_left;
     size_t crop_right;
+#else
+    unsigned int crop_top;
+    unsigned int crop_bottom;
+    unsigned int crop_left;
+    unsigned int crop_right;
+#endif
     /**
      * @}
      */
diff --git a/libavutil/version.h b/libavutil/version.h
index 40f92af055..a974890d1f 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -115,6 +115,7 @@ 
 #define FF_API_FRAME_PICTURE_NUMBER     (LIBAVUTIL_VERSION_MAJOR < 59)
 #define FF_API_HDR_VIVID_THREE_SPLINE   (LIBAVUTIL_VERSION_MAJOR < 59)
 #define FF_API_FRAME_PKT                (LIBAVUTIL_VERSION_MAJOR < 59)
+#define FF_API_CROP_SIZE_T              (LIBAVUTIL_VERSION_MAJOR < 59)
 
 /**
  * @}