| 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 |
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 [...]
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.
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 --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 {
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(-)