Hi heres the first part of the j2k review (ill review the rest of it ASAP but as the arithmetic coder is sufficiently independant theres no sense in delaying this)
aec.c:
[...]
> void ff_aec_init_contexts(AecState *aec)
> {
> memset(aec->contexts, 0, 19*sizeof(AecContext));
sizeof(aec->contexts) would be better
[...]
aec.h:
[...]
> const static AecCxState cx_states[47] = {
> {0x5601, 1, 1, 1},
> {0x3401, 2, 6, 0},
> {0x1801, 3, 9, 0},
> {0x0AC1, 4, 12, 0},
> {0x0521, 5, 29, 0},
> {0x0221, 38, 33, 0},
> {0x5601, 7, 6, 1},
> {0x5401, 8, 14, 0},
> {0x4801, 9, 14, 0},
> {0x3801, 10, 14, 0},
> {0x3001, 11, 17, 0},
> {0x2401, 12, 18, 0},
> {0x1C01, 13, 20, 0},
> {0x1601, 29, 21, 0},
> {0x5601, 15, 14, 1},
> {0x5401, 16, 14, 0},
> {0x5101, 17, 15, 0},
> {0x4801, 18, 16, 0},
> {0x3801, 19, 17, 0},
> {0x3401, 20, 18, 0},
> {0x3001, 21, 19, 0},
> {0x2801, 22, 19, 0},
> {0x2401, 23, 20, 0},
> {0x2201, 24, 21, 0},
> {0x1C01, 25, 22, 0},
> {0x1801, 26, 23, 0},
> {0x1601, 27, 24, 0},
> {0x1401, 28, 25, 0},
> {0x1201, 29, 26, 0},
> {0x1101, 30, 27, 0},
> {0x0AC1, 31, 28, 0},
> {0x09C1, 32, 29, 0},
> {0x08A1, 33, 30, 0},
> {0x0521, 34, 31, 0},
> {0x0441, 35, 32, 0},
> {0x02A1, 36, 33, 0},
> {0x0221, 37, 34, 0},
> {0x0141, 38, 35, 0},
> {0x0111, 39, 36, 0},
> {0x0085, 40, 37, 0},
> {0x0049, 41, 38, 0},
> {0x0025, 42, 39, 0},
> {0x0015, 43, 40, 0},
> {0x0009, 44, 41, 0},
> {0x0005, 45, 42, 0},
> {0x0001, 45, 43, 0},
> {0x5601, 46, 46, 0}
> };
the array should be in aec.c
>
> typedef struct {
> unsigned int state;
> unsigned int mps;
mps and state can be merged so that mps is the least signifcant bit
see cabac.c/h if you dont know what i mean
[...]
> /**
> * number of encoded bytes
> */
> int ff_aec_length(AecState *aec);
>
> /**
> * flush the encoder [returns number of bytes encoded]
> * */
* */ vs. */ is inconsistant
> int ff_aec_flush(AecState *aec);
>
> /** decoder */
>
> void ff_aec_initdec(AecState *aec, uint8_t *bp);
doxygen comment isnt correct for the function
[...]
aecdec.c:
[...]
> /**
> * Arithmetic entropy decoder
> * @file aecdec.c
the aec* names are a little generic, we already have an arithmetic coder
in lavc maybe you could give the files more specific names
[...]
> do{
> if (!aec->ct)
> bytein(aec);
> aec->a = aec->a << 1;
> aec->c = aec->c << 1;
aec->c += aec->c;
> aec->ct--;
i think it would be possible to put a marker bit in aec->c so that
the ct-- could be avoided and instead a check like
if(!(aec->c & 0xFF))
bytein(aec);
could be used
this would avoid reading, writing and increasng the ct field of the struct
the same trick might also be possible on the encoder side
[...]
> int ff_aec_decode(AecState *aec, int cx)
> {
> aec->curctx = aec->contexts + cx;
this is likely inefficient, you could just pass a pointer to the current
context as argument to ff_aec_decode and any other (likely inlined)
functions which need it
[...]
aecenc.c:
[...]
> void ff_aec_encode(AecState *aec, int cx, int d)
> {
> int qe;
>
> aec->curctx = aec->contexts + cx;
as with the decoder, storing the current ctx in the struct seems inefficient
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
signature.asc
Description: Digital signature
_______________________________________________ FFmpeg-soc mailing list [email protected] http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
