[FFmpeg-devel] [PATCH] Bink Video decoder for FFmpeg 0.6

[Vitor Sessak]

Kostya wrote:
> I've fixes some bugs and added some checks. This should be mature enough
> to include it into upcoming FFmpeg release.

Nice!

Some comments:

> P.S. Quant matrices have different quantizers so the biggest common
> denominator is 1/6 which is quite inconvenient to scale single matrix,
> thus whole 16 matrices instead.

I don't understand this comment (maybe because I'm not very experienced 
in video codecs), but I think it should be documented in a comment 
somewhere in the code.

> +static int decode_frame(AVCodecContext *avctx, void *data, int *data_size, AVPacket *pkt)
> +{
> +    BinkContext * const c = avctx->priv_data;
> +    GetBitContext gb;
> +    int blk, bw, bh;
> +    int i, j, plane, plane_idx, bx, by;
> +    uint8_t *dst, *prev, *ref, *ref_start, *ref_end;
> +    int v, col[2];
> +    const uint8_t *scan;
> +    const uint32_t *quant;
> +    int xoff, yoff;
> +    DECLARE_ALIGNED_16(DCTELEM, block[64]);
> +
> +    if(c->pic.data[0])
> +        avctx->release_buffer(avctx, &c->pic);
> +
> +    if(avctx->get_buffer(avctx, &c->pic) < 0){
> +        av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> +        return -1;
> +    }
> +
> +    init_get_bits(&gb, pkt->data, pkt->size*8);
> +    if (c->version >= 'i')
> +        skip_bits_long(&gb, 32);

Is a check for enough bits in the packet missing?

> +static av_cold int decode_init(AVCodecContext *avctx)
> +{
> +    BinkContext * const c = avctx->priv_data;
> +    static VLC_TYPE table[16 * 128][2];
> +    int i;
> +
> +    c->version = avctx->codec_tag >> 24;
> +    if (c->version < 'c') {
> +        av_log(avctx, AV_LOG_ERROR, "Too old version '%c'\n", c->version);
> +        return -1;
> +    }

av_log_missing_feature()?

> +    if (avcodec_check_dimensions(avctx, avctx->width, avctx->height) < 0) {
> +        return 1;
> +    }

This is AFAIK checked in common code already.

-Vitor