[FFmpeg-devel] [Fwd: Summer of code small task patch]

[Michael Niedermayer]

On Fri, Mar 27, 2009 at 10:18:07PM +0200, Dylan Yudaken wrote:
>
> Michael Niedermayer wrote:
>> On Wed, Mar 25, 2009 at 10:06:28PM +0100, Diego Biurrun wrote:
>>   
>>> On Wed, Mar 25, 2009 at 09:22:15PM +0100, Michael Niedermayer wrote:
>>>     
>>>> On Wed, Mar 25, 2009 at 05:43:05PM +0200, Dylan Yudaken wrote:
>>>>       
>>>>> Kostya wrote:
>>>>>         
>>>> [...]
>>>>       
>>>>> /**
>>>>>  * Initialise the Double Precision Discrete Cosine Transform
>>>>>  * functions fdct & idct
>>>>>  *
>>>>>  */
>>>>> void ref_dct_init(void)
>>>>>         
>>>> av_cold and non static functions needs a ff prefix
>>>>       
>>> Do we need the prefix for this test program?
>>>     
>>
>> when its license is ok we could use it like any other dct ...
>>   
> attached an updated patch using the ff prefix (and the other changes). I 
> wasnt sure what the story was with them so I just added. I was under the 
> impression that this was supposed to just be a reference and not in normal 
> use. It could potentially be sped up a little if necessary but I was under 
> the impression it wasnt important.

anything that would impress us is good for qualification tasks ...

[...]
> Index: libavcodec/dct-test.c
> ===================================================================
> --- libavcodec/dct-test.c	(revision 18203)
> +++ libavcodec/dct-test.c	(working copy)
> @@ -46,9 +46,9 @@
>  void *fast_memcpy(void *a, const void *b, size_t c){return memcpy(a,b,c);};
>  
>  /* reference fdct/idct */
> -void fdct(DCTELEM *block);
> -void idct(DCTELEM *block);
> -void init_fdct(void);
> +void ff_ref_fdct(DCTELEM *block);
> +void ff_ref_idct(DCTELEM *block);
> +void ff_ref_dct_init(void);

renaming things should be a seperate patch
actually a patch should either do functional changes or non functional not
both


[...]
> +/**
> + * Initialize the Double Precision Discrete Cosine Transform
> + * functions fdct & idct.
> + */
> +void ff_ref_dct_init(void)

av_cold


> +{
> +    unsigned int i = 0, j;

> +    double c0  = sqrt(0.125);
> +
> +    for (j = 0; j < 8; ++j) {
> +        coefficients[i * 8 + j] = c0 * cos(i * (j + 0.5) * (M_PI / 8.0));
> +    }
> +    for (i = 1; i < 8; ++i) {
> +        for (j = 0; j < 8; ++j) {
> +            coefficients[i * 8 + j] = 0.5 * cos((i * (j + 0.5)) * (M_PI / 8.0));
> +        }
> +    }

this can be simplified


> +}
> +
> +/**
> + * Transform 8x8 block of data with a double precision forward DCT <br>
> + * This is a reference implementation.
> + *
> + * @param block Pointer to 8x8 block of data to transform
> + */

> +void ff_ref_fdct(short *block)
> +{
> +    /* This implements the equation block = A*block*A' */
> +
> +    unsigned int i, j, k;
> +    double tmp;
> +    double out[8 * 8];
> +
> +    /* out = coefficients*block */
> +    for (i = 0; i < 64; i+=8) {
> +        for (j = 0; j < 8; ++j) {
> +            tmp = 0;

declaration and initialization can be merged


> +            for (k = 0; k < 8; ++k) {
> +                tmp += coefficients[i + k] * block[8 * k + j];
> +            }
> +            out[i + j] = tmp;
> +        }
> +    }
> +
> +    /* block = out*(coefficients') */
> +    for (i = 0; i < 64; i+=8) {
> +        for (j = 0; j < 8; ++j) {
> +            tmp = 0;
> +            for (k = 0; k < 8; ++k) {
> +                tmp += out[i + k] * coefficients[j * 8 + k];
> +            }

> +            block[i + j] = (short) floor(8.0 * tmp + 0.499999999);

the multiply can be avoided


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

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090328/8d70aa27/attachment.pgp>