[Ffmpeg-devel] [PATCH] DTS decoder

[Michael Niedermayer]

Hi

On Fri, Feb 23, 2007 at 07:43:41AM +0200, Kostya wrote:
[...]
> [...] 
> > > static int dca_subframe_header(DCAContext * s)
> > > {
> > >     /* Primary audio coding side information */
> > >     int j, k;
> > > 
> > >     /* Subsubframe count */
> > >     s->subsubframes = get_bits(&s->gb, 2) + 1;
> > > #ifdef DEBUG
> > >     av_log(s->avctx, AV_LOG_DEBUG, "subsubframes: %i\n", s->subsubframes);
> > > #endif
> > > 
> > >     /* Partial subsubframe sample count */
> > >     s->partial_samples = get_bits(&s->gb, 3);
> > > #ifdef DEBUG
> > >     av_log(s->avctx, AV_LOG_DEBUG, "partial samples: %i\n",
> > >            s->partial_samples);
> > > #endif
> > > 
> > >     /* Get prediction mode for each subband */
> > >     for (j = 0; j < s->prim_channels; j++) {
> > >         for (k = 0; k < s->subband_activity[j]; k++)
> > >             s->prediction_mode[j][k] = get_bits(&s->gb, 1);
> > > #ifdef DEBUG
> > >         av_log(s->avctx, AV_LOG_DEBUG, "prediction mode:");
> > >         for (k = 0; k < s->subband_activity[j]; k++)
> > >             av_log(s->avctx, AV_LOG_DEBUG, " %i", s->prediction_mode[j][k]);
> > >         av_log(s->avctx, AV_LOG_DEBUG, "\n");
> > > #endif
> > 
> > and please get rid of the debug code interspaced everywhere
> > use #define TRACE or if needed extent the TRACE related code
> 
> In what way? Totally drop or just move into one block?

well do whatever you prefer, either is much better then the confussing
interleaved debug currently


[...]

> 
> > > {
> > >     int i;
> > >     float t;
> > > 
> > >     switch (srcfmt) {
> > >     case DCA_MONO:
> > >     case DCA_CHANNEL:
> > >     case DCA_STEREO_TOTAL:
> > >     case DCA_STEREO_SUMDIFF:
> > >     case DCA_4F2R:
> > >         av_log(NULL, 0, "Not implemented!\n");
> > >         break;
> > >     case DCA_STEREO:
> > >         break;
> > >     case DCA_3F:
> > >         DOWNMIX_TO_STEREO(MIX_FRONT3(samples),);
> > >         break;
> > >     case DCA_2F1R:
> > >         DOWNMIX_TO_STEREO(MIX_REAR1(samples, i + 512),);
> > >         break;
> > >     case DCA_3F1R:
> > >         DOWNMIX_TO_STEREO(MIX_FRONT3(samples),
> > >                           MIX_REAR1(samples, i + 768));
> > >         break;
> > >     case DCA_2F2R:
> > >         DOWNMIX_TO_STEREO(MIX_REAR2(samples, i + 512, i + 768),);
> > >         break;
> > >     case DCA_3F2R:
> > >         DOWNMIX_TO_STEREO(MIX_FRONT3(samples),
> > >                           MIX_REAR2(samples, i + 768, i + 1024));
> > >         break;
> > >     }
> > > }
> > 
> > all the downmixig code of the various audio codecs should be merged ideally
> 
> Yes, but there are two codecs now which use downmixing (ac3 and this) and
> none has been integrated yet.

yes, thats why i said ideally ...


[...]
> > [...]
> > 
> > > 
> > > /**
> > >  * Main frame decoding function
> > >  * FIXME add arguments
> > >  */
> > > static int dca_decode_frame(AVCodecContext * avctx,
> > >                             void *data, int *data_size,
> > >                             uint8_t * buf, int buf_size)
> > > {
> > > 
> > >     int i, j, k;
> > >     int16_t *samples = data;
> > >     DCAContext *s = avctx->priv_data;
> > >     int channels;
> > > 
> > > 
> > >     *data_size = 0;
> > 
> > you should check that the buffer is large enough
> 
> BTW maybe there should be an additional field for expected data size
> so only the codecs with variable frame size would explicitly check it?

see AVCodecContext.frame_size


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

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- 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/6fa99451/attachment.pgp>