[FFmpeg-devel] zlib decoder

[Måns Rullgård]

Reimar Doeffinger wrote:
> Hello,
> On Mon, Jul 02, 2007 at 10:30:16PM +0100, M?ns Rullg?rd wrote:
>> Here, at long last, is my highly anticipated zlib decoder.
>
> Yay!

I was wondering when you'd notice.

>> static void
>> copy_bytes(uint8_t *dst, uint8_t *src, unsigned int len)
>> {
>>     while (len--)
>>         *dst++ = *src++;
>> }
>>
>> static int
>> copy_offset(AVInflateContext *ctx, uint8_t *p, uint8_t *start, uint8_t *end,
>>             unsigned int offset, unsigned int len)
>> {
>>     unsigned int outlen = FFMIN(len, end - p);
>
> looks quite similar to the LZO code which is benchmarked a bit on at
> least x86, maybe you can reuse some parts of it?

That could well be.  I didn't think of checking.

>>     bitpos = get_bits_count(&ctx->gb);
>>     ctx->skip = bitpos & 7;
>>     bytepos = bitpos / 8;
>>
>>     if (needbits || ctx->skip) {
>>         unsigned isize;
>>         uint8_t *in;
>>
>>         if (tailbits) {
>>            isize = (tailbits - bitpos + 7) / 8;
>
> if (tailbits > bitpos) seems safer to me, even if it should be true
> anyway (didn't check so closely).
> I also can't help but wonder: is that tail stuff only for really
> handling gzip streams?
> Do we actually have those and not only gzip blocks where we wouldn't
> need such extra handling??

There is nothing special about gzip.  A block can always be arbitrarily
long without anything necessarily being byte-aligned.

>>    /* TODO: don't copy to buffer if last byte has been written */
>>    if (*outsize >= sizeof(ctx->buf)) {
>>        ctx->bufsize = sizeof(ctx->buf);
>>        memcpy(ctx->buf, out - ctx->bufsize, ctx->bufsize);
>>    } else if (ctx->bufsize + *outsize > sizeof(ctx->buf)) {
>>        unsigned drop = ctx->bufsize + *outsize - sizeof(ctx->buf);
>>        unsigned bsize = ctx->bufsize - drop;
>>        memmove(ctx->buf, ctx->buf + drop, bsize);
>>        memcpy(ctx->buf + bsize, outbuf, *outsize);
>>        ctx->bufsize = sizeof(ctx->buf);
>>    } else {
>>        memcpy(ctx->buf + ctx->bufsize, outbuf, *outsize);
>>        ctx->bufsize += *outsize;
>>    }
>
> If I didn't mess up the following seems simple (although potentially
> slower):
>
>> int cpsize = FFMIN(*outsize, sizeof(ctx->buf));
>> int drop = ctx->bufsize + cpsize - sizeof(ctx->buf);
>> if (drop > 0) {
>>   ctx->bufsize -= drop;
>>   memmove(ctx->buf, ctx->buf + drop, ctx->bufsize);
>> }
>> memcpy(ctx->buf + ctx->bufsize, outbuf, cpsize);
>> ctx->bufsize += cpsize;

Hmm... that probably does the same thing.  I'm not sure what memmove()
does if source and destination are the same, so it might save some
useless work if we check for that.

> And maybe extending and using using fifo.h is possible and acceptable
> speed-wise?

A FIFO isn't at all what we need.  I thought about making it a proper
ring buffer instead, but I figured it was better to get it working at
all first.  Perhaps some generic functions for managing ring buffers
would be useful.

-- 
M?ns Rullg?rd
mans at mansr.com