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

[Michael Niedermayer]

On Mon, Mar 31, 2008 at 10:39:23PM -0700, Eli Friedman wrote:
> Per subject, second work-in-progress revision of Escape 130.  I've
> fixed a few bugs and cleaned it up quite a bit.  The decoder is much
> closer to working correctly, but there are still a few issues that I
> haven't tracked down.

quick review ...


[...]
> +static int can_safely_read(GetBitContext* gb, int bits) {
> +    return get_bits_count(gb) + bits <= gb->size_in_bits;
> +}

duplicate


[...]
> +    new_frame.reference = 3;
> +    if (avctx->get_buffer(avctx, &new_frame)) {
> +        av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> +        return -1;
> +    }

> +    

trailing whitespace


> +    new_y = new_frame.data[0];
> +    new_cb = new_frame.data[1];
> +    new_cr = new_frame.data[2];
> +    new_y_stride = new_frame.linesize[0];
> +    new_cb_stride = new_frame.linesize[1];
> +    new_cr_stride = new_frame.linesize[2];
> +    old_y = s->frame.data[0];
> +    old_cb = s->frame.data[1];
> +    old_cr = s->frame.data[2];
> +    old_y_stride = s->frame.linesize[0];
> +    old_cb_stride = s->frame.linesize[1];
> +    old_cr_stride = s->frame.linesize[2];

vertical align


> +
> +    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.


[...]
> +                    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?


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- 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/20080403/fca29b97/attachment.pgp>