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

[Michael Niedermayer]

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?


> +                     */
> +                    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


>              }
>          } 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 ()

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100922/850ad00a/attachment.pgp>