[FFmpeg-devel] PATCH libswscale yuv2rgbtst infastructure

[Marc Hoffman]

Michael Niedermayer writes:
 > Hi
 > 
 > On Tue, May 01, 2007 at 06:31:32AM -0400, Marc Hoffman wrote:
 > Content-Description: message body text
 > > 
 > > yuv to rgb color space converter test, debug and performance
 > > infastructure.
 > > 
 > 
 > [...]
 > 
 > > +#include "swscale_internal.h"
 > 
 > this is not needed for testing
ok removed.
 > 
 > 
 > > +#include "rgb2rgb.h"
 > > +
 > > +#define ACCL (SWS_CPU_CAPS_MMX|SWS_CPU_CAPS_MMX2|SWS_CPU_CAPS_3DNOW|SWS_CPU_CAPS_ALTIVEC|SWS_CPU_CAPS_BFIN)
 > 
 > hmm

? how do I do this better than?  I thought about just setting the
flags to 0 but that doesn't seem right.

 > 
 > 
 > > +#define NITERS 10
 > > +
 > > +
 > > +#if defined(__MINGW32__) || defined(CONFIG_OS2)
 > > +static int64_t getutime(void)
 > > +{
 > > +    return av_gettime();
 > > +}
 > > +#else
 > > +static int64_t getutime(void)
 > > +{
 > > +    struct rusage rusage;
 > > +
 > > +    getrusage(RUSAGE_SELF, &rusage);
 > > +    return (rusage.ru_utime.tv_sec * 1000000LL) + rusage.ru_utime.tv_usec;
 > > +}
 > > +#endif
 > 
 > by how much does variance decrease if rusage.ru_utime is used?
 > if you dont know or its not much then please remove this

so are you saying just use av_gettime?

 > 
 > 
 > [...]
 > > +    SwsFunc csc_accl = 0;
 > > +    SwsFunc csc_c    = 0;
 > 
 > why are they set to 0? they are not read
 > 
style changed.

 > 
 > > +    int sws_flags    = 0;
 > 
 > useless
removed.

 > 
 > [...]
 > > +    st0 = getutime ();
 > > +    for (i=0;i<NITERS;i++) {
 > > +        csc_accl (sws, src, strides, 0, h   , dst, dstrides);
 > > +    }
 > > +
 > > +    st1 = getutime ();
 > > +    for (i=0;i<NITERS;i++) {
 > > +        csc_c (sws, src, strides, 0, h   , ref, dstrides);
 > > +    }
 > > +    st2 = getutime();
 > 
 > try:
 > 
 > csc_accl(sws, src, strides, 0, h   , dst, dstrides);
 > csc_c   (sws, src, strides, 0, h   , ref, dstrides);
 > for (i=0;i<NITERS;i++) {
 >     for(j=0; j<2; j++){
 >         st0 = getutime ();
 >         if(j==0) csc_accl(sws, src, strides, 0, h   , dst, dstrides);
 >         else     csc_c   (sws, src, strides, 0, h   , ref, dstrides);
 >         st1 = getutime ();
 >         mean[j]+= st1 - st0;
 >         var[j] +=(st1 - st0)*(st1 - st0);
 >     }
 > }
 > 
 > 1. this runs the code once before benchmaring to ensure its in the code cache
 > 2. it interleaves the tests so systematic errors (cron jobs, ...) are reduced
 > 3. it calculated mean & variance
 > 
Thanks! actually much better.

 > 
 > [...]
 > > +    if (argc < 3) {
 > > +        fprintf (stderr, "expecting the following arguments fname width height\n");
 > 
 > fname? you mean filename
 > 
good catch.

 > 
 > > +        exit (-1);
 > 
 > return -1
 > 
done.

 > [...]
 > 
 > > +    dst[1] = 0;
 > > +    dst[2] = 0;
 > 
 > uint8_t *dst[3]={0};

I changed all of them to look like this.

 > 
 > [...]
 > 
 > > +    free (dst[0]);
 > > +    free (ref[0]);
 > > +    free (src[0]);
 > 
 > free immedeatly before exit /return from main is useless
 > 
done.
 > 
 > > +
 > > +    exit (0);
 > 
 > return 0
done.

I'm also thinking that we should check this

    sws->flags |=  ACCL;
    csc_accl =  yuv2rgb_get_func_ptr (sws);
    sws->flags &=  ~(ACCL);
    csc_c    =  yuv2rgb_get_func_ptr (sws);

+    if (csc_c == csc_accl) {
+       av_log (0,0, "%s: no accelerator found\n", sws_format_name (c->dstFormat))
+       return 0;
+    }

    csc_accl(sws, src, strides, 0, h   , dst, dstrides);
    csc_c   (sws, src, strides, 0, h   , ref, dstrides);

What do you think?

Thanks
Marc