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

[Baptiste Coudurier]

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 ?

> 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
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 ?

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: avfilter_par.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110116/861d631e/attachment.txt>