[FFmpeg-devel] [RCF] lavfi aspect ratio setting path

[Michael Niedermayer]

On Sun, Jan 16, 2011 at 03:23:23PM -0800, Baptiste Coudurier wrote:
> On 1/16/11 3:20 PM, Michael Niedermayer wrote:
> > On Sun, Jan 16, 2011 at 04:56:42PM +0100, Stefano Sabatini wrote:
> >> On date Sunday 2010-12-12 12:34:10 -0800, Baptiste Coudurier encoded:
> >>> On 12/3/10 9:53 PM, Baptiste Coudurier wrote:
> >>>> On 12/2/10 6:28 PM, Michael Niedermayer wrote:
> >>>>> On Mon, Nov 29, 2010 at 03:26:40AM -0800, Baptiste Coudurier wrote:
> >>>>> [...]
> >>>>>> @@ -419,6 +430,10 @@
> >>>>>>  
> >>>>>>      codec->width  = ist->output_video_filter->inputs[0]->w;
> >>>>>>      codec->height = ist->output_video_filter->inputs[0]->h;
> >>>>>> +    ost->st->sample_aspect_ratio = codec->sample_aspect_ratio =
> >>>>>
> >>>>>> +        frame_aspect_ratio == 0 ? // overriden by the -aspect cli option
> >>>>>> +        av_d2q(frame_aspect_ratio*codec->height/codec->width, 255) :
> >>>>>> +        ist->output_video_filter->inputs[0]->sample_aspect_ratio;
> >>>>>
> >>>>> that looks odd if frame_aspect_ratio == 0 then
> >>>>>  av_d2q(frame_aspect_ratio*codec->height/codec->width, 255)
> >>>>>  will be used
> >>>>>  but thats av_d2q(0*codec->height/codec->width, 255)=0
> >>>>
> >>>> Yes you are right, it required more modifications to make it work.
> >>>> Updated patch.
> >>>>
> >>>
> >>> Ping.
> >>
> >> Any news on this? This is a show-stopper for many users.
> > 
> > this patch does so many unrelated things iam just confused by it, i know
> > some of the changes are definitly wrong and introduce bugs then for some
> > i have no idea at all what they are supposed to do
> 
> Like what ?

the vf_scale change
after it there is no code left touching aspect in it but if you scale
320x240 to 320x320 then the sample aspect ratio changes.
There are changes in ffmpeg to code only excuted for stream copy (aka no
filters, no scaling) that cant be correct, the stream copy handling should not
change.


> We agreed that sample_aspect_ratio must be added to AVFilterLink, then
> according to this, ffmpeg.c is modified.

Theres nothing wrong with adding it to AVFilterLink.

But this whole thread IIRC started due to the command line option -aspect not
working. This can be fixed by inserting a filter that sets the aspect instead
of a second codepath to mess with the aspect. (we missed this option it seems
or maybe the suggested solution looked simpler i dont remember...)

And this patch looks like it attempts to do both at once. which i dont think
is a good idea.

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

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- 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/20110117/25963cc1/attachment.pgp>