diff mbox series

[FFmpeg-devel,1/2] avformat/mov: Use av_rescale when calculating bit rate

Message ID 20211020140354.1124965-2-derek.buitenhuis@gmail.com
State New
Headers show
Series mov: bit rate calculation fixes | expand

Checks

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

Commit Message

Derek Buitenhuis Oct. 20, 2021, 2:03 p.m. UTC 
It is less susceptible to overflows.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 libavformat/mov.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Michael Niedermayer Oct. 20, 2021, 6:30 p.m. UTC | #1 
On Wed, Oct 20, 2021 at 03:03:53PM +0100, Derek Buitenhuis wrote:
> It is less susceptible to overflows.
> 
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
>  libavformat/mov.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 57c67e3aac..431f430368 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -7989,12 +7989,14 @@ static int mov_read_header(AVFormatContext *s)
>              AVStream *st = s->streams[i];
>              MOVStreamContext *sc = st->priv_data;
>              if (st->duration > 0) {
> -                if (sc->data_size > INT64_MAX / sc->time_scale / 8) {
> +                /* Akin to sc->data_size * 8 * sc->time_scale / st->duration but accounting for overflows. */
> +                st->codecpar->bit_rate = av_rescale(sc->data_size, sc->time_scale * 8, st->duration);

Does something ensure time_scale * 8 fits in int ? if not this needs a cast to *int64_t

thx

[...]
Derek Buitenhuis Oct. 20, 2021, 6:43 p.m. UTC | #2 
On 10/20/2021 7:30 PM, Michael Niedermayer wrote:
> Does something ensure time_scale * 8 fits in int ? if not this needs a cast to *int64_t

You're right, this could break on systems with 32-bit int.

v2 sent.

- erek
diff mbox series

Patch

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 57c67e3aac..431f430368 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -7989,12 +7989,14 @@  static int mov_read_header(AVFormatContext *s)
             AVStream *st = s->streams[i];
             MOVStreamContext *sc = st->priv_data;
             if (st->duration > 0) {
-                if (sc->data_size > INT64_MAX / sc->time_scale / 8) {
+                /* Akin to sc->data_size * 8 * sc->time_scale / st->duration but accounting for overflows. */
+                st->codecpar->bit_rate = av_rescale(sc->data_size, sc->time_scale * 8, st->duration);
+                if (st->codecpar->bit_rate == INT64_MIN) {
                     av_log(s, AV_LOG_ERROR, "Overflow during bit rate calculation %"PRId64" * 8 * %d\n",
                            sc->data_size, sc->time_scale);
+                    st->codecpar->bit_rate = 0;
                     return AVERROR_INVALIDDATA;
                 }
-                st->codecpar->bit_rate = sc->data_size * 8 * sc->time_scale / st->duration;
             }
         }
     }
@@ -8004,13 +8006,14 @@  static int mov_read_header(AVFormatContext *s)
             AVStream *st = s->streams[i];
             MOVStreamContext *sc = st->priv_data;
             if (sc->duration_for_fps > 0) {
-                if (sc->data_size > INT64_MAX / sc->time_scale / 8) {
+                /* Akin to sc->data_size * 8 * sc->time_scale / sc->duration_for_fps but accounting for overflows. */
+                st->codecpar->bit_rate = av_rescale(sc->data_size, sc->time_scale * 8, sc->duration_for_fps);
+                if (st->codecpar->bit_rate == INT64_MIN) {
                     av_log(s, AV_LOG_ERROR, "Overflow during bit rate calculation %"PRId64" * 8 * %d\n",
                            sc->data_size, sc->time_scale);
+                    st->codecpar->bit_rate = 0;
                     return AVERROR_INVALIDDATA;
                 }
-                st->codecpar->bit_rate = sc->data_size * 8 * sc->time_scale /
-                    sc->duration_for_fps;
             }
         }
     }