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

[Michael Niedermayer]

On Sat, Nov 20, 2010 at 12:42:52PM +0100, Stefano Sabatini wrote:
> 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).

SAR should be at a similar level as w/h
alot of encoders and formats wont allow changing sar (like they dont allow w/h
changes)
and a sar change if it occurs is very likely also to change the w/h
and if we have a input that changes sar (and w/h) a scale filter somewhere
could be used to get rid of these changes for outputs that cant handle it



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

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- 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/cb606717/attachment.pgp>