[FFmpeg-devel] [PATCH] demuxer aspect ratio

[Michael Niedermayer]

On Fri, Aug 22, 2008 at 01:23:37AM +0200, Aurelien Jacobs wrote:
> Michael Niedermayer wrote:
> 
> > On Fri, Aug 22, 2008 at 12:22:49AM +0200, Aurelien Jacobs wrote:
> > > Hi,
> > > 
> > > Currently, when a video must be rendered with non-square pixels, it is
> > > specified in AVCodecContext.sample_aspect_ratio. The problem is that
> > > this field is set by both decoders and demuxers. In some cases, the
> > > demuxer properly set this field with correct SAR, and then the
> > > decoder overwrite it with dumb values.
> > > I think we need an API so that the caller can chose to use either
> > > the value from the decoder or the demuxer (the demuxer one should
> > > generally be preferred if available).
> > > Attached patch adds a AVStream.display_aspect_ratio dedicated to
> > 
> > sample_aspect_ratio is strongly preferred over display.
> > It is much easier to work with as it does not change when croping or adding
> > borders. Also when rendering a frame into another that can be done when the
> > sample_aspect_ratio matches, it cannot when not without scaling ...
> 
> Oh, indeed. That's something I didn't considered.
> Now, I attached the same patch but using sample_aspect_ratio instead of
> display_aspect_ratio.
> 
[...]
> Index: ffplay.c
> ===================================================================
> --- ffplay.c	(revision 14883)
> +++ ffplay.c	(working copy)
> @@ -657,11 +657,14 @@
>      vp = &is->pictq[is->pictq_rindex];
>      if (vp->bmp) {
>          /* XXX: use variable in the frame */
> -        if (is->video_st->codec->sample_aspect_ratio.num == 0)
> -            aspect_ratio = 0;
> -        else
> +        if (is->video_st->sample_aspect_ratio.num)
> +            aspect_ratio = av_q2d(is->video_st->sample_aspect_ratio)
> +                * is->video_st->codec->width / is->video_st->codec->height;
> +        else if (is->video_st->codec->sample_aspect_ratio.num)
>              aspect_ratio = av_q2d(is->video_st->codec->sample_aspect_ratio)
>                  * is->video_st->codec->width / is->video_st->codec->height;
> +        else
> +            aspect_ratio = 0;

ok but 
* is->video_st->codec->width / is->video_st->codec->height;
can be factored out after this

rest ok, but a few things are missing
* ffmpeg.c should set it for muxers (and dont forget -vcodec copy)
* it maybe should be added to the AVOption array


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

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080822/a6a32cc0/attachment.pgp>