[FFmpeg-devel] [PATCH] Add a time_base field to AVFilterPad.

[Michael Niedermayer]

On Wed, Oct 06, 2010 at 02:00:53AM +0200, Stefano Sabatini wrote:
> On date Tuesday 2010-10-05 13:48:41 +0200, Michael Niedermayer encoded:
> > On Tue, Oct 05, 2010 at 11:16:51AM +0200, Stefano Sabatini wrote:
> > > On date Monday 2010-10-04 00:12:15 +0200, Michael Niedermayer encoded:
> > > > On Sun, Oct 03, 2010 at 11:23:49PM +0200, Stefano Sabatini wrote:
> > > > > On date Sunday 2010-10-03 21:07:55 +0200, Michael Niedermayer encoded:
> > > [...]
> > > > > static int config_output(AVFilterLink *outlink)
> > > > > {
> > > > >     AVFilterContext *ctx = outlink->src;
> > > > > 
> > > > >     /* choose the output timebase */
> > > > >     AVRational tb1 = ctx->input_pads[MAIN   ].time_base;
> > > > >     AVRational tb2 = ctx->input_pads[OVERLAY].time_base;
> > > > >     AVRational tb;
> > > > >     int exact = 1;
> > > > > 
> > > > >     exact *= av_reduce(&tb1.num, &tb1.den, tb1.num, tb1.den, INT32_MAX);
> > > > >     exact *= av_reduce(&tb2.num, &tb2.den, tb2.num, tb2.den, INT32_MAX);
> > > > >     exact *= av_reduce(&tb.num, &tb.den,
> > > > >                        (int64_t)tb1.num * tb2.num, (int64_t)tb1.den * tb2.den, INT_MAX);
> > > > 
> > > > your code makes no sense
> > > > with a/b and c/d
> > > > 
> > > > you need something like:
> > > > reduce(gcd(ad,cb)/bd)
> > > 
> > > AVRational = {int, int}
> > >
> > 
> > > int*int may overflow int64_t,
> > 
> > not on any supported platform nor would it work if such platform was supported
> > because that case would then inevtitable fail on all other platforms
> > 
> > 
> > > so I convert {int,int} to {int32_t,int32_t}.
> > 
> > I couldnt fit 2 apples in the box so i cut them in half and put 2 halfs in
> > that solves the problem that i cannot put more than 1 whole apple in
> > 
> > 
> > > 
> > > Finally I do int32_t*int32_t in the third reduce, so I know that it
> > > can't overflow int64_t:
> > >    exact *= av_reduce(&tb.num, &tb.den,
> > >                       (int64_t)tb1.num * tb2.num, (int64_t)tb1.den * tb2.den, INT_MAX);
> > 
> > your code makes absolutely no sense
> > 
> > try
> > tb1=1/6
> > tb2=1/15
> > correct result=1/30
> > your code produces 1/90 this is wrong its not overflow you apply the wrong
> > math to it like doing a multiplication where you would need divission
> 
> Uhm.. yes I'm (I was?) stupid, give me some more time to meditate on
> this, hope the math is right now, a possibly more correct code should
> be:
> 
> static int config_output(AVFilterLink *outlink)
> {
>     AVFilterContext *ctx = outlink->src;
> 
>     // common timebase computation:
>     // tb1 = n1 / d1
>     // tb2 = n2 / d2
>     // tb = gcd(n1, n2) / lcm(d1, d2) =
>     //      gcd(n1, n2) * gcd(d1, d2) / d1*d2

tb1=5/5
tb2=1/1

gcd(5, 1) * gcd(5, 1) / 5*1



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

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- 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/20101006/d00cc4e2/attachment.pgp>