[FFmpeg-devel] [PATCH] Indeo5 decoder

Michael Niedermayer michaelni
Tue Apr 7 05:17:06 CEST 2009


On Mon, Apr 06, 2009 at 08:41:57PM +0200, Maxim wrote:
> Michael Niedermayer schrieb:
> > On Wed, Apr 01, 2009 at 02:43:04AM +0200, Maxim wrote:
> >   
> >> Diego Biurrun schrieb:
> >>     
> >>> Have you checked this works standalone, i.e. there are no other
> >>> dependencies?
> >>>
> >>> Try disabling all decoders and encoders and just enabling indeo5 please.
> >>>   
> >>>       
> >> Yes, it does...
> >>
> >> Attached is an updated patch that fixes all things you pointed to.
> >> The following things were changed as well:
> >> - the inverse transform contains algebraic simplifications showed by
> >> Michael in his earlier post
> >> - a step was added to the inverse transform that tests for col/rows
> >> containing zero coeffs and sets the corresponding output to zero instead
> >> of performing the inverse transform on zeroes
> >> - simplifications of the motion compensation functions
> >>
> >> Please any further reviews (Michael?)
> >>     
> 
> An updated patch attached fixing the most things showed by patcheck...
> 
[...]

> +/** static dequant tables (initialized at startup) */
> +static uint16_t     deq8x8_intra[5][24][64]; ///< dequant tables for intra blocks 8x8
> +static uint16_t     deq8x8_inter[5][24][64]; ///< dequant tables for inter blocks 8x8
> +static uint16_t     deq4x4_intra[24][16];    ///< dequant tables for intra blocks 4x4
> +static uint16_t     deq4x4_inter[24][16];    ///< dequant tables for inter blocks 4x4

this isnt the correct doxy syntax to document several variables IIRC


> +
> +
> +/**
> + *  Build static indeo5 dequantization tables.
> + */
> +static av_cold void build_dequant_tables(void)
> +{
> +    int         mat, i, lev;
> +    uint32_t    q1, q2, sf1, sf2;
> +
> +    for (mat = 0; mat < 5; mat++) {
> +        /* build 8x8 intra/inter tables for all 24 quant levels */
> +        for (lev = 0; lev < 24; lev++) {
> +            sf1 = ivi5_scale_quant_8x8_intra[mat][lev];
> +            sf2 = ivi5_scale_quant_8x8_inter[mat][lev];
> +
> +            for (i = 0; i < 64; i++) {
> +                q1 = (ivi5_base_quant_8x8_intra[mat][i] * sf1) >> 8;
> +                q2 = (ivi5_base_quant_8x8_inter[mat][i] * sf2) >> 8;
> +                deq8x8_intra[mat][lev][i] = av_clip(q1, 1, 255);
> +                deq8x8_inter[mat][lev][i] = av_clip(q2, 1, 255);

1..255 but they arent uint8_t 
av_clip() seems useless  and the whole table precalc maybe as well


> +            }
> +        }
> +    } // for mat
> +
> +    /* build 4x4 intra/inter tables for all 24 quant levels */
> +    for (lev = 0; lev < 24; lev++) {
> +        sf1 = ivi5_scale_quant_4x4_intra[lev];
> +        sf2 = ivi5_scale_quant_4x4_inter[lev];
> +
> +        for (i = 0; i < 16; i++) {
> +            q1 = (ivi5_base_quant_4x4_intra[i] * sf1) >> 8;
> +            q2 = (ivi5_base_quant_4x4_inter[i] * sf2) >> 8;
> +            deq4x4_intra[lev][i] = av_clip(q1, 1, 255);
> +            deq4x4_inter[lev][i] = av_clip(q2, 1, 255);
> +        }
> +    }

probably same


> +}
> +
> +
> +/**
> + *  Decode indeo5 GOP (Group of pictures) header.
> + *  This header is present in key frames only.
> + *  It defines parameters for all frames in a GOP.
> + *
> + *  @param ctx      [in,out] ptr to the decoder context
> + *  @param avctx    [in] ptr to the AVCodecContext
> + *  @return         result code: 0 = OK, -1 = error
> + */
> +static int decode_gop_header(IVI5DecContext *ctx, AVCodecContext *avctx)
> +{
> +    int             result, i, p, tile_size, pic_size_indx, mb_size, blk_size, blk_size_changed = 0;
> +    IVIBandDesc     *band, *band1, *band2;
> +    IVIPicConfig    pic_conf;
> +
> +    ctx->gop_flags = get_bits(&ctx->gb, 8);
> +
> +    /* get size of the GOP header if present */
> +    ctx->gop_hdr_size = (ctx->gop_flags & 1) ? get_bits(&ctx->gb, 16) : 0;
> +

> +    /* get 32-bit lock word in the case of password protected video */
> +    ctx->is_protected = !!(ctx->gop_flags & 0x20);

!! is useless
is_protected itself is probably redundant

[...]
> +/**
> + *  Decode indeo5 band header.
> + *
> + *  @param ctx      [in,out] ptr to the decoder context
> + *  @param band     [in,out] ptr to the band descriptor
> + *  @param avctx    [in] ptr to the AVCodecContext
> + *  @return         result code: 0 = OK, -1 = error
> + */
> +static int decode_band_hdr(IVI5DecContext *ctx, IVIBandDesc *band, AVCodecContext *avctx)
> +{
> +    int         i, result;
> +    uint8_t     band_flags;
> +    uint32_t    band_data_size;
> +    IVIHuffDesc new_huff;
> +

> +    band_flags = get_bits(&ctx->gb, 8);
> +
> +    if (band_flags & 1) {
> +        band->is_empty = 1; /* ok this band is empty (there is no coded data) */
> +        return 0;
> +    }
> +
> +    band_data_size = (ctx->frame_flags & 0x80) ? get_bits_long(&ctx->gb, 24) : 0;
> +
> +    band->inherit_mv     = !!(band_flags & 2);
> +    band->inherit_qdelta = !!(band_flags & 8);
> +    band->qdelta_present = !!(band_flags & 4);

these looks like a messy way to read a few flags

[...]
> +/**
> + *  Decode info (block type, cbp, quant delta, motion vector)
> + *  for all macroblocks in the current tile.
> + *
> + *  @param ctx      [in,out] ptr to the decoder context
> + *  @param band     [in,out] ptr to the band descriptor
> + *  @param tile     [in,out] ptr to the tile descriptor
> + *  @param avctx    [in] ptr to the AVCodecContext
> + *  @return         result code: 0 = OK, -1 = error
> + */
> +static int decode_mb_info(IVI5DecContext *ctx, IVIBandDesc *band, IVITile *tile,
> +                          AVCodecContext *avctx)
> +{
> +    int         x, y, mv_x, mv_y, mv_delta, offs, mb_offset, mv_scale, blks_per_mb;
> +    IVIMbInfo   *mb, *ref_mb;
> +    int         row_offset = band->mb_size * band->pitch;
> +
> +    mb     = tile->mbs;
> +    ref_mb = tile->ref_mbs;
> +    offs   = tile->ypos * band->pitch + tile->xpos;
> +
> +    /* factor for motion vector scaling */
> +    mv_scale = (ctx->planes[0].bands[0].mb_size >> 3) - (band->mb_size >> 3);
> +    mv_x = mv_y = 0;
> +
> +    for (y = tile->ypos; y < (tile->ypos + tile->height); y += band->mb_size) {
> +        mb_offset = offs;
> +
> +        for (x = tile->xpos; x < (tile->xpos + tile->width); x += band->mb_size) {
> +            mb->xpos     = x;
> +            mb->ypos     = y;
> +            mb->buf_offs = mb_offset;
> +
> +            if (get_bits1(&ctx->gb)) {
> +                if (ctx->frame_type == IVI5_FRAMETYPE_INTRA) {
> +                    av_log(avctx, AV_LOG_ERROR, "Empty macroblock in an INTRA picture!\n");
> +                    return -1;
> +                }
> +                mb->type = 1; /* empty macroblocks are always INTER */
> +                mb->cbp  = 0; /* all blocks are empty */
> +
> +                mb->q_delta = 0;
> +                if (!band->plane && !band->band_num && (ctx->frame_flags & 8)) {
> +                    mb->q_delta = get_vlc2(&ctx->gb, ctx->mb_vlc->table, IVI_VLC_BITS, 1);
> +                    mb->q_delta = IVI_TOSIGNED(mb->q_delta);
> +                }
> +
> +                mb->mv_x = mb->mv_y = 0; /* no motion vector coded */
> +                if (band->inherit_mv){
> +                    /* motion vector inheritance */
> +                    switch (mv_scale) {
> +                        case 0:
> +                        case 1:
> +                            av_log(avctx, AV_LOG_ERROR, "Unsupported MV scaling!\n");
> +                            return -1;
> +                        case 2:
> +                            mb->mv_x = IVI_MV_DIV4(ref_mb->mv_x);
> +                            mb->mv_y = IVI_MV_DIV4(ref_mb->mv_y);
> +                            break;
> +                    }

as mv_scale doesnt change inside the loop theres no point in doing this
check inside the loop

[...]
> +/**
> + *  Switch buffers.
> + *
> + *  @param ctx      [in,out] ptr to the decoder context
> + *  @param avctx    [in] ptr to the AVCodecContext
> + */
> +static void switch_buffers(IVI5DecContext *ctx, AVCodecContext *avctx)
> +{
> +    int             p;
> +    IVIPlaneDesc    *plane;
> +
> +    for (p = 0; p < 3; p++) {
> +        plane = &ctx->planes[p];
> +
> +        switch (ctx->frame_type) {
> +            case IVI5_FRAMETYPE_INTRA:
> +                plane->buf_switch = 0;
> +                break;
> +            case 1:

> +                if (ctx->prev_frame_type != 3)

please use named constants not literal numbers



> +                    plane->buf_switch ^= 1; /* swap buffers only if there is no frame of the type 3 */
> +                break;
> +            case 3:
> +                plane->buf_switch ^= 1;
> +                break;
> +            case IVI5_FRAMETYPE_NULL:
> +                return;
> +            default:
> +                av_log(avctx, AV_LOG_ERROR, "unsupported frame type: %d\n", ctx->frame_type);
> +        }
> +
> +        if (plane->num_bands == 1) {

> +            plane->bands[0].buf = (plane->buf_switch) ? plane->buf2 : plane->buf1;
> +            plane->bands[0].ref_buf = (plane->buf_switch) ? plane->buf1 : plane->buf2;

vertical align

[...]
> +/** butterfly operation for the inverse slant transform */
> +#define IVI_SLANT_BFLY(x, y) t1 = x-y; x += y; y = t1;
> +
> +/** This is a reflection a,b = 1/2, 5/4 for the inverse slant transform */
> +#define IVI_IREFLECT(s1, s2) {\
> +    t1 = s1 + ((s1 + s2*2 + 2) >> 2);\
> +    s2 = ((s1*2 - s2 + 2) >> 2) - s2;\
> +    s1 = t1;}
> +
> +/** This is a reflection a,b = 1/2, 7/8 for the inverse slant transform */
> +#define IVI_SLANT_PART4(s1, s2) {\
> +    t1 = s2 + ((s1*4 - s2 + 4)  >> 3);\
> +    s2 = ((-s1 - s2*4 + 4) >> 3) + s1;\
> +    s1 = t1;}
> +
> +/** inverse slant8 transform */
> +#define IVI_INV_SLANT8(s1, s4, s8, s5, s2, s6, s3, s7, d1, d2, d3, d4, d5, d6, d7, d8) {\
> +    IVI_SLANT_PART4(s4, s5);\
> +    IVI_SLANT_BFLY(s1, s5); IVI_SLANT_BFLY(s2, s6); IVI_SLANT_BFLY(s7, s3); IVI_SLANT_BFLY(s4, s8);\
> +    IVI_SLANT_BFLY(s1, s2); IVI_IREFLECT  (s4, s3); IVI_SLANT_BFLY(s5, s6); IVI_IREFLECT  (s8, s7);\
> +    IVI_SLANT_BFLY(s1, s4); IVI_SLANT_BFLY(s2, s3); IVI_SLANT_BFLY(s5, s8); IVI_SLANT_BFLY(s6, s7);\
> +    d1 = COMPENSATE(s1);\
> +    d2 = COMPENSATE(s2);\
> +    d3 = COMPENSATE(s3);\
> +    d4 = COMPENSATE(s4);\
> +    d5 = COMPENSATE(s5);\
> +    d6 = COMPENSATE(s6);\
> +    d7 = COMPENSATE(s7);\
> +    d8 = COMPENSATE(s8);}
> +
> +/** inverse slant4 transform */
> +#define IVI_INV_SLANT4(s1, s4, s2, s3, d1, d2, d3, d4) {\
> +    IVI_SLANT_BFLY(s1, s2); IVI_IREFLECT  (s4, s3);\
> +    IVI_SLANT_BFLY(s1, s4); IVI_SLANT_BFLY(s2, s3);\
> +    d1 = COMPENSATE(s1);\
> +    d2 = COMPENSATE(s2);\
> +    d3 = COMPENSATE(s3);\
> +    d4 = COMPENSATE(s4);}
> +
> +
> +/**
> + *  Two-dimensional inverse slant 8x8 transform.
> + *
> + *  @param  in      [in]  pointer to the vector of transform coefficients
> + *  @param  out     [out] pointer to the output buffer (frame)
> + *  @param  pitch   [in]  pitch to move to the next y line
> + *  @param  flags   [in]  pointer to the array of column flags:
> + *                        1 - non_empty column, 0 - empty one
> + */
> +void ff_ivi_inverse_slant_8x8(int32_t *in, int16_t *out, uint32_t pitch, uint8_t *flags)

the transform should be in a seperate file

[...]
> +/**
> + *  8x8 block motion compensation with adding delta.
> + *
> + *  @param  buf     [in,out] pointer to the block in the current frame buffer containing delta
> + *  @param  ref_buf [in]     pointer to the corresponding block in the reference frame
> + *  @param  pitch   [in]     pitch for moving to the next y line
> + *  @param  mc_type [in]     interpolation type
> + */
> +static void mc_8x8_delta(int16_t *buf, int16_t *ref_buf, uint32_t pitch, int mc_type)
> +{
> +    int     i, j;
> +    int16_t *wptr;
> +
> +    switch (mc_type) {
> +        case 0: /* fullpel (no interpolation) */
> +            for (i = 0; i < 8; i++, buf += pitch, ref_buf += pitch) {
> +                buf[0] += ref_buf[0];
> +                buf[1] += ref_buf[1];
> +                buf[2] += ref_buf[2];
> +                buf[3] += ref_buf[3];
> +                buf[4] += ref_buf[4];
> +                buf[5] += ref_buf[5];
> +                buf[6] += ref_buf[6];
> +                buf[7] += ref_buf[7];
> +            }
> +            break;
> +        case 1: /* horizontal halfpel interpolation */
> +            for (i = 0; i < 8; i++, buf += pitch, ref_buf += pitch) {
> +                for (j = 0; j < 8; j++)
> +                    buf[j] += (ref_buf[j] + ref_buf[j+1]) >> 1;
> +            }
> +            break;
> +        case 2: /* vertical halfpel interpolation */
> +            wptr = ref_buf + pitch;
> +            for (i = 0; i < 8; i++, buf += pitch, wptr += pitch, ref_buf += pitch) {
> +                for (j = 0; j < 8; j++)
> +                    buf[j] += (ref_buf[j] + wptr[j]) >> 1;
> +            }
> +            break;
> +        case 3: /* vertical and horizontal halfpel interpolation */
> +            wptr = ref_buf + pitch;
> +            for (i = 0; i < 8; i++, buf += pitch, wptr += pitch, ref_buf += pitch) {
> +                for (j = 0; j < 8; j++)
> +                    buf[j] += (ref_buf[j] + ref_buf[j+1] + wptr[j] + wptr[j+1]) >> 2;
> +            }
> +            break;
> +    }
> +}

so should MC


[...]
> +/**
> + *  Convert and output the current plane.
> + *  This conversion is done by adding back the bias value of 128
> + *  (subtracted in the encoder) and clipping the result.
> + *
> + *  @param plane        [in]  pointer to the descriptor of the plane being processed
> + *  @param dst          [out] pointer to the buffer receiving converted pixels
> + *  @param dst_pitch    [in]  pitch for moving to the next y line
> + */
> +void ff_ivi_output_plane(IVIPlaneDesc *plane, uint8_t *dst, int dst_pitch)
> +{
> +    int             x, y;
> +    const int16_t   *src = (plane->buf_switch) ? plane->buf2 : plane->buf1;
> +
> +    for (y = 0; y < plane->height; y++) {
> +        for (x = 0; x < plane->width; x++)
> +            dst[x] = av_clip(src[x] + 128, 0, 255);

we have a specific function for cliping to 8bit


[...]
> +int  ff_ivi_create_huff_from_desc(const IVIHuffDesc *cb, VLC *pOut, int flag);
> +int  ff_ivi_dec_huff_desc(GetBitContext *gb, IVIHuffDesc *desc);
> +int  ff_ivi_huff_desc_cmp(const IVIHuffDesc *desc1, const IVIHuffDesc *desc2);
> +void ff_ivi_huff_desc_copy(IVIHuffDesc *dst, const IVIHuffDesc *src);
> +int  ff_ivi_init_planes(IVIPlaneDesc *planes, const IVIPicConfig *cfg);
> +void ff_ivi_free_buffers(IVIPlaneDesc *planes);
> +int  ff_ivi_init_tiles(IVIPlaneDesc *planes, const int tile_width, const int tile_height);
> +int  ff_ivi_dec_tile_data_size(GetBitContext *gb);
> +void ff_ivi_inverse_slant_8x8(int32_t *in, int16_t *out, uint32_t pitch, uint8_t *flags);
> +void ff_ivi_inverse_slant_4x4(int32_t *in, int16_t *out, uint32_t pitch, uint8_t *flags);
> +void ff_ivi_dc_slant_2d(int32_t *in, int16_t *out, uint32_t pitch, int blk_size);
> +int  ff_ivi_decode_blocks(GetBitContext *gb, IVIBandDesc *band, IVITile *tile);
> +void ff_ivi_process_empty_tile(AVCodecContext *avctx, IVIBandDesc *band, IVITile *tile, int32_t mv_scale);
> +void ff_ivi_output_plane(IVIPlaneDesc *plane, uint8_t *dst, int dst_pitch);

doxy should be in the headers not the C files (easier to find there)

[...]
> +/**
> + *  standard picture dimensions
> + */
> +struct {
> +    uint16_t    width;
> +    uint16_t    height;
> +} ivi5_common_pic_sizes[15] = {{640, 480}, {320, 240}, {160, 120}, {704, 480}, {352, 240}, {352, 288}, {176, 144},
> +                               {240, 180}, {640, 240}, {704, 240}, {80,  60},  {88,  72},  {0,   0},   {0,   0}, {0, 0}};

you dont need a struct for this

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

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- 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/20090407/f0ac18b5/attachment.pgp>



More information about the ffmpeg-devel mailing list