[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 ...
> if you want your name in the file, i think maybe something like
>
> /**
>  * this function finds the foobar.
>  * @author (copyright) Marco Gerards <marco at gnu.org>
>
> above functions you wrote, would be a compromise

No, there is no need for that.  I just added it because I thought it
was the right thing to do.  Feel free to remove my name from this patch.

>> +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

oh, ok :)

>> +
>> +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
>
> could you check if svq3_get_ue_golomb() does the same as 
> dirac_get_ue_golomb()?

Before I wrote this code, I did this.  They are not the same.

> if no please check if its maybe just that svq3 code currently doenst support
> more than 32bit codewords

At least it worked up to a certain length.  The value 100000 couldn't
be read using svq3_get_ue_golomb, IIRC.  So this would be the main
reason it doesn't work for me, most likely the only reason.

> if they do the same, a benchmark (with actual video data) of both would be
> interresting

Sure.

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

ok

>> +        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 ...

Right.

After testing this a bit more, I will get back to you on this.

Thanks,
Marco