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

[Diego Biurrun]

On Sat, Apr 12, 2008 at 03:27:43PM +0200, Lars T?uber wrote:
> 
> this is a first try to get ffmpeg support 24 bit lpcm audio on dvd.
> The byte order description on multimedia wiki is wrong, but I had a
> look at the mplayer code to be sure about the right order.
>
> --- 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

> --- 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?

> --- ffmpeg/libavformat/mpeg.c	2008-03-04 21:54:06.000000000 +0100
> +++ ffmpeg.1/libavformat/mpeg.c	2008-04-12 14:40:53.000000000 +0200
> @@ -518,7 +518,13 @@ static int mpegps_read_packet(AVFormatCo
>          st->codec->channels = 1 + (b1 & 7);
> -        st->codec->bit_rate = st->codec->channels * st->codec->sample_rate * 2;
> +
> +        if ((( b1 >> 6) & 3 ) == 2 ) {
> +            st->codec->codec_id = CODEC_ID_PCM_S24DVD;
> +            /* 3 bytes instead of 2 bytes per sample */
> +            st->codec->bit_rate = st->codec->channels * st->codec->sample_rate * 3;
> +        } else
> +            st->codec->bit_rate = st->codec->channels * st->codec->sample_rate * 2;

Please break these long lines.

> --- 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. */

Diego