[FFmpeg-devel] [PATCH] Escape 130 (RPL) WIP2

[Eli Friedman]

On Wed, Apr 2, 2008 at 4:41 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>  > +static int can_safely_read(GetBitContext* gb, int bits) {
>  > +    return get_bits_count(gb) + bits <= gb->size_in_bits;
>  > +}
>
>  duplicate

Would this be appropriate to put into bitstream.h?  How does code
outside the Escape codecs deal with overread?
>
>  > +
>  > +    av_log(NULL, AV_LOG_DEBUG,
>  > +           "Strides: %i, %i\n",
>  > +           new_y_stride, new_cb_stride);
>  > +
>  > +    for (block_index = 0; block_index < total_blocks; block_index++) {
>  > +        // Note that this call will make us skip the rest of the blocks
>  > +        // if the frame prematurely ends
>  > +        if (skip == -1)
>  > +            skip = decode_skip_count(&gb);
>  > +
>  > +        if (skip) {
>  > +            if (old_y) {
>  > +                y[0] = old_y[0] / 4;
>  > +                y[1] = old_y[1] / 4;
>  > +                y[2] = old_y[old_y_stride] / 4;
>  > +                y[3] = old_y[old_y_stride+1] / 4;
>  > +                cb = old_cb[0] / 8;
>  > +                cr = old_cr[0] / 8;
>  > +            } else {
>  > +                y[0] = y[1] = y[2] = y[3] = 0;
>  > +                cb = cr = 16;
>  > +            }
>
>  You could simplify this (and someother code) a little by allocating
>  a last_frame in addition to the current one initially.

Okay.

>
>  [...]
>  > +                    for (i = 0; i < 4; i++)
>  > +                        y[i] = av_clip(y[i] + adjust[adjust_index], 0, 63);
>  > +                }
>  > +            }
>  > +
>  > +            if (get_bits1(&gb)) {
>  > +                if (get_bits1(&gb)) {
>  > +                    cb = get_bits(&gb, 5);
>  > +                    cr = get_bits(&gb, 5);
>  > +                } else {
>  > +                    unsigned adjust_index = get_bits(&gb, 3);
>  > +                    static const int8_t adjust[2][8] =
>  > +                        {  { 1, 1, 0, -1, -1, -1,  0,  1 },
>  > +                           { 0, 1, 1,  0, -1, -2, -1, -1 } };
>  > +                    cb = (cb + adjust[0][adjust_index]) & 31;
>  > +                    cr = (cr + adjust[1][adjust_index]) & 31;
>
>  the chroma adjust differs from luma clip vs. wraparound is that intended?

I was experimenting because I had incomplete information.


I've just recently received some better information about this codec,
and I've managed to iron out most of the remaining bugs.  There are a
couple of things I need to think about a bit and restructure somewhat
before I'll have the final version ready, though.

-Eli