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

[Stefano Sabatini]

On date Friday 2010-11-19 19:14:28 -0800, Baptiste Coudurier encoded:
> 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.

The aspect filters have another problem, they may fail with
some encoder which needs to set the DAR/SAR when openining the
context, because they set DAR/SAR frame by frame (while this *may* be
a global property).

If this is the case (I need confirmation from someone with more
expertise in encoders), then the only way to fix the problem is to
make the aspect a property of the link, and make it configurable
during the configuration stage. In case the aspect changes during
filtering, we may need to re-configure the filtergraph (as it should
happen when w/h change).
-- 
FFmpeg = Fundamental Fancy Most Problematic Evangelical Gadget