[FFmpeg-devel] [PATCH2/3] asfdec.c doesn't detectaspectratio properly in some case

[Richard Buteau]

> -----Original Message-----
> From: ffmpeg-devel-bounces at mplayerhq.hu [mailto:ffmpeg-devel-
> bounces at mplayerhq.hu] On Behalf Of Michael Niedermayer
> Sent: Tuesday, September 21, 2010 7:12 PM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [PATCH2/3] asfdec.c doesn't
> detectaspectratio properly in some case
> 
> On Tue, Sep 21, 2010 at 03:49:42PM -0700, Richard Buteau wrote:
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel-bounces at mplayerhq.hu [mailto:ffmpeg-devel-
> > > bounces at mplayerhq.hu] On Behalf Of Michael Niedermayer
> > > Sent: Tuesday, September 21, 2010 3:02 PM
> > > To: FFmpeg development discussions and patches
> > > Subject: Re: [FFmpeg-devel] [PATCH2/3] asfdec.c doesn't detect
> > > aspectratio properly in some case
> > >
> > > On Tue, Sep 21, 2010 at 10:15:38AM -0700, Richard Buteau wrote:
> > > > Hi all,
> > > > In some case ASF file don't have aspect ratio set in
> > > > ff_asf_metadata_header but in ff_asf_extended_content_header.
> > > > The results was a stretch out file.
> > > > This patch extract that aspect ratio info and pass it down to
the
> > > > container.
> > > > Richard
> > > >
> > > >
> > >  [...]
> > > Content-Description: asfAR.patch.txt
> > > > diff -uNr -x .svn -x '*.d' ffmpeg/libavformat/asfdec.c
> > > ffmpeg.asfAR/libavformat/asfdec.c
> > > > --- ffmpeg/libavformat/asfdec.c	2010-09-14 13:04:36.000000000
> > -0500
> > > > +++ ffmpeg.asfAR/libavformat/asfdec.c	2010-09-21
> > 11:54:10.000000000
> > > -0500
> > > > @@ -467,6 +467,12 @@
> > > >                      value_len  = get_le16(pb);
> > > >                      if (!value_type && value_len%2)
> > > >                          value_len += 1;
> > > > +                    int stream_num=0; //Ideally we should find
> > > > + the
> > > video stream and set it there. My sample has that stream set to 0
> > maybe
> > > that mean the container.
> > >
> > > breaks gcc 2.95
> > > try tools/patcheck please
> > > and assigning global information to just one stream is wrong
> > >
> > > and please upload the asf file that needs this
> > >
> > If for some reason there is a stream with both
ff_asf_metadata_header
> > and ff_asf_extended_content_header present AR info could get
> > overwritten if I blindly copied it all over. At Line 641 I passed it
> > down to each stream that has no AR and is CODEC_TYPE_VIDEO, so this
> should be safe.
> > I added a check for CODEC_TYPE_VIDEO and I clean up the errors from
> > patcheck.
> >
> > Updated patch attached. (Passed the tools/patcheck)
> >
> > File uploaded in uploaded in
> > ffmpeg.org:/MPlayer/incoming/source_file.wmv
> > >
> > > [...]
> > > --
> > > Michael     GnuPG fingerprint:
> > 9FF2128B147EF6730BADF133611EC787040B0FAB
> > >
> > > Good people do not need laws to tell them to act responsibly,
while
> > bad
> > > people will find a way around the laws. -- Plato
> 
> Content-Description: asfAR.patch_updated.txt
> > diff -uNr -x .svn -x '*.d' ffmpeg/libavformat/asfdec.c
> ffmpeg.asfAR/libavformat/asfdec.c
> > --- ffmpeg/libavformat/asfdec.c	2010-09-14 13:04:36.000000000
-0500
> > +++ ffmpeg.asfAR/libavformat/asfdec.c	2010-09-21
17:43:41.000000000
> -0500
> > @@ -467,7 +467,15 @@
> >                      value_len  = get_le16(pb);
> >                      if (!value_type && value_len%2)
> >                          value_len += 1;
> > -                    get_tag(s, name, value_type, value_len);
> > +                    /**
> > +                     * My sample has that stream set to 0 maybe
that
> mean the container.
> > +                     * Asf stream count start at 1. I am using 0 to
> > + the container value since it's unused
> 
> what gurantees that 0 is unused?
Nothing. I was just trying to reuse what is there.
I have saved it somewhere else.
> 
> 
> > +                     */
> > +                    if (!strcmp(name, "AspectRatioX")){
> > +                        dar[0].num= get_value(s->pb, value_type);
> > +                    } else if(!strcmp(name, "AspectRatioY")){
> > +                        dar[0].den= get_value(s->pb, value_type);
> 
> > +                    } else get_tag(s, name, value_type, value_len);
> 
> add a newline between else and get_tag() please
fixed

> 
> 
> >              }
> >          } else if (!guidcmp(&g, &ff_asf_metadata_header)) {
> >              int n, stream_num, name_len, value_len, value_type,
> >value_num;  @@ -626,11 +634,16 @@
> >              AVStream *st = s-
> >streams[stream_num];
> >              if (!st->codec->bit_rate)
> >                  st->codec->bit_rate = bitrate[i];
> > -            if (dar[i].num > 0 && dar[i].den > 0)
> > +            if (dar[i].num > 0 && dar[i].den > 0){
> >                  av_reduce(&st->sample_aspect_ratio.num,
> >                            &st->sample_aspect_ratio.den,
> >                            dar[i].num, dar[i].den, INT_MAX);
> > -//av_log(s, AV_LOG_ERROR, "dar %d:%d sar=%d:%d\n", dar[i].num,
> > dar[i].den, st->sample_aspect_ratio.num, st-
> >sample_aspect_ratio.den);
> > +            } else if ((dar[0].num > 0) && (dar[0].den > 0) && (st-
> >codec->codec_type==CODEC_TYPE_VIDEO)) // Use ASF container value if
> the stream doesn't AR set.
> 
> superflous ()
Removed ()
> 
> [...]
> --
> Michael     GnuPG fingerprint:
9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Asymptotically faster algorithms should always be preferred if you
have
> asymptotical amounts of data
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: asfAR.patch_updatedv2.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100922/5961d0a5/attachment.txt>