[FFmpeg-devel] [PATCH] Document av_gcd()

[Stefano Sabatini]

On date Saturday 2009-11-28 01:30:30 +0100, Stefano Sabatini encoded:
> On date Sunday 2009-03-01 19:34:31 +0100, Stefano Sabatini encoded:
> > On date Saturday 2009-02-28 18:49:27 +0100, Reimar D?ffinger encoded:
> > > On Sat, Feb 28, 2009 at 06:23:37PM +0100, Michael Niedermayer wrote:
> > > > On Sat, Feb 28, 2009 at 04:53:08PM +0100, Stefano Sabatini wrote:
> > > > > Hi,
> > > > > as in subject, regards.
> > > > > -- 
> > > > > FFmpeg = Funny and Forgiving Multimedia Pitiful Elected Ghost
> > > > 
> > > > > Index: libavutil/mathematics.h
> > > > > ===================================================================
> > > > > --- libavutil/mathematics.h	(revision 17629)
> > > > > +++ libavutil/mathematics.h	(working copy)
> > > > > @@ -50,6 +50,9 @@
> > > > >      AV_ROUND_NEAR_INF = 5, ///< Round to nearest and halfway cases away from zero.
> > > > >  };
> > > > >  
> > > > > +/**
> > > > > + * Returns the greatest common divisor of a and b.
> > > > > + */
> > > > >  int64_t av_const av_gcd(int64_t a, int64_t b);
> > > > >  
> > > > >  /**
> > > > 
> > > > ok,also please add
> > > > "if either or both are <=0 then the behavior is undefined"
> > > 
> > > My r_frame_rate code in utils.c uses the = 0 case already, since I
> > > considered that the common mathematical definition...
> > 
> > Trying to find a compromise, I think documenting the case gcd(a, 0) = a
> > doesn't hurt.
> 
> Patches updated, implementing the case av_gcd(a, 0) = a rather than
> av_gcd(0, b) = b we also have a theoretical speed advantage (one call
> to function against two), but what's more important we don't need to
> document/specify the behavior for the case:
> av_gcd(0, b)
> 
> which may simplify optimizations.
> 
> Regards.
> -- 
> FFmpeg = Frenzy and Free Mysterious Prodigious Eccentric Geek

> Index: ffmpeg/libavformat/nutdec.c
> ===================================================================
> --- ffmpeg.orig/libavformat/nutdec.c	2009-11-28 01:06:59.000000000 +0100
> +++ ffmpeg/libavformat/nutdec.c	2009-11-28 01:07:25.000000000 +0100
> @@ -207,7 +207,7 @@
>      for(i=0; i<nut->time_base_count; i++){
>          GET_V(nut->time_base[i].num, tmp>0 && tmp<(1ULL<<31))
>          GET_V(nut->time_base[i].den, tmp>0 && tmp<(1ULL<<31))
> -        if(av_gcd(nut->time_base[i].num, nut->time_base[i].den) != 1){
> +        if(av_gcd(nut->time_base[i].den, nut->time_base[i].num) != 1){
>              av_log(s, AV_LOG_ERROR, "time base invalid\n");
>              return -1;
>          }
> Index: ffmpeg/libavformat/utils.c
> ===================================================================
> --- ffmpeg.orig/libavformat/utils.c	2009-11-28 01:09:45.000000000 +0100
> +++ ffmpeg/libavformat/utils.c	2009-11-28 01:18:57.000000000 +0100
> @@ -2183,7 +2183,7 @@
>                  duration_count[index]++;
>                  // ignore the first 4 values, they might have some random jitter
>                  if (duration_count[index] > 3)
> -                    duration_gcd[index] = av_gcd(duration_gcd[index], duration);
> +                    duration_gcd[index] = av_gcd(duration, duration_gcd[index]);

Since I don't want to waste the time of the reviewer(s) and of the
casual reader, just want to add that duration is >0 in this block,
in av_find_stream_info(), patch updated with a comment.

[...]

Regards.
-- 
FFmpeg = Fiendish Funny Mega Pitiful Experimenting Geek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: flip-undefined-gcd-use.patch
Type: text/x-diff
Size: 2513 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091128/a4fe021f/attachment.patch>