[FFmpeg-devel] [PATCH] fftools/ffprobe: Avoid overflow when calculating DAR

[FFmpeg-devel] [PATCH] fftools/ffprobe: Avoid overflow when calculating DAR

[Derek Buitenhuis]

Both the codecpar's width and height, and the SAR num and den are
ints, which can overflow. Cast to int64_t, which is what av_reduce
takes.

Without this, occasionally, display_aspect_ratio can be negative in
ffprobe's -show_stream output.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 fftools/ffprobe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 0d4cd0b048..5b40dad527 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -3324,8 +3324,8 @@ static int show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_id
         if (sar.num) {
             print_q("sample_aspect_ratio", sar, ':');
             av_reduce(&dar.num, &dar.den,
-                      par->width  * sar.num,
-                      par->height * sar.den,
+                      (int64_t) par->width  * sar.num,
+                      (int64_t) par->height * sar.den,
                       1024*1024);
             print_q("display_aspect_ratio", dar, ':');
         } else {
-- 
2.43.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] fftools/ffprobe: Avoid overflow when calculating DAR

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 727 bytes --]

On Fri, May 03, 2024 at 05:36:23PM +0100, Derek Buitenhuis wrote:
> Both the codecpar's width and height, and the SAR num and den are
> ints, which can overflow. Cast to int64_t, which is what av_reduce
> takes.
> 
> Without this, occasionally, display_aspect_ratio can be negative in
> ffprobe's -show_stream output.
> 
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
>  fftools/ffprobe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

LGTM

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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] fftools/ffprobe: Avoid overflow when calculating DAR

[Stefano Sabatini]

On date Friday 2024-05-03 17:36:23 +0100, Derek Buitenhuis wrote:
> Both the codecpar's width and height, and the SAR num and den are
> ints, which can overflow. Cast to int64_t, which is what av_reduce
> takes.
> 
> Without this, occasionally, display_aspect_ratio can be negative in
> ffprobe's -show_stream output.
> 
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
>  fftools/ffprobe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index 0d4cd0b048..5b40dad527 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -3324,8 +3324,8 @@ static int show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_id
>          if (sar.num) {
>              print_q("sample_aspect_ratio", sar, ':');
>              av_reduce(&dar.num, &dar.den,
> -                      par->width  * sar.num,
> -                      par->height * sar.den,
> +                      (int64_t) par->width  * sar.num,
> +                      (int64_t) par->height * sar.den,
>                        1024*1024);
>              print_q("display_aspect_ratio", dar, ':');
>          } else {

LGTM, 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".

Re: [FFmpeg-devel] [PATCH] fftools/ffprobe: Avoid overflow when calculating DAR

[Anton Khirnov]

Quoting Derek Buitenhuis (2024-05-03 18:36:23)
> Both the codecpar's width and height, and the SAR num and den are
> ints, which can overflow. Cast to int64_t, which is what av_reduce
> takes.
> 
> Without this, occasionally, display_aspect_ratio can be negative in
> ffprobe's -show_stream output.
> 
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
>  fftools/ffprobe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index 0d4cd0b048..5b40dad527 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -3324,8 +3324,8 @@ static int show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_id
>          if (sar.num) {
>              print_q("sample_aspect_ratio", sar, ':');
>              av_reduce(&dar.num, &dar.den,
> -                      par->width  * sar.num,
> -                      par->height * sar.den,
> +                      (int64_t) par->width  * sar.num,
> +                      (int64_t) par->height * sar.den,

Aren't we supposed to avoid assumptions that int is always strictly
smaller than 64bit?

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