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

[Baptiste Coudurier]

On 11/19/2010 06:55 PM, Michael Niedermayer wrote:
> On Thu, Nov 18, 2010 at 04:45:48PM -0800, Baptiste Coudurier wrote:
>> On 11/18/2010 04:33 PM, Michael Niedermayer wrote:
>>> On Thu, Nov 18, 2010 at 04:12:29PM -0800, Baptiste Coudurier wrote:
>>>> Hi Stefano,
>>>>
>>>> On 11/18/2010 01:27 PM, Stefano Sabatini wrote:
>>>>> Hi,
>>>>>
>>>>> -aspect currently works by changing the PAR in the video encoder
>>>>> context and in the output stream, thus readjusting the output DAR.
>>>>>
>>>>> If the libavfilter is enabled, in output_packet() we have:
>>>>>        if (ist->picref->video)
>>>>>            ost->st->codec->sample_aspect_ratio = ist->picref->video->pi
>>>>>
>>>>> so sample_aspect_ratio is set to the value in the filtered frame,
>>>>> thus ignoring the value set with -aspect.
>>>>>
>>>>> Workaround for this issue is either configure with --disable-avfilter
>>>>> and use -aspect, or use the aspect filter.
>>>>>
>>>>> I suggest to disable the -aspect option when using libavfilter, thus
>>>>> forcing the users to employ the aspect filter. Alternatively we could
>>>>> just drop support for -aspect in favor of libavfilter, but when that
>>>>> approach made sense with the pad and crop options which were
>>>>> "polluting" the ffmpeg code with much hard-to-maintain complex code,
>>>>> this doesn't seem the case for the aspect option and I still want to
>>>>> keep that option for users which for some reason (e.g. because of the
>>>>> lavfi regressions) are using an ffmpeg compiled with
>>>>> --disable-avfilter.
>>>>
>>>> First, thanks for looking into this.
>>>>
>>>> I, too, would like to keep the -aspect cli option.
>>>>
>>>> I have a patch in my tree that actually set the aspect at the end of
>>>> configure_filters and disable the code you quoted above.
>>>
>>> can you post that to the list so i can take a look?
>>> from your description it sounds like a good solution
>>
>> Here it is.
>>
>> --
>> Baptiste COUDURIER
>> Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
>> FFmpeg maintainer                                  http://www.ffmpeg.org
>
>>   ffmpeg.c |    7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>> 52f66ace0baf991b397cbe489f8df5b7623e0f33  0001-Fix-output-aspect-ratio-with-libavfilter.patch
>>  From 246c5cdba7defcd697597549fd110fb8b86cae62 Mon Sep 17 00:00:00 2001
>> From: bcoudurier<baptiste.coudurier at gmail.com>
>> Date: Sun, 24 Oct 2010 22:33:42 -0700
>> Subject: [PATCH 1/5] Fix output aspect ratio with libavfilter
>>
>> ---
>>   ffmpeg.c |    7 +++----
>>   1 files changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/ffmpeg.c b/ffmpeg.c
>> index 1681664..f06cc3e 100644
>> --- a/ffmpeg.c
>> +++ b/ffmpeg.c
>> @@ -450,6 +450,9 @@ static int configure_filters(AVInputStream *ist, AVOutputStream *ost)
>>
>>       codec->width  = ist->output_video_filter->inputs[0]->w;
>>       codec->height = ist->output_video_filter->inputs[0]->h;
>> +    codec->sample_aspect_ratio =
>> +        av_d2q(frame_aspect_ratio*codec->height/codec->width, 255);
>> +    ost->st->sample_aspect_ratio = codec->sample_aspect_ratio;
>
> this looks like it would break the aspect filter, in the sense that its
> value would be ignored. But maybe i miss something?

Yes, it will be ignored.
The aspect filter handling is weird anyway, dump_format and thus ffmpeg 
won't display the correct aspect ratio, I consider this broken so I 
disable his effects.

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org