[FFmpeg-devel] [PATCH 01/10] zlib decoder
Michael Niedermayer
michaelni
Tue Nov 27 19:34:37 CET 2007
On Sun, Nov 18, 2007 at 10:31:56PM +0000, M?ns Rullg?rd wrote:
>
> Here's an updated version of my zlib decoder with comments from
> various people taken into account.
[...]
> #define CONTAINER_RAW 0
> #define CONTAINER_ZLIB 1
> #define CONTAINER_GZIP 2
>
> #define DEFLATE_TYPE_NOCOMP 0
> #define DEFLATE_TYPE_FIXED 1
> #define DEFLATE_TYPE_DYNAMIC 2
> #define DEFLATE_TYPE_RESERVED 3
IMHO enums would be cleaner
[...]
> static unsigned int
> build_codes(unsigned int ncodes, uint8_t *clen, unsigned int *codes)
> {
> unsigned int next_code[16];
> unsigned int cl_count[16];
> unsigned int max_bits = 0;
> unsigned int max_code = 0;
> unsigned int code;
> unsigned int i;
>
> memset(cl_count, 0, sizeof(cl_count));
>
> for (i = 0; i < ncodes; i++) {
> cl_count[clen[i]]++;
> max_bits = FFMAX(max_bits, clen[i]);
> if (clen[i])
> max_code = i;
> }
>
> dprintf(ctx, "build_codes: ncodes=%d max_code=%d max_bits=%d\n",
> ncodes, max_code, max_bits);
>
> code = 0;
> cl_count[0] = 0;
>
> for (i = 1; i <= max_bits; i++) {
> code = (code + cl_count[i-1]) << 1;
> next_code[i] = code;
> }
>
> for (i = 0; i <= max_code; i++) {
> unsigned int len = clen[i];
> if (len) {
> codes[i] = rev_bits16(next_code[len]++, len);
> } else {
> codes[i] = 0;
> }
> }
>
> return max_code + 1;
> }
cant one of the following be used or one of them be replaced by the above?
ff_huff_build_tree()
ff_mjpeg_build_huffman_codes()
huff_build_tree()
or something be factored out ...
yes i hate the amount of vlc/huffman building code we already have ...
>
> static int
> build_vlc_fixed(AVInflateContext *ctx)
> {
> unsigned int ll_codes[288];
> uint8_t ll_len[288];
> unsigned int i;
>
> for (i = 0; i < 144; i++) {
> ll_codes[i] = rev_bits16(0x30 + i, 8);
> ll_len[i] = 8;
> }
> for (; i < 256; i++) {
> ll_codes[i] = rev_bits16(0x190 + i - 144, 9);
> ll_len[i] = 9;
> }
> for (; i < 280; i++) {
> ll_codes[i] = rev_bits16(i - 256, 7);
> ll_len[i] = 7;
> }
> for (; i < 288; i++) {
> ll_codes[i] = rev_bits16(0xc0 + i - 280, 8);
> ll_len[i] = 8;
> }
cant build_codes() be used to generate ll_codes ?
[...]
> #define decode_code_lens() \
> do { \
> ctx->i = 0; \
> while (ctx->i < ctx->hlit + 257 + ctx->hdist + 1) { \
> read_vlc(CLV, ctx->clv, &ctx->gb, cl_vlc); \
> \
> if (ctx->clv < 16) { \
> ctx->ll_len[ctx->i++] = ctx->clv; \
> } else if (ctx->clv < 19) { \
> ctx->clr = 0; \
> \
> if (ctx->clv == 16) { \
> read_bits(CLR1, ctx->clr, &ctx->gb, 2); \
> ctx->clr += 3; \
> ctx->clv = ctx->ll_len[ctx->i-1]; \
> } else if (ctx->clv == 17) { \
> read_bits(CLR2, ctx->clr, &ctx->gb, 3); \
> ctx->clr += 3; \
> ctx->clv = 0; \
> } else if (ctx->clv == 18) { \
the if() is unneeded
> read_bits(CLR3, ctx->clr, &ctx->gb, 7); \
> ctx->clr += 11; \
> ctx->clv = 0; \
> } \
> \
> while (ctx->clr--) { \
> ctx->ll_len[ctx->i++] = ctx->clv; \
> } \
this can overflow the array
> } else { \
> av_log(ctx, AV_LOG_ERROR, \
> "decode_code_lens: invalid code %d\n", ctx->clv); \
> goto err; \
> } \
> } \
> } while(0)
the bitstream for this can be maximally 280 bytes long in worst and quite
unrealistic case if i didnt make a mistake
so it can be implemented with a small buffer and without the need for the
switch case macro exit/restart machinery
>
> #define build_vlc_dynamic() \
> do { \
> check_bits(HLIT, &ctx->gb, 14); \
> ctx->hlit = get_bits(&ctx->gb, 5); \
> ctx->hdist = get_bits(&ctx->gb, 5); \
> ctx->hclen = get_bits(&ctx->gb, 4); \
> \
> dprintf(ctx, "hlit=%d hdist=%d hclen=%d\n", \
> ctx->hlit, ctx->hdist, ctx->hclen); \
> \
> for (ctx->i = 0; ctx->i < ctx->hclen + 4; ctx->i++) { \
> read_bits(CLLEN, ctx->cl_len[cl_perm[ctx->i]], &ctx->gb, 3); \
> } \
> for (; ctx->i < 19; ctx->i++) { \
> ctx->cl_len[cl_perm[ctx->i]] = 0; \
> } \
> \
> if (build_codelen_codes(ctx)) \
> goto err; \
> \
> decode_code_lens(); \
> \
> free_vlc(&ctx->cl_vlc); \
> \
> if (build_dynamic_codes(ctx)) \
> goto err; \
> } while(0)
this can only use up to 289 bytes, so it as well does not need the
switch case macro exit/restart machinery and could be implemented much
cleaner as a simple function which would return failure if it has not
enough data. It could then easily be restarted from 0 when more data
is available avoiding the mid exit/restart
the same is true for other parts
[...]
> read_vlc(CODE, ctx->code, &ctx->gb, ll_vlc);
> if (ctx->code < 256) {
> *out++ = ctx->code;
> } else if (ctx->code > 256 && ctx->code < 286) {
> ctx->code -= 257;
>
> ctx->len = len_tab[ctx->code][1] + 3;
> if (len_tab[ctx->code][0]) {
> read_bits(LENEXTRA, tmp, &ctx->gb,
> len_tab[ctx->code][0]);
> ctx->len += tmp;
> }
>
> if (ctx->btype == DEFLATE_TYPE_DYNAMIC) {
> read_vlc(DDIST, ctx->code, &ctx->gb, dist_vlc);
> } else {
> read_bits(FDIST, ctx->code, &ctx->gb, 5);
> ctx->code = ff_reverse[ctx->code] >> 3;
> }
>
> ctx->dist =
> (dist_tab[ctx->code][1] << dist_tab[ctx->code][0]) + 1;
> if (dist_tab[ctx->code][0]) {
> read_bits(DISTEXTRA, tmp, &ctx->gb,
> dist_tab[ctx->code][0]);
> ctx->dist += tmp;
> }
>
this also doesnt need any check exit restart in the middle
a single check at the end is enough
if the check indicates that not enough data was available then the
whole can be restarted when the next block has been appended to the buffer.
theres nothing gained by stoping and restarting in the middle of a code
not a single byte more could be output ...
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071127/3efe228e/attachment.pgp>
More information about the ffmpeg-devel
mailing list