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

[Michael Niedermayer]

On Sun, Jan 16, 2011 at 04:16:01PM -0800, Baptiste Coudurier wrote:
> On 1/16/11 4:00 PM, Michael Niedermayer wrote:
> > 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.
> 
> Well, currently -s does not change it, why -vf scale should change it ?

because if the aspect is correct before scale it wont be correct anymore
afterwards this happens for example in a real 640x480 ->320x200 rescaling


>
> > 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.
> 
> You cannot set frame_aspect_ratio unconditionally anymore if you want
> "-aspect" to work with libavfilter.
> And -aspect is required to work for the stream copy case., which makes
> the cli very inconsistent if "-aspect" does not work when encoding.
> 
> I double checked my tree and I don't have these changes, this must be an
> old patch.
> 
> >> 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...)
> 
> Not only, aspect ratio information is not printed correctly in
> dump_format and this heavily confuses people.
>


> > And this patch looks like it attempts to do both at once. which i dont think
> > is a good idea.
> 
> I don't care about this, I want the situation fixed. It's been two

Well as is i cannot review this patch because quite a few hunks make no sense
and i dont even know which of the now 5+ things this patch combines they are
related to.
* fix vf_pad,
* break vf_scale,
* unrelated simplification of vf_transpose that might or might not break it
* reanme aspect_ratio to sample_aspect_ratio
* pass aspect differently through libavfilter
* maybe fix -aspect



> months and nobody cares about it, that proves that no dev actually use
> ffmpeg.
> 
> Here is what I have in my tree since a lot of time now.

> Btw why would the pad filter sets sar to 1:1 anyway ?

because vf_pad is buggy

ill apply a few part of your patch, thank you for looking into them

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

Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- 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/e020756a/attachment.pgp>