diff mbox

[FFmpeg-devel,1/3] avutil/opt: Fix setting int64 to its maximum

Message ID 20161120115744.28351-1-michael@niedermayer.cc
State Accepted
Headers show

Commit Message

Michael Niedermayer Nov. 20, 2016, 11:57 a.m. UTC
Found-by: Andreas
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavutil/opt.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Carl Eugen Hoyos Nov. 20, 2016, 12:29 p.m. UTC | #1
2016-11-20 12:57 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>:

> -    case AV_OPT_TYPE_INT64:
> -        *(int64_t *)dst = llrint(num / den) * intnum;
> -        break;
> +    case AV_OPT_TYPE_INT64:{
> +        double d = num / den;
> +        if (intnum == 1 && d == (double)INT64_MAX) *(int64_t *)dst = INT64_MAX;
> +        else                                       *(int64_t *)dst = llrint(d) * intnum;
> +        break;}

Feel free to ignore but I believe the patch gets more readable with
carriage returns before the brackets.
(They exist similarly in adpcm.c)

Carl Eugen
Andreas Cadhalpun Nov. 20, 2016, 7:53 p.m. UTC | #2
On 20.11.2016 12:57, Michael Niedermayer wrote:
> Found-by: Andreas
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavutil/opt.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index cd16bd1..6669356 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -126,9 +126,11 @@ static int write_number(void *obj, const AVOption *o, void *dst, double num, int
>          break;
>      case AV_OPT_TYPE_DURATION:
>      case AV_OPT_TYPE_CHANNEL_LAYOUT:
> -    case AV_OPT_TYPE_INT64:
> -        *(int64_t *)dst = llrint(num / den) * intnum;
> -        break;
> +    case AV_OPT_TYPE_INT64:{
> +        double d = num / den;
> +        if (intnum == 1 && d == (double)INT64_MAX) *(int64_t *)dst = INT64_MAX;
> +        else                                       *(int64_t *)dst = llrint(d) * intnum;
> +        break;}
>      case AV_OPT_TYPE_FLOAT:
>          *(float *)dst = num * intnum / den;
>          break;
> 

LGTM. Thanks for finding a better fix for the problem!

Best regards,
Andreas
Michael Niedermayer Nov. 20, 2016, 8:50 p.m. UTC | #3
On Sun, Nov 20, 2016 at 08:53:19PM +0100, Andreas Cadhalpun wrote:
> On 20.11.2016 12:57, Michael Niedermayer wrote:
> > Found-by: Andreas
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavutil/opt.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavutil/opt.c b/libavutil/opt.c
> > index cd16bd1..6669356 100644
> > --- a/libavutil/opt.c
> > +++ b/libavutil/opt.c
> > @@ -126,9 +126,11 @@ static int write_number(void *obj, const AVOption *o, void *dst, double num, int
> >          break;
> >      case AV_OPT_TYPE_DURATION:
> >      case AV_OPT_TYPE_CHANNEL_LAYOUT:
> > -    case AV_OPT_TYPE_INT64:
> > -        *(int64_t *)dst = llrint(num / den) * intnum;
> > -        break;
> > +    case AV_OPT_TYPE_INT64:{
> > +        double d = num / den;
> > +        if (intnum == 1 && d == (double)INT64_MAX) *(int64_t *)dst = INT64_MAX;
> > +        else                                       *(int64_t *)dst = llrint(d) * intnum;
> > +        break;}
> >      case AV_OPT_TYPE_FLOAT:
> >          *(float *)dst = num * intnum / den;
> >          break;
> > 
> 
> LGTM. Thanks for finding a better fix for the problem!

applied

thx

[...]
diff mbox

Patch

diff --git a/libavutil/opt.c b/libavutil/opt.c
index cd16bd1..6669356 100644
--- a/libavutil/opt.c
+++ b/libavutil/opt.c
@@ -126,9 +126,11 @@  static int write_number(void *obj, const AVOption *o, void *dst, double num, int
         break;
     case AV_OPT_TYPE_DURATION:
     case AV_OPT_TYPE_CHANNEL_LAYOUT:
-    case AV_OPT_TYPE_INT64:
-        *(int64_t *)dst = llrint(num / den) * intnum;
-        break;
+    case AV_OPT_TYPE_INT64:{
+        double d = num / den;
+        if (intnum == 1 && d == (double)INT64_MAX) *(int64_t *)dst = INT64_MAX;
+        else                                       *(int64_t *)dst = llrint(d) * intnum;
+        break;}
     case AV_OPT_TYPE_FLOAT:
         *(float *)dst = num * intnum / den;
         break;