[Ffmpeg-devel] [RFC] av_random...

Michael Niedermayer michaelni
Tue Jan 9 22:14:56 CET 2007


Hi

On Tue, Jan 09, 2007 at 02:13:51PM -0600, Ryan Martell wrote:
> 
> On Jan 8, 2007, at 8:24 PM, Michael Niedermayer wrote:
> 
> >Hi
> >
> >On Mon, Jan 08, 2007 at 11:56:24AM -0600, Ryan Martell wrote:
> >[...]
> >>>>{
> >>>>   unsigned int y= 0;
> >>>
> >>>y is not read before its written so the =0 is redundant
> >>
> >>Fixed.
> >>
> >>How's this?
> >>
> >
> >[...]
> >>#include <stdio.h>
> >>#include "random.h"
> >>
> >>//#define DEBUG
> >>
> >>/* Period parameters */
> >>#define M 397
> >>#define A 0x9908b0df	/* constant vector a */
> >>#define UPPER_MASK 0x80000000	/* most significant w-r bits */
> >>#define LOWER_MASK 0x7fffffff	/* least significant r bits */
> >>
> >>/** initializes mt[AV_RANDOM_N] with a seed */
> >>void av_init_random(unsigned long seed, AVRandomState *state)
> >>{
> >>    int index;
> >>
> >>    /*
> >>     This differs from the wikipedia article.  Source is from the  
> >>Makoto
> >>     Makoto Matsumoto and Takuji Nishimura code, with the  
> >>following comment:
> >>     */
> >>     /* See Knuth TAOCP Vol2. 3rd Ed. P.106 for multiplier. */
> >>     /* In the previous versions, MSBs of the seed affect   */
> >>     /* only MSBs of the array mt[].                        */
> >>    state->mt[0] = seed & 0xffffffff;
> >
> >if seed is a uint32_t then the & 0xffffffff; becomes unneeded
> 
> if i don't use a uint32_t, then the code has no dependencies on  
> ffmpeg header files, and it can be compiled with gcc random.c.  I  
> kind of like that better than getting rid of the single and on  
> something that only happens infrequently.  Okay?

ok


[...]
> #ifdef DEBUG
> void av_benchmark_random(void)
> {
>     int i;
>     AVRandomState state;
>     
>     av_init_random(0xdeadbeef, &state);
> 
>     av_log(NULL, AV_LOG_ERROR, "1000 outputs of av_random()\n");
>     {
>         START_TIMER;
>         for (i = 0; i < 1000; i++) {
>             av_random(&state);
>         }
>         STOP_TIMER("1000 calls of av_random");

this should rather be
    int x=0;
    for (j = 0; j < 100; j++) {
        START_TIMER;
        x+= av_random(&state);
        STOP_TIMER("first call to av_random");
        for (i = 1; i < AV_RANDOM_N; i++) {
            START_TIMER;
            x+= av_random(&state);
            STOP_TIMER("AV_RANDOM_N calls of av_random");
        }
    }
    av_log(NULL, AV_LOG_ERROR, "final value:%X\n", x);

without the x, the compiler can simply optimize the whole av_random() out
(gcc of course is too stupid to do this but still it might remove a few
operations at random)

and spliting the quick and slow cases provides more information, also
*_TIMER is designed for code which is approxiately equally fast on each
run and thats where it provides the best accuracy ...

printing the final value gives some regression test (a simple if(x==expected)
ould be fine too, the thing that is important is that x is used otherwise
a compiler could remove x and the whole av_random())

except that the code looks good ...

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070109/b5678d09/attachment.pgp>



More information about the ffmpeg-devel mailing list