[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