[FFmpeg-devel] [PATCH v5 2/7] libavformat/asfdec: Fix get_value return type

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Michael Niedermayer
> Sent: Thursday, 30 September 2021 14:04
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v5 2/7] libavformat/asfdec: Fix
> get_value return type
> 
> On Thu, Sep 30, 2021 at 01:45:46PM +0200, Michael Niedermayer wrote:
> > On Thu, Sep 30, 2021 at 02:58:30AM +0000, Soft Works wrote:
> > > get_value had a return type of int, which means that reading
> > > QWORDS (case 4) was broken due to truncation of the result from
> > > avio_rl64().
> > >
> > > Signed-off-by: softworkz <softworkz at hotmail.com>
> > > ---
> > > v5: Split into pieces as requested
> > >
> > >  libavformat/asfdec_f.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
> > > index 72ba8b32a0..076b5ab147 100644
> > > --- a/libavformat/asfdec_f.c
> > > +++ b/libavformat/asfdec_f.c
> > > @@ -202,7 +202,7 @@ static int asf_probe(const AVProbeData *pd)
> > >
> > >  /* size of type 2 (BOOL) is 32bit for "Extended Content
> Description Object"
> > >   * but 16 bit for "Metadata Object" and "Metadata Library
> Object" */
> > > -static int get_value(AVIOContext *pb, int type, int type2_size)
> > > +static uint64_t get_value(AVIOContext *pb, int type, int
> type2_size)
> > >  {
> > >      switch (type) {
> > >      case 2:
> > > @@ -568,9 +568,9 @@ static int
> asf_read_ext_content_desc(AVFormatContext *s, int64_t size)
> > >           * ASF stream count starts at 1. I am using 0 to the
> container value
> > >           * since it's unused. */
> > >          if (!strcmp(name, "AspectRatioX"))
> > > -            asf->dar[0].num = get_value(s->pb, value_type, 32);
> > > +            asf->dar[0].num = (int)get_value(s->pb, value_type,
> 32);
> > >          else if (!strcmp(name, "AspectRatioY"))
> > > -            asf->dar[0].den = get_value(s->pb, value_type, 32);
> > > +            asf->dar[0].den = (int)get_value(s->pb, value_type,
> 32);
> > >          else
> > >              get_tag(s, name, value_type, value_len, 32);
> > >      }
> > > @@ -630,11 +630,11 @@ static int
> asf_read_metadata(AVFormatContext *s, int64_t size)
> > >                  i, stream_num, name_len_utf16, value_type,
> value_len, name);
> > >
> > >          if (!strcmp(name, "AspectRatioX")){
> > > -            int aspect_x = get_value(s->pb, value_type, 16);
> > > +            int aspect_x = (int)get_value(s->pb, value_type,
> 16);
> > >              if(stream_num < 128)
> > >                  asf->dar[stream_num].num = aspect_x;
> > >          } else if(!strcmp(name, "AspectRatioY")){
> > > -            int aspect_y = get_value(s->pb, value_type, 16);
> > > +            int aspect_y = (int)get_value(s->pb, value_type,
> 16);
> > >              if(stream_num < 128)
> > >                  asf->dar[stream_num].den = aspect_y;
> > >          } else {
> >
> > a 64bit/64bit aspect does not work after this, it still just
> silently
> > truncates the value to 32bit, the truncation just happens later
> 
> forgot: see av_reduce()

These lines are not about making any change. The get_value() calls
for AspectRatioX/Y are returning 16bit values, before and after.

I added the explicit cast to (int) because get_value() doesn't return
int anymore and as such, to indicate that the cast is intentional.
But the values in this case are limited to 16bit integers anyway.

softworkz