[XviD-devel] quant functions b0rked

[Edouard Gomez]

Edouard Gomez (ed.gomez at free.fr) wrote:
> The errors comes from the asumption (sign(DC) = 0 if DC>0, 1 otherwise)
> sign(DC)*dcscaler/2 == sign(DC) xor dcscaler/2.

Oops, i misunderstood that part of the code, in reality it's just
fine. The bogus code is the last part of dc quantization.

So far the algorithm for this pseudo code:
if(dc<0) dc -= dcscaler/2;
else dc += dcscaler/2;

And it's wrong there:
dc *= ((1<<16)/dcscaler + 1);
dc >>= 16;

Here's the  concerned ASM code extracted from  the quantization function
and turned into  and independent function that is  supposed to scale the
DC coefficient only:

       ;; int scale_dc_only(int32_t coeff, int32_t dscaler);
       ;; We assume:
       ;; sign(x) == 0 if x >= 0
       ;; sign(x) == 1 if x <  0
align 4
int_div
dd 0
%assign i 1
%rep 255 
     dd  (1 << 16) / (i)
     %assign i i+1
%endrep

align ALIGN
cglobal scale_dc_only_asm
scale_dc_only_asm:
        push    ebp
        push    edi

        mov     eax, [esp + 8 + 4]      ; eax = coeff
        mov     ebp, [esp + 8 + 8]      ; ebp = dscaler
        mov     edi, eax                ; edi = coeff
        sar     edi, 31                 ; edi = sign(coeff)..sign(coeff)
        shr     ebp, 1                  ; ebp = dscaler/2
        xor     ebp, edi                ; ebp = (sign(coeff)?-1:1)*dscaler/2 - sign(coeff)
        sub     eax, edi                ; eax = coeff + sign(coeff)
        lea     eax, [byte eax + ebp]   ; eax = coeff + (sign(coeff)?-1:1)*dscaler/2
        mov     edi, [esp + 8 + 8]      ; edi = dscaler

;; The bogus step is here
        imul    eax, [int_div+4*edi]    ; /!\ should be eax = eax / dscaler
        sar     eax, 16
;; End of the bogus step

        pop     edi
        pop     ebp

        ret

The C code that we would like to mimic is:

int32_t scale_dc_only_c(int32_t coeff, int32_t dcscaler)
{
        double p;

        p = coeff;
        p /= dcscaler;
        if(p<0) p -= 0.5f;
        else p += 0.5f;

        return((int32_t)p);
}

Ok, that's  where i am atm, if  some asm guru finds  the solution, email
email... i'm not so good at rounding issues.

-- 
Edouard Gomez