[FFmpeg-devel] Dirac Golomb decoder [PATCH]

[Marco Gerards]

Michael Niedermayer <michaelni at gmx.at> writes:

Hi,

>>   * exp golomb vlc stuff
>>   * Copyright (c) 2003 Michael Niedermayer <michaelni at gmx.at>
>> + * Copyright (c) 2007 Marco Gerards <marco at gnu.org>
>>   *
>
> as reimar already said, we are affraid of ending up with hundreads of
> copyright lines ...

I removed it.

>> +const uint8_t ff_interleaved_dirac_golomb_vlc_len[256]={
>> +9,9,7,7,9,9,7,7,5,5,5,5,5,5,5,5,
>> +9,9,7,7,9,9,7,7,5,5,5,5,5,5,5,5,
>> +3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,
>> +3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,
>> +9,9,7,7,9,9,7,7,5,5,5,5,5,5,5,5,
>> +9,9,7,7,9,9,7,7,5,5,5,5,5,5,5,5,
>> +3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,
>> +3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,
>> +1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,
>> +1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,
>> +1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,
>> +1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,
>> +1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,
>> +1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,
>> +1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,
>> +1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,};
>
> this table is identical to ff_interleaved_golomb_vlc_len

I am using that table now.

>> +
>> +const uint8_t ff_interleaved_dirac_golomb_vlc_code[256]={
>> +0, 1, 0, 0, 2, 3, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 
>> +4, 5, 2, 2, 6, 7, 3, 3, 1, 1, 1, 1, 1, 1, 1, 1, 
>> +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
>> +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
>> +8, 9, 4, 4, 10,11,5, 5, 2, 2, 2, 2, 2, 2, 2, 2, 
>> +12,13,6, 6, 14,15,7, 7, 3, 3, 3, 3, 3, 3, 3, 3, 
>> +1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 
>> +1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 
>> +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
>> +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
>> +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
>> +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
>> +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
>> +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
>> +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
>> +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,};
>
> this looks suspiciously similar to ff_interleaved_ue_golomb_vlc_code
> remove_msb(a+1) = b

Yes, for golomb you need to substract one at the end.  I removed that
from the table, so the table can be used for multiple loops and
substract at the end.

> could you check if svq3_get_ue_golomb() does the same as 
> dirac_get_ue_golomb()?
> if no please check if its maybe just that svq3 code currently doenst support
> more than 32bit codewords
> if they do the same, a benchmark (with actual video data) of both would be
> interresting

This code is quite hard to change.  If I make it more generic, I am
afraid I will slow it down and make the benchmark useless.

Can you propose a way to fix this without slowing it down too much?  I
can benchmark it in that case.

> [...]
>> +        buf = GET_CACHE(re, gb);
>> +
>> +        buf >>= 32 - 8;
>
> buf= SHOW_UBITS(re, gb, 8);

changed.

>> +        LAST_SKIP_BITS(re, gb, FFMIN(ff_interleaved_dirac_golomb_vlc_len[buf], 8));
>> +
>> +        ret <<= (ff_interleaved_dirac_golomb_vlc_len[buf] - 1) >> 1;
>> +        ret |= ff_interleaved_dirac_golomb_vlc_code[buf];
>> +        if (ff_interleaved_dirac_golomb_vlc_len[buf] != 9)
>> +            break;
>
> this can be done more efficiently (see svq3_get_ue_golomb())
> also small values should be much more common than large ones so the
> loop would most of the time do just 1 iteration ...

Do you mean I should make a special condition for the case the loop
should not be entered and just return the value?  And in case more
than 8 bits should be read use the code I have now?

Or is there something with the code above?  In that case I do not get
it.  This works very different from svq3 so I will not be able to do
the same.

Thanks,
Marco