diff mbox series

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

Message ID 20240503163623.376990-1-derek.buitenhuis@gmail.com
State Accepted
Commit f8a613d6a86f1f2875cbebc8f1f60cfe39256fd1
Headers show
Series [FFmpeg-devel] fftools/ffprobe: Avoid overflow when calculating DAR | expand

Commit Message

Derek Buitenhuis May 3, 2024, 4:36 p.m. UTC
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(-)

Comments

Michael Niedermayer May 3, 2024, 11:13 p.m. UTC | #1
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

[...]
Stefano Sabatini May 4, 2024, 3:38 p.m. UTC | #2
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.
Anton Khirnov May 23, 2024, 7:52 a.m. UTC | #3
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?
diff mbox series

Patch

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 {