On Thu, Sep 30, 2021 at 12:11:13PM +0000, Soft Works wrote:
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
> > Michael Niedermayer
> > Sent: Thursday, 30 September 2021 14:04
> > To: FFmpeg development discussions and patches <ffmpeg-
> > de...@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 <softwo...@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.I dont see what would stop a file that encodes a 64bit aspect from being read as 64bit and then randomly and silently truncated neither before nor after this patch. The type is read and passed to get_value() its not checked or i have missed it Either 64bit should be supported and converted to a supported ratio or it should produce some warning/error not silent truncation I dont understand why adding a explicit cast to int would improve anything. thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Modern terrorism, a quick summary: Need oil, start war with country that has oil, kill hundread thousand in war. Let country fall into chaos, be surprised about raise of fundamantalists. Drop more bombs, kill more people, be surprised about them taking revenge and drop even more bombs and strip your own citizens of their rights and freedoms. to be continued
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
