[FFmpeg-devel] [RFC] LPCM 24 bits support

[Michael Niedermayer]

On Thu, Apr 17, 2008 at 10:13:40PM +0200, Lars T?uber wrote:
> On Thu, 17 Apr 2008 22:49:08 +0200 Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Thu, Apr 17, 2008 at 09:10:07PM +0100, M?ns Rullg?rd wrote:
> > > > +                for (c=0; c < avctx->channels*2; c++)
> > > > +                    *samples++ = audio24[c] >> 8;
> > > 
> > > What's the point in saving 24 bits per sample to a temporary buffer,
> > > only to discard the low 8 bits later?  Also, properly rounding the
> > > values to 16 bits (rather than truncating) might be preferable.
> > 
> > truncation is ok, the 0.5 DC error is not something a physically existing
> > object could hear.
> > 
> > also trivial rounding like (x+128)>>8 has a problem, namely
> > 0xFFFFFF+128 = 0x100007F
> > and >>8 = 0x10000
> 
> Resubmitting with truncation.
[...]

> diff -pur ffmpeg/libavcodec/pcm.c ffmpeg.1/libavcodec/pcm.c
> --- ffmpeg/libavcodec/pcm.c	2008-03-21 13:17:05.000000000 +0100
> +++ ffmpeg.1/libavcodec/pcm.c	2008-04-17 21:46:27.000000000 +0200
> @@ -492,6 +498,31 @@ static int pcm_decode_frame(AVCodecConte
>              *samples++ = s->table[*src++];
>          }
>          break;
> +    case CODEC_ID_PCM_DVD: {
> +            int audio24[8*2], *ap;
> +            const uint8_t *src_LSB;
> +

> +            n = buf_size / (avctx->channels * 2 * avctx->bits_per_sample / 8);

Is there something that prevents that from overflowing? If not then this is a
little risky


> +            while (n--) {
> +                ap = audio24;
> +                src_LSB = src + avctx->channels * 2 * 2;
> +

> +                if (avctx->bits_per_sample == 20)
> +                    for (c=0; c < avctx->channels; c++, src+=4, src_LSB++ ) {
> +                        *ap++ = src[0]<<16 | src[1]<<8 | (*src_LSB & 0xf0);
> +                        *ap++ = src[2]<<16 | src[3]<<8 | (*src_LSB & 0x0f)<<4;
> +                    }

Is there something that prevents channels from being larger than 8?
The array should either be sized related to MAX_CHANNELS or channels be
checked against the size of the array.

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

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- 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/20080418/8216c57e/attachment.pgp>