[FFmpeg-devel] [RCF] lavfi aspect ratio setting path
Michael Niedermayer
michaelni
Mon Jan 17 03:18:35 CET 2011
On Sun, Jan 16, 2011 at 06:06:06PM -0800, Baptiste Coudurier wrote:
> On 1/16/11 6:01 PM, Michael Niedermayer wrote:
> > On Mon, Jan 17, 2011 at 02:52:29AM +0100, Michael Niedermayer wrote:
> >> On Sun, Jan 16, 2011 at 04:16:01PM -0800, Baptiste Coudurier wrote:
> >>> 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 ?
> >>
> >> because if the aspect is correct before scale it wont be correct anymore
> >> afterwards this happens for example in a real 640x480 ->320x200 rescaling
> >>
> >>
> >>>
> >>>> 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
> >>
> >> Well as is i cannot review this patch because quite a few hunks make no sense
> >> and i dont even know which of the now 5+ things this patch combines they are
> >> related to.
> > [...]
> >
> >> * unrelated simplification of vf_transpose that might or might not break it
> >
> > breaks, as aspect is defined like this in avcodec and coped from and to it
> > over avfilter
> > /**
> > * sample aspect ratio (0 if unknown)
> > * That is the width of a pixel divided by the height of the pixel.
> > * Numerator and denominator must be relatively prime and smaller than 256 for some video standards.
> > * - encoding: Set by user.
> > * - decoding: Set by libavcodec.
> > */
> > AVRational sample_aspect_ratio;
> >
> > the change turns 0 into inf
> >
> > not saying this definition is smart, IMHO it should be 0/0 for undefined but
> > thats not how the current ABI/API defines it
>
> What are you talking about ? I remove the picref->pixel_aspect field.
> The corresponding change must be to use
> AVFilterLink->sample_aspect_ratio. This hunk is correct.
i talk about this:
Index: libavfilter/vf_transpose.c
===================================================================
--- libavfilter/vf_transpose.c (revision 26397)
+++ libavfilter/vf_transpose.c (working copy)
@@ -102,6 +102,9 @@
outlink->w = inlink->h;
outlink->h = inlink->w;
+ outlink->sample_aspect_ratio.num = inlink->sample_aspect_ratio.den;
+ outlink->sample_aspect_ratio.den = inlink->sample_aspect_ratio.num;
+
av_log(ctx, AV_LOG_INFO, "w:%d h:%d dir:%d -> w:%d h:%d rotation:%s vflip:%d\n",
inlink->w, inlink->h, trans->dir, outlink->w, outlink->h,
trans->dir == 1 || trans->dir == 3 ? "clockwise" : "counterclockwise",
@@ -117,13 +120,6 @@
outlink->w, outlink->h);
outlink->out_buf->pts = picref->pts;
- if (picref->video->pixel_aspect.num == 0) {
- outlink->out_buf->video->pixel_aspect = picref->video->pixel_aspect;
- } else {
- outlink->out_buf->video->pixel_aspect.num = picref->video->pixel_aspect.den;
- outlink->out_buf->video->pixel_aspect.den = picref->video->pixel_aspect.num;
- }
-
avfilter_start_frame(outlink, avfilter_ref_buffer(outlink->out_buf, ~0));
}
the code before keep 0 as 0 but afterwards turn 0 to infinity
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- 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/20110117/4f0ecdee/attachment.pgp>
More information about the ffmpeg-devel
mailing list