[Ffmpeg-devel] [PATCH] DTS decoder

[Kostya]

On Fri, Feb 23, 2007 at 08:14:23PM +0100, Michael Niedermayer wrote:
> 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

done 
> 
> [...]
> 
> > +#define DCA_CHANNEL_MAX  DCA_3F2R       /* We don't handle anything above that */
> 
> unused

Dropped 

[dca_parse_frame_header_mini() and dca_parse_frame_header()]

merged into one function, fixed return value and flags 
 
> [...]
> 
> > +    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 

They are

[...]
> [...]
> > +    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);

done 

[merge loops into one and use arrays for variable parameter]
done

[...]
> iam still protesting against the uppercased local variable names
 
Renamed them to lowercase

[...]
> > +        /* 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

just inverted the input parameter 

[...] 
> > +    /* 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?

for hiding gcc warning, dropped them

[...] 
> > +    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 !!!

I've removed half a dozen of such variables sparing those that could be needed
in future (like missing features, more flexible downmixing, etc)
> 
> 
> [...]
> > +#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 ...

I don't think so. DT$ designers allow 4 bitstream formats (raw/14bits BE/LE),
each has its own marker (4 bytes for raw, 5.5 bytes for 14bits bitstream)

> [...]
> -- 
> 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: dca.patch.gz
Type: application/x-gzip
Size: 10781 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070224/92f01199/attachment.bin>