A FATE test please. Without it it's just asking to be silently broken without
anyone noticing for years.

On Tue, 14 May 2013 07:45:24 +0200, Kostya Shishkov <[email protected]> 
wrote:
> +
> +    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 ends prematurely.
> +        if (skip == -1)
> +            skip = decode_skip_count(&gb);

I think checking the return value and printing an error would make this much
more clear to less smart readers than you.

> +
> +        if (skip) {
> +            y[0] = old_y[0];
> +            y[1] = old_y[1];
> +            y[2] = old_y[old_y_stride];
> +            y[3] = old_y[old_y_stride + 1];
> +            y_avg = ya[0];
> +            cb = old_cb[0];
> +            cr = old_cr[0];
> +        } else {
> +            if (get_bits1(&gb)) {
> +                unsigned sign_selector       = get_bits(&gb, 6);
> +                unsigned difference_selector = get_bits(&gb, 2);
> +                y_avg = 2 * get_bits(&gb, 5);
> +                for (i = 0; i < 4; i++) {
> +                    y[i] = av_clip(y_avg + offset_table[difference_selector] 
> *
> +                                   sign_table[sign_selector][i], 0, 63);
> +                }
> +            } else if (get_bits1(&gb)) {
> +                if (get_bits1(&gb)) {
> +                    y_avg = get_bits(&gb, 6);
> +                } else {
> +                    unsigned adjust_index = get_bits(&gb, 3);
> +                    y_avg = (y_avg + luma_adjust[adjust_index]) & 63;
> +                }
> +                for (i = 0; i < 4; i++)
> +                    y[i] = y_avg;
> +            }
> +
> +            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);
> +                    cb = (cb + chroma_adjust[0][adjust_index]) & 31;
> +                    cr = (cr + chroma_adjust[1][adjust_index]) & 31;
> +                }
> +            }
> +        }
> +        *ya++ = y_avg;
> +
> +        new_y[0]                = y[0];
> +        new_y[1]                = y[1];
> +        new_y[new_y_stride]     = y[2];
> +        new_y[new_y_stride + 1] = y[3];
> +        *new_cb = cb;
> +        *new_cr = cr;
> +
> +        old_y += 2;
> +        old_cb++;
> +        old_cr++;
> +        new_y += 2;
> +        new_cb++;
> +        new_cr++;
> +        block_x++;
> +        if (block_x * 2 == avctx->width) {
> +            block_x = 0;
> +            old_y  += old_y_stride * 2  - avctx->width;
> +            old_cb += old_cb_stride     - avctx->width / 2;
> +            old_cr += old_cr_stride     - avctx->width / 2;
> +            new_y  += new_y_stride * 2  - avctx->width;
> +            new_cb += new_cb_stride     - avctx->width / 2;
> +            new_cr += new_cr_stride     - avctx->width / 2;
> +        }
> +
> +        skip--;
> +    }
> +
> +    new_y  = s->new_y;
> +    new_cb = s->new_u;
> +    new_cr = s->new_v;
> +    dstY   = pic->data[0];
> +    dstU   = pic->data[1];
> +    dstV   = pic->data[2];
> +    for (j = 0; j < avctx->height; j++) {
> +        for (i = 0; i < avctx->width; i++)
> +            dstY[i] = new_y[i] << 2;
> +        dstY  += pic->linesize[0];
> +        new_y += new_y_stride;
> +    }
> +    for (j = 0; j < avctx->height / 2; j++) {
> +        for (i = 0; i < avctx->width / 2; i++) {
> +            dstU[i] = chroma_vals[new_cb[i]];
> +            dstV[i] = chroma_vals[new_cr[i]];
> +        }
> +        dstU   += pic->linesize[1];
> +        dstV   += pic->linesize[2];
> +        new_cb += new_cb_stride;
> +        new_cr += new_cr_stride;
> +    }
> +
> +    av_dlog(avctx, "Frame data: provided %d bytes, used %d bytes\n",
> +            buf_size, get_bits_count(&gb) >> 3);
> +
> +    FFSWAP(uint8_t*, s->old_y, s->new_y);
> +    FFSWAP(uint8_t*, s->old_u, s->new_u);
> +    FFSWAP(uint8_t*, s->old_v, s->new_v);
> +
> +    *got_frame = 1;
> +
> +    return buf_size;
> +}
> +
> +

extra newline?

Otherwise looks simple enough to be ok.

-- 
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to