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

[Michael Niedermayer]

On Fri, Apr 18, 2008 at 09:37:12PM +0200, Lars T?uber wrote:
> On Fri, 18 Apr 2008 22:08:24 +0200 Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Fri, Apr 18, 2008 at 09:05:23PM +0200, Lars T?uber wrote:
> > > On Fri, 18 Apr 2008 15:39:43 +0200 Michael Niedermayer <michaelni at gmx.at> wrote:
> > > > On Thu, Apr 17, 2008 at 10:13:40PM +0200, Lars T?uber wrote:
> [...]
> > > > > +            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?
> > > 
> > > Yes, because it's calculated as shown above. (There are only 3 bits that tell the number of channels)
> > > 
> > > > The array should either be sized related to MAX_CHANNELS or channels be
> > > > checked against the size of the array.
> > > 
> > > the widest sample size is 24 bits*channels => fit into int
> > > channels <= 8
> > > exactly 2 samples per block
> > > => max 2*8 ints/block
> > > 
> > > Is that enough to be safe?
> > 
> > Well, what if someone would store CODEC_ID_PCM_DVD in for example .mkv .avi
> > .mov  ...
> 
> I added a check.

[...]

> @@ -492,6 +498,35 @@ 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;
> +
> +            if (avctx->channels <= 0 || avctx->channels > 8) {
> +                av_log(avctx, AV_LOG_ERROR, "PCM_DVD channels out of bounds\n");
> +                return -1;
> +            }
> +            n = buf_size / (avctx->channels * 2 * avctx->bits_per_sample / 8);
> +            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;
> +                    }
> +                else {
> +                    for (c=0; c < 2*avctx->channels; c++, src+=2, src_LSB++ )
> +                        *ap++ = src[0]<<16 | src[1]<<8 | *src_LSB;
> +                }
> +                src = src_LSB;
> +
> +                for (c=0; c < avctx->channels*2; c++)
> +                    *samples++ = audio24[c] >> 8;
> +            }

and what if bits_per_sample is something else than 20 or 24 ?

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- 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/662d28c3/attachment.pgp>