[FFmpeg-devel] [PATCH][RFC] Indeo3 replacement

[Maxim]

Vitor Sessak schrieb:
> [...]
>

> A few comments:
>
>> //@{
>> //! vq table selector codes
>> #define DELTA_DYAD 0
>> #define DELTA_QUAD 1
>> #define RLE_ESC_F9 2
>> #define RLE_ESC_FA 3
>> #define RLE_ESC_FB 4
>> #define RLE_ESC_FC 5
>> #define RLE_ESC_FD 6
>> #define RLE_ESC_FE 7
>> #define RLE_ESC_FF 8
>> #define RLE_FORBIDDEN 9
>> //@}
>
> Does this group Doxy formatting works for you?

Ummh, I just looked into the generated doc and seen that these #defines
are documented separately! Therefore I'd say it doesn't do exactly what
I want to get - I just want to have in particular these defines grouped
and documented together like an enum. Is it possible to do it that way
or should I remove the group brackets?

>> /**
>> * Build decoder delta tables dynamically.
>> */
>> static av_cold void build_delta_tables(void)
>> {
>> int n, i, j, quads;
>> int32_t a, b, corr;
>> int32_t *delta_t, *quad_t, *delta_lo, *delta_hi;
>> const vqtEntry *rom_t;
>> const int8_t *delta_rom;
>> int8_t *sel_t;
>>
>> for (n = 0; n < 24; n++) {
>> rom_t = &vq_delta_rom[n];
>> delta_rom = rom_t->delta_tab;
>>
>> delta_t = &delta_tabs [n][0];
>> sel_t = &selector_tabs[n][0];
>> delta_lo = &delta_m10_lo [n][0];
>> delta_hi = &delta_m10_hi [n][0];
>>
>> /* set all code selectors = forbidden */
>> memset(sel_t, RLE_FORBIDDEN, 249);
>>
>> /* initialize selectors for RLE escape codes (0xF8...0xFF) */
>> sel_t[249] = RLE_ESC_F9;
>> sel_t[250] = RLE_ESC_FA;
>> sel_t[251] = RLE_ESC_FB;
>> sel_t[252] = RLE_ESC_FC;
>> sel_t[253] = RLE_ESC_FD;
>> sel_t[254] = RLE_ESC_FE;
>> sel_t[255] = RLE_ESC_FF;
>>
>> for (i = 0; i < rom_t->num_dyads; i++) {
>> /* get two vq deltas from the ROM table */
>> a = delta_rom[i*2];
>> b = delta_rom[i*2+1];
>> /* concatenate two vq deltas to form a two-pixel corrector (dyad) */
>> #ifdef WORDS_BIGENDIAN
>> corr = (a << 8) + b;
>> #else
>> corr = (b << 8) + a;
>> #endif
>> delta_t[i] = corr;
>
>
> Are you sure that this is correct? Later on, you do
>
>> prim_delta = &delta_tabs [prim_indx] [0];
>
> and
>
>> delta_tab = (line & 1) ? prim_delta : second_delta;
>
> and
>
>> src16[0] = ref16[0] + delta_tab[*data_ptr++];
>
> Wouldn't this result depends on endianness?

Yes, it heavily depends on endianness! That's why I put this
"byte-reverse" stuff in the table generation function. Let us proof it!

Assume some pixel data in memory, like that: 0x26, 0x26, 0x26, 0x26
Now we want to add a delta to two pixels at once in the "soft-SIMD"
fashion. Delta = -1, +3

On a little-endian machine:

prepare the corrector first:

corr = (b << 8) + a = (3 << 8) - 1 = 0x2FF

now add this delta to the first two pixels:

src16[0] = ref16[0] + corr = 0x2626 + 0x2FF = 0x2925

but after storing it in the memory we'll get: 0x25, 0x29, (0x26, 0x26)
because a little-endian machine reverses the byte order.

Now the same on a big-endian machine:

corr = (a << 8) + b = (-1 << 8) + 3 = 0xFFFFFF03

now add this delta to the first two pixels:

src16[0] = ref16[0] + corr = 0x2626 + 0xFFFFFF03 = 0x2529

but after storing it in the memory we'll get: 0x25, 0x29, (0x26, 0x26)
because a big-endian machine stores in the same byte order.

I just got a message from Peter Ross telling me that the code works on
PPC as well...

>> /**
>> * Decode a vector-quantized cell.
>> * It consists of several routines, each of which handles one or more
>> "modes"
>> * with which a cell can be encoded.
>
>
> [...]
>
>> switch (mode) {
>> case 0: /*------------------ MODES 0 & 1 (4x4 block processing)
>> --------------------*/
>> case 1:
>> skip_flag = 0;
>
> [... a lot of code ...]
>
>>
>> case 3: /*------------------ MODES 3 & 4 (4x8 block processing)
>> --------------------*/
>> case 4:
>
> Are you sure the code is not cleaner if ones puts the code of each
> case statement in a function?

I already took that into consideration. The hurdle for doing this was
just the fact that such a function will require min. 9 parameters (max.
12) and alot of variables needed to be declared several times! But I
could surely try to split these parts out...

Regards
Maxim