[Ffmpeg-devel] [PATCH] DTS decoder

Michael Niedermayer michaelni
Fri Feb 23 20:14:23 CET 2007


Hi

On Fri, Feb 23, 2007 at 06:33:08PM +0200, Kostya wrote:
> New revision.
> Notes:
> * INIT_VLC_BITALLOC:
>   I understand this may be not the best solution but let's look
>   on these tables structure:
>     1 table with 3 elements
>     sets of 3 tables with 5,7,9 and 13 elements per table in each set
>     sets of 7 tables with 17,25,33,65,129 elements per table in each set
>   so it would be a waste of space to put them in one 3-dimensional array
>   and another solution is to give each 1-d array its own name and
>   create another structure to reference them all - that's quite messy too

it is less messy then duplicating the init code _10_ times with the 
preprocessor also you can turn the macro into a function with no changes
to the tables


[...]

> +#define DCA_CHANNEL_MAX  DCA_3F2R       /* We don't handle anything above that */

unused


[...]

> +#define DCA_ADJUST_LEVEL 0x100
> +
> +#define BUFFER_SIZE 24576

unused


[...]
> +static int dca_parse_frame_header_mini(DCAContext * s)
> +{
> +    int sample_rate_index;
> +    int bit_rate_index;
> +    /* Sync code */
> +    get_bits(&s->gb, 32);
> +    s->frame_type = get_bits(&s->gb, 1);
> +    s->samples_deficit = get_bits(&s->gb, 5);
> +    s->crc_present = get_bits(&s->gb, 1);
> +    s->frame_length = (get_bits(&s->gb, 7) + 1) * 32;
> +    s->frame_size = get_bits(&s->gb, 14) + 1;
> +

this is duplicated, and slightly differently:
---
> +    s->frame_type = get_bits(&s->gb, 1);
> +    s->samples_deficit = get_bits(&s->gb, 5) + 1;
> +    s->crc_present = get_bits(&s->gb, 1);
> +    s->sample_blocks = get_bits(&s->gb, 7) + 1;
> +    s->frame_size = get_bits(&s->gb, 14) + 1;
> +    s->amode = get_bits(&s->gb, 6);
> +    s->sample_rate = get_bits(&s->gb, 4);
> +    s->bit_rate = get_bits(&s->gb, 5);
---



> +    if ((s->frame_size <= 0) || (s->frame_size > DCA_MAX_FRAME_SIZE))
> +        return 0;

how is s->frame_size <= 0 supposed to be possible?


> +
> +    /* Audio channel arrangement */
> +    s->flags = get_bits(&s->gb, 6);
> +    sample_rate_index = get_bits(&s->gb, 4);
> +    s->sample_rate = dca_sample_rates[sample_rate_index];
> +    if (!s->sample_rate)
> +        return 0;
> +
> +    bit_rate_index = get_bits(&s->gb, 5);
> +    s->bit_rate = dca_bit_rates[bit_rate_index];
> +    if (!s->bit_rate)
> +        return 0;

error return should be -1 for consistency with the rest of lav*


[...]

> +    s->ext_descr = get_bits(&s->gb, 3);
> +    s->ext_coding = get_bits(&s->gb, 1);

these are never used why are they put into the context? are they needed
for some not yet implemented features?
the same is true for several other variables 



[...]
> +    for (i = 0; i < s->prim_channels; i++) {
> +        s->subband_activity[i] = get_bits(&s->gb, 5) + 2;
> +        if (s->subband_activity[i] > DCA_SUBBANDS)
> +            s->subband_activity[i] = DCA_SUBBANDS;

int tmp= get_bits(&s->gb, 5) + 2;
s->subband_activity[i]= FFMIN(tmp, DCA_SUBBANDS);
maybe? but iam fine with the current code too


[...]
> +    for (i = 0; i < s->prim_channels; i++) {
> +        s->joint_intensity[i] = get_bits(&s->gb, 3);
> +    }
> +    for (i = 0; i < s->prim_channels; i++) {
> +        s->transient_huffman[i] = get_bits(&s->gb, 2);
> +    }
> +    for (i = 0; i < s->prim_channels; i++) {
> +        s->scalefactor_huffman[i] = get_bits(&s->gb, 3);
> +    }
> +    for (i = 0; i < s->prim_channels; i++) {
> +        s->bitalloc_huffman[i] = get_bits(&s->gb, 3);
> +        /* if (s->bitalloc_huffman[i] == 7) bailout */
> +    }

static void get_array(GetBitContext *gb, uint8_t *data, int bits, int len){
    while(len--)
        *data++ = get_bits(gb, bits);
}

get_array(&s->gb, s->joint_intensity    , 3, s->prim_channels);
get_array(&s->gb, s->transient_huffman  , 2, s->prim_channels);
get_array(&s->gb, s->scalefactor_huffman, 3, s->prim_channels);
get_array(&s->gb, s->bitalloc_huffman   , 3, s->prim_channels);


> +
> +    /* Get codebooks quantization indexes */
> +    memset(s->quant_index_huffman, 0, sizeof(s->quant_index_huffman));
> +    for (i = 0; i < s->prim_channels; i++)
> +        s->quant_index_huffman[i][1] = get_bits(&s->gb, 1);
> +    for (j = 2; j < 6; j++)
> +        for (i = 0; i < s->prim_channels; i++)
> +            s->quant_index_huffman[i][j] = get_bits(&s->gb, 2);
> +    for (j = 6; j < 11; j++)
> +        for (i = 0; i < s->prim_channels; i++)
> +            s->quant_index_huffman[i][j] = get_bits(&s->gb, 3);

    static const len[11]= {1,1,2,2,2,2,3,3,3,3,3};
    for (j=1; j < 11; j++)
        for (i = 0; i < s->prim_channels; i++)
            s->quant_index_huffman[i][j] = get_bits(&s->gb, len[j]);


[...]
> +    for (i = 0; i < s->prim_channels; i++)
> +        if (s->quant_index_huffman[i][1] == 0)
> +            s->scalefactor_adj[i][1] = adj_table[get_bits(&s->gb, 2)];
> +    for (j = 2; j < 6; j++)
> +        for (i = 0; i < s->prim_channels; i++)
> +            if (s->quant_index_huffman[i][j] < 3)
> +                s->scalefactor_adj[i][j] = adj_table[get_bits(&s->gb, 2)];
> +    for (j = 6; j < 11; j++)
> +        for (i = 0; i < s->prim_channels; i++)
> +            if (s->quant_index_huffman[i][j] < 7)
> +                s->scalefactor_adj[i][j] = adj_table[get_bits(&s->gb, 2)];

    static const thr[11]= {1,1,3,3,3,3,7,7,7,7,7};
    for (j=1; j < 11; j++)
        for (i = 0; i < s->prim_channels; i++)
            if (s->quant_index_huffman[i][j] < thr[j])
                s->scalefactor_adj[i][j] = adj_table[get_bits(&s->gb, 2)];

[...]
> +        for (k = 0; k < s->subband_activity[j]; k++) {
> +            s->scale_factor[j][k][0] = 0;
> +            s->scale_factor[j][k][1] = 0;
> +        }

memset()

[...]
> +        /* Scale factor index */
> +        s->lfe_scale_factor = scale_factor_quant7[get_bits(&s->gb, 8)];
> +
> +        /* Quantization step size * scale factor */
> +        lfe_scale = 0.035 * s->lfe_scale_factor;

lfe_scale_factor is not used anywher except here, so it shouldnt be in
the context


[...]
> +
> +    return 0;
> +}
> +
> +static void qmf_32_subbands(DCAContext * s, int chans,
> +                            float samples_in[32][8], float *samples_out,
> +                            float scale, float bias)
> +{
> +    float *prCoeff;
> +    int i, j, k;
> +    float praXin[33], *raXin = &praXin[1];
> +
> +    float *subband_fir_hist = s->subband_fir_hist[chans];
> +    float *subband_fir_hist2 = s->subband_fir_noidea[chans];
> +
> +    int chindex = 0, subindex;
> +
> +    praXin[0] = 0.0;
> +
> +    /* Select filter */
> +    if (!s->multirate_inter)    /* Non-perfect reconstruction */
> +        prCoeff = (float *) fir_32bands_nonperfect;
> +    else                        /* Perfect reconstruction */
> +        prCoeff = (float *) fir_32bands_perfect;
> +
> +    /* Reconstructed channel sample index */
> +    for (subindex = 0; subindex < 8; subindex++) {
> +        float A[16], B[16], SUM[16], DIFF[16];

iam still protesting against the uppercased local variable names


> +
> +        /* Load in one sample from each subband */
> +        for (i = 0; i < s->subband_activity[chans]; i++)
> +            raXin[i] = samples_in[i][subindex];
> +
> +        /* Clear inactive subbands */
> +        for (i = s->subband_activity[chans]; i < 32; i++)
> +            raXin[i] = 0.0;

for(; i < 32; i++)


> +
> +        /* Multiply by cosine modulation coefficients and
> +         * create temporary arrays SUM and DIFF */
> +        for (j = 0, k = 0; k < 16; k++) {
> +            A[k] = 0.0;
> +            B[k] = 0.0;
> +            for (i = 0; i < 16; i++, j++){
> +                A[k] += (raXin[2 * i] + raXin[2 * i + 1]) * s->cos_mod[j];
> +                B[k] += (raXin[2 * i] + raXin[2 * i - 1]) * s->cos_mod[j + 256];
> +            }

maybe the 16 should rather be s->subband_activity[chans]/2+1 or so
and the raXin[2 * i] + raXin[2 * i +- 1] sums can be precalculated


> +            SUM[k] = A[k] + B[k];
> +            DIFF[k] = A[k] - B[k];

also A and B can be changed to scalars they dont need to be arrays

furthermore this probably can be factorized so its solveable in O(n log n)
instead of O(n*n) (n=16) (but iam too lazy to figure out the details so feel
free to ignore this)


[...]
> +                subband_fir_hist2[i] += prCoeff[i+j] * (subband_fir_hist[i+j] - subband_fir_hist[j+k]);
> +                subband_fir_hist2[32+i] += prCoeff[32 + i + j]*(-subband_fir_hist[i + j] - subband_fir_hist[j + k]);

these can be vertically aligned so it looks more beautifull


> +            }
> +
> +        /* Create 32 PCM output samples */
> +        for (i = 0; i < 32; i++)
> +            samples_out[chindex++] = subband_fir_hist2[i] / scale + bias;

scale= 1/scale;
for()
    .... * scale


> +
> +        /* Update working arrays */
> +        memmove(&subband_fir_hist[32], &subband_fir_hist[0], (512 - 32) * sizeof(float));

hmm moving ~2k around ...


[...]
> +    /* Select decimation filter */
> +    if (decimation_select == 1) {
> +        decifactor = 128;
> +        prCoeff = (float *) lfe_fir_128;
> +    } else {
> +        decifactor = 64;
> +        prCoeff = (float *) lfe_fir_64;

what are the float * casts good for?


[...]

> +            /* Select quantization index code book */
> +            int sel = s->quant_index_huffman[k][abits];
> +            /* Determine its type */
> +            int q_type = 1;     /* (Assume Huffman type by default) */
> +            if (abits >= 11 || !abits
> +                || !dca_smpl_bitalloc[abits].vlc[sel].table) {
> +                /* Not Huffman type */
> +                if (abits <= 7)
> +                    q_type = 3; /* Block code */
> +                else
> +                    q_type = 2; /* No further encoding */
> +            }
> +
> +            if (abits == 0)
> +                q_type = 0;     /* No bits allocated */
> +
> +            /*
> +             * Extract bits from the bit stream
> +             */
> +            switch (q_type) {
> +            case 0:            /* No bits allocated */
> +                for (m = 0; m < 8; m++)
> +                    subband_samples[k][l][m] = 0;
> +                break;
> +
> +            case 1:            /* Huffman code */
> +                for (m = 0; m < 8; m++)
> +                    subband_samples[k][l][m] =
> +                        get_bitalloc(&s->gb, &dca_smpl_bitalloc[abits], sel);
> +                break;
> +
> +            case 2:            /* No further encoding */
> +                for (m = 0; m < 8; m++) {
> +                    /* Extract (signed) quantization index */
> +                    subband_samples[k][l][m] = get_sbits(&s->gb, abits - 3);
> +                }
> +                break;
> +
> +            case 3:            /* Block code */
> +                {
> +                    int block_code1, block_code2, size, levels;
> +                    int block[8];
> +
> +                    if(abits < 1 || abits > 6){
> +                        size = abits_sizes[6];
> +                        levels = abits_levels[6];
> +                    }else{
> +                        size = abits_sizes[abits-1];
> +                        levels = abits_levels[abits-1];
> +                    }
> +
> +                    block_code1 = get_bits(&s->gb, size);
> +                    /* FIXME Should test return value */
> +//START_TIMER
> +                    decode_blockcode(block_code1, levels, block);
> +                    block_code2 = get_bits(&s->gb, size);
> +                    decode_blockcode(block_code2, levels, &block[4]);
> +//STOP_TIMER("block_code")
> +                    for (m = 0; m < 8; m++)
> +                        subband_samples[k][l][m] = block[m];
> +
> +                }
> +                break;
> +
> +            default:           /* Undefined */
> +                av_log(s->avctx, AV_LOG_DEBUG, "Unknown quantization index codebook");
> +                return -1;
> +            }

if(!abits){
    for (m = 0; m < 8; m++)
        subband_samples[k][l][m] = 0;
}else if (abits >= 11 || !dca_smpl_bitalloc[abits].vlc[sel].table) {
    if(abits <= 7){
        int block_code1, block_code2, size, levels;
        int block[8];

        if(abits < 1 || abits > 6){
            size = abits_sizes[6];
            levels = abits_levels[6];
        }else{
            size = abits_sizes[abits-1];
            levels = abits_levels[abits-1];
        }

        block_code1 = get_bits(&s->gb, size);
        /* FIXME Should test return value */

        decode_blockcode(block_code1, levels, block);
        block_code2 = get_bits(&s->gb, size);
        decode_blockcode(block_code2, levels, &block[4]);

        for (m = 0; m < 8; m++)
            subband_samples[k][l][m] = block[m];
    }else{
        for (m = 0; m < 8; m++) 
            subband_samples[k][l][m] = get_sbits(&s->gb, abits - 3);
}else{
    for (m = 0; m < 8; m++)
        subband_samples[k][l][m] =
            get_bitalloc(&s->gb, &dca_smpl_bitalloc[abits], sel);
}


[...]

> +    for (i = 0; i < 128; i++) {
> +        s->dynrange_tab[i] = pow(10, (i * 0.25) / 20);

i just wanted to say
pow(10, i/80.0);
then greped, and realized its unused, please check that all the fields are
actually used, and remove the ones which are not !!!


[...]
> +#define IS_MARKER(state, i, buf, buf_size) \
> + ((state == DCA_MARKER_14B_LE && (i < buf_size-2) && (buf[i+1] & 0xF0) == 0xF0 && buf[i+2] == 0x07) \
> + || (state == DCA_MARKER_14B_BE && (i < buf_size-2) && buf[i+1] == 0x07 && (buf[i+2] & 0xF0) == 0xF0) \
> + || state == DCA_MARKER_RAW_LE || state == DCA_MARKER_RAW_BE)

cant that be simplified? in its current form its a little slow i guess
they way its used ...

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

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070223/cfb1f163/attachment.pgp>



More information about the ffmpeg-devel mailing list