[FFmpeg-devel] [PATCH] Indeo5 decoder

[Michael Niedermayer]

On Tue, Apr 07, 2009 at 05:08:34PM +0200, Maxim wrote:
> Michael Niedermayer schrieb:
> > On Tue, Apr 07, 2009 at 10:52:34AM +0200, Maxim wrote:
> >   
> >> Michael Niedermayer schrieb:
> >>     
> >>> On Mon, Apr 06, 2009 at 08:41:57PM +0200, Maxim wrote:
> >>>       
> > [...]
> >   
> >>>> +
> >>>> +
> >>>> +/**
> >>>> + *  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
> >>>   
> >>>       
> >> They were made uint16_t in order to achieve a compatibility with Indeo4
> >> that uses 9bits dequant tables...
> >> The table precalculation should help avoiding huge static tables...
> >>     
> >
> > let me clarify my question, what is gained by merging a multiply and shift
> > into the table?
> > is it faster? if so then by how much?
> >   
> 
> It provides a possibility to calculate the scalefactors using integer
> operations without floatig-point math and it's therefore faster...

moving the operations from one function to another does not change
anything related to float vs int.
My question is just why you create a bigger table instead of doing it during
block decode?
Maybe its faster, but you did not say this and its definitly more messy
as is.


[...]
> >
> >   
> >> [...]
> >>
> >>     
> >>>> +/**
> >>>> + *  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
> >>>   
> >>>       
> >> I'm sorry, how this comment should be interpreted?
> >>     
> >
> > like the previous you snipped, that is that this belongs in a seperate file.
> >   
> 
> Aha, ok. Should I move them to "dsputils"?

maybe, iam not sure

but they definitly should be in a seperate file ...

[...]

-- 
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/20090407/e9f35df5/attachment.pgp>