[Ffmpeg-devel] [PATCH] THP PCM decoder (GSoC Qualification)

[Michael Niedermayer]

Hi

On Mon, Apr 02, 2007 at 07:24:49PM +0200, Marco Gerards wrote:
> Baptiste Coudurier <baptiste.coudurier at smartjog.com> writes:
[...]
> >> [...]
> >> +    }
> >> +    else {
> >> +       ret = av_get_packet(pb, pkt, thp->audiosize);
> >> +       if (ret != thp->audiosize) {
> >> +          av_free_packet(pkt);
> >> +          return AVERROR_IO;
> >> +       }
> >> +      pkt->stream_index = thp->audio_stream_index;
> >> +      thp->audiosize = 0;
> >> +      thp->frame++;
> >
> > Can't seek be avoided now ? Does audio follow video in data stream ? If
> > so Im wondering if reading video + audio in the same time, using buffer
> > then output audio after, would not be cleaner and simpler.
> 
> It's convenient that I can make a video and audio packet and read
> simultaneously.  I think the code otherwise will get more complex.  Do
> you think it is worth the complexity and reading everything at once?
> 
> I do not think seeking can be avoided.  Some data follows the frame,
> it is not documented.  So there is a gap between the total framesize
> and the image+audio size.

how does that extra data look ? (iam just curious ...)


[...]
> Index: libavcodec/adpcm.c
> ===================================================================
> --- libavcodec/adpcm.c	(revision 8597)
> +++ libavcodec/adpcm.c	(working copy)
> @@ -442,6 +442,8 @@
>      switch(avctx->codec->id) {
>      case CODEC_ID_ADPCM_IMA_QT: /* XXX: can't test until we get .mov writer */
>          break;
> +    case CODEC_ID_ADPCM_THP:
> +        break;
>      case CODEC_ID_ADPCM_IMA_WAV:
>          n = avctx->frame_size / 8;

what is this good for?


>              c->status[0].prev_sample = (signed short)samples[0]; /* XXX */
> @@ -1308,6 +1310,69 @@
>              src++;
>          }
>          break;
> +    case CODEC_ID_ADPCM_THP:
> +      {
> +        GetBitContext gb;
> +        float table[16][2];
> +        int samplecnt;
> +        int prev1[2], prev2[2];
> +        int ch;
> +
> +        if (buf_size < 80) {
> +          av_log(avctx, AV_LOG_ERROR, "frame too small\n");
> +          return -1;
> +        }

indention in ffmpeg is 4 spaces


> +
> +        init_get_bits(&gb, src, buf_size);

size is in bits ...


> +        src += buf_size;
> +
> +                    get_bits(&gb, 32); /* Channel size */
> +        samplecnt = get_bits(&gb, 32);
> +
> +        for (ch = 0; ch < 2; ch++)
> +          for (i = 0; i < 16; i++) {
> +              /* Read the fixed point entry and store as floating
> +                 point.  */
> +              int entry = get_sbits(&gb, 16);
> +              table[i][ch] = (float) entry  / (1 << 11);

please remove all floating point code from the decoder, theres no need
for it and it makes the code binary identical (easy testable) on different
architectures


> +          }
> +
> +        /* Initialize the previous sample.  */
> +        for (ch = 0; ch < 2; ch++) {
> +            prev1[ch] = get_sbits(&gb, 16);
> +            prev2[ch] = get_sbits(&gb, 16);
> +        }
> +
> +        for (ch = 0; ch <= st; ch++) {
> +            int sample = samplecnt;
> +
> +            /* Read in every sample for this channel.  */
> +            while (sample > 0) {

a for() would be simpler here


> +                uint8_t index = get_bits (&gb, 4) & 7;
> +                int exp = get_bits (&gb, 4);
> +                float factor1 = table[index * 2][ch];
> +                float factor2 = table[index * 2 + 1][ch];
> +        

trailing whitespace


> +                /* Decode 14 samples.  */
> +                for (n = 0; n < 14; n++) {
> +                    int sampledat = get_sbits (&gb, 4);
> +                    *samples = prev1[ch]*factor1 
> +                               + prev2[ch]*factor2 + (sampledat << exp);
> +                    prev2[ch] = prev1[ch];
> +                    prev1[ch] = *samples++;
> +
> +                    if (samples >= samples_end) {
> +                       av_log(avctx, AV_LOG_ERROR, "allocated output buffer is too small\n");
> +                       return -1;
> +                    }

this check can be moved out of the loop


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

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- 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/20070402/40728f32/attachment.pgp>