[FFmpeg-devel] [PATCH 1/7] avformat/vivo: Do not use the general expression evaluator for parsing a floating point value

[Anton Khirnov]

Quoting Michael Niedermayer (2021-12-06 11:35:50)
> On Mon, Dec 06, 2021 at 10:25:01AM +0100, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2021-12-05 22:19:01)
> > > Fixes: Timeout
> > > Fixes: 41564/clusterfuzz-testcase-minimized-ffmpeg_dem_VIVO_fuzzer-6309014024093696
> > > 
> > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > ---
> > >  libavformat/vivo.c | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/libavformat/vivo.c b/libavformat/vivo.c
> > > index b2904cd25a7..8e819d910b7 100644
> > > --- a/libavformat/vivo.c
> > > +++ b/libavformat/vivo.c
> > > @@ -206,11 +206,12 @@ static int vivo_read_header(AVFormatContext *s)
> > >                      return AVERROR_INVALIDDATA;
> > >                  value_used = 1;
> > >              } else if (!strcmp(key, "FPS")) {
> > > -                AVRational tmp;
> > > +                double d;
> > > +                if (sscanf(value, "%f", &d) != 1)
> > > +                    return AVERROR_INVALIDDATA;
> > >  
> > >                  value_used = 1;
> > > -                if (!av_parse_ratio(&tmp, value, 10000, AV_LOG_WARNING, s))
> > > -                    fps = av_inv_q(tmp);
> > > +                fps = av_d2q(1/d, 10000);
> > >              }
> > >  
> > >              if (!value_used)
> > > -- 
> > > 2.17.1
> > 
> > Won't this be inexact? 
> 
> you mean the 1/d or in general ?
> the code before used av_expr_parse_and_eval() in av_parse_ratio() so was
> using floating point already. The 1/d could be done in AVRational if you
> prefer?
> The intermediate is inexact, the end result i would expect to be exact
> as it should be the "closest" fraction with num and den below 10000
> 
> we also could do the whole without floating point. The actual fields
> all where of the format AB.XY so 2 digits after the decimal dot.
> if it was always that then its very easy to parse. But if some
> oddball 3E1 or .99 something turns up that could add bugs for little
> gain

I mean involving floats at all. If that's already the case, then I don't
object to the patch (though it would be nice to know if it is in fact
fixed point and parse it as such).

-- 
Anton Khirnov