[FFmpeg-devel] [patch] 24bit lpcm in mpeg (vob)

[Lars Täuber]

Hallo!

On Sat, 12 Apr 2008 16:58:53 +0200 Diego Biurrun <diego at biurrun.de> wrote:
> > --- ffmpeg/libavcodec/allcodecs.c	2008-04-11 18:11:43.000000000 +0200
> > +++ ffmpeg.1/libavcodec/allcodecs.c	2008-04-11 22:43:02.000000000 +0200
> > @@ -232,6 +232,7 @@ void avcodec_register_all(void)
> >      REGISTER_ENCDEC  (PCM_U32BE, pcm_u32be);
> >      REGISTER_ENCDEC  (PCM_U32LE, pcm_u32le);
> >      REGISTER_ENCDEC  (PCM_ZORK , pcm_zork);
> > +    REGISTER_DECODER (PCM_S24DVD, pcm_s24dvd);
> 
> alphabetical order please

OK, done.

 
> > --- ffmpeg/libavcodec/pcm.c	2008-03-21 13:17:05.000000000 +0100
> > +++ ffmpeg.1/libavcodec/pcm.c	2008-04-12 14:08:37.000000000 +0200
> > @@ -434,6 +434,19 @@ static int pcm_decode_frame(AVCodecConte
> > +    case CODEC_ID_PCM_S24DVD:
> > +        n = buf_size / 12;
> > +        for(;n>0;n--) {
> > +          *samples++ = bytestream_get_be16(&src);
> > +          *samples++ = bytestream_get_be16(&src);
> > +          *samples++ = bytestream_get_be16(&src);
> > +          *samples++ = bytestream_get_be16(&src);
> > +          src+=4;       /**
> > +                          ignore the least significant bytes of each sample,
> > +                          because we only support 16bit audio right now
> > +                        */
> 
> This comment looks weird to me.  Why not put it above the line?

Changed to: 
+          *samples++ = bytestream_get_be16(&src);
+          /* ignore the least significant bytes of each sample,
+             because we only support 16bit audio right now 
+           */
+          src+=4;

> > +            st->codec->bit_rate = st->codec->channels * st->codec->sample_rate * 2;
> 
> Please break these long lines.

Done.

 
> > --- ffmpeg.1/libavcodec/utils.c	2008-04-12 14:54:36.000000000 +0200
> > +++ ffmpeg.3/libavcodec/utils.c	2008-04-12 15:24:44.000000000 +0200
> > @@ -1326,13 +1326,14 @@ int av_get_bits_per_sample(enum CodecID 
> >      case CODEC_ID_PCM_U32LE:
> >          return 32;
> > +    case CODEC_ID_PCM_S24DVD:
> > +        return 48;  /** here are 2 samples interleaved per block */
> 
> That sounds ungrammatical, maybe just say
> 
>   /** 2 samples are interleaved per block. */

Changed.

Lars
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 24pcm_dvd.patch
Type: text/x-diff
Size: 3904 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080412/cd6f19a1/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: better_sanity_check.patch
Type: text/x-diff
Size: 875 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080412/cd6f19a1/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: better_sanity_check_for_24lpcm.patch
Type: text/x-diff
Size: 683 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080412/cd6f19a1/attachment-0002.patch>