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

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

[James Almer]

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
     /**
      * @}
      */
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)
 
 /**
  * @}
-- 
2.40.0

_______________________________________________
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".

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

[Anton Khirnov]

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

-- 
Anton Khirnov
_______________________________________________
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".

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

[Marton Balint]

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
_______________________________________________
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".