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

[Michael Niedermayer]

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?

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

It is not what we do, but why we do it that matters.
-------------- 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/20101120/f56b066e/attachment.pgp>