[FFmpeg-devel] [PATCH]Basic XSUB encoder (take 3)

[Reimar Döffinger]

On Thu, Feb 05, 2009 at 02:16:41AM +0100, Bj?rn Axelsson wrote:
> On Thu, 5 Feb 2009, Reimar D?ffinger wrote:
> > > +{
> > > +    if (len <= 255)
> > > +        put_bits(pb, 2 + ((ff_log2_tab[len] >> 1) << 2), len);
> > > +    else
> > > +        put_bits(pb, 14, 0);
> > > +    put_bits(pb, 2, color);
> >
> > If you add a
> > > if (!len) return;
> > you would not need most of the PADDING checks.
> 
> Well, there are only two checks and one is constant and can be optimized
> away by the compiler.

For speed, your checks are certainly better, just though it might look
nicer.

> > > +            // One run + padding will need 3 bytes in worst case
> > > +            if (bufsize - ((put_bits_count(&pb) + 7) >> 3) < 3)
> >
> > The worst case is 3 runs + padding.
> 
> I disagree.
> Worst case bit usage is 4 bits left padding (with PADDING <= 2), 16
> bits main run, 4 bits right padding and 4 bits for byte alignment.
> If left padding is needed, a new row begins byte-aligned and the 4 bits
> for alignment are not used, which gives 24 bits.
> If left padding is not needed, at most 20 bits will be encoded and with
> alignment that gives us also 24 bits at most.

Ok, so your value is right, but the comment is still confusing then.
I'd still be in favour of checking against < 7 so that changing the
value of PADDING can not create issues.
I was also wondering (in case anyone else is reading this), how about a
put_bits_free function that returns how many bits still fit into the
given context?

> > you can avoid this check by doing at the very beginning
> > > bufsize -= 27 + 7*2 + 4*3 + 2;
> > > if (bufsize <= 0) {
> > ...
> > and using
> > > xsub_encode_rle(&q, bufsize - (q-hdr),
> 
> Hmm. It can also be done without wasting the 2 bytes if not needed:
> bufsize -= 27 + 7*2 + 4*3 + 2*(h->rects[0]->h & 1);

Well, the main idea was to simplify, those are only 2 bytes that are
wasted during encoding, and most applications will just use one huge
buffer of e.g. 1 MB anyway, so it does not hurt if the estimates are too
large.

Greetings,
Reimar D?ffinger