[FFmpeg-devel] LPCM for mpeg-ts, the second

Baptiste Coudurier baptiste.coudurier
Sun Aug 2 10:42:50 CEST 2009


Hi,

On 08/02/2009 12:54 AM, Christian P. Schmidt wrote:
> Hi all,
>
> Attached a second attempt at adding the support for LPCM streams in mpeg transport streams.
>
> After some discussions it's made into a separate file which might at some time also host the code for LPCM in DVDs, removing the
> decoding of the header from the mpeg demuxer and putting it in the codec.
>
> It is only tested with the following formats:
> - 2 channels, 16 bits, 48kHz (my original sample)
> - 6 channels, 24 bits, 48kHz (played cropped to stereo on my laptop, thanks to Baptiste for providing the sample)
> - 8 channels, 16 bits, 48kHz (it is recognized, but ffplay can't play back because SDL rejects 8 channels)
>
> The following limitations exist:
> - Also decodes 16bit to 32bit as the codec apparently can't influence the sample format, and the first packet needs to be decoded
> before we know how many bits are stored (at least ffplay didn't make any sense out of anything else)

Decoder can influence and will, just set the SAMPLE_FMT appropriately in 
decode function, not init. If ffplay has a problem with that, the bug or 
side effect is somewhere else.

Besides I think it's time to set sample_fmt to SAMPLE_FMT_NONE in init 
and not SAMPLE_FMT_S16 in utils.c

 > [...]
 >
> +
> +/* Initialize the PCM codec from the first data header read from the LPCM stream */
> +static int pcm_bluray_init (AVCodecContext *avctx, const uint8_t p2, const uint8_t p3)

I think "const uint8_t *p" is better that two bytes that means nothing 
really understandable.

> +    const uint8_t bits_per_samples[4] = { 0, 16, 0, 24 };
> +    //    const uint8_t bits_per_samples[4] = { 0, 16, 20, 24 }; Not sure how to decode 20bit
> +    const int64_t channel_layouts[16] = {
> +        0, CH_LAYOUT_MONO, 0, CH_LAYOUT_STEREO,
> +        CH_LAYOUT_SURROUND, CH_LAYOUT_2_1, CH_LAYOUT_4POINT0, CH_LAYOUT_2_2,
> +        CH_LAYOUT_5POINT0, CH_LAYOUT_5POINT1, CH_LAYOUT_7POINT0, CH_LAYOUT_7POINT1,
> +        0, 0, 0, 0};
> +    const uint8_t channels[16] = {0, 1, 0, 2, 3, 3, 4, 4, 5, 6, 7, 8, 0, 0, 0, 0};
> +    const int sample_rates [16] = {
> +        0, 48000, 0, 0, 96000, 192000, 0, 0,
> +        0, 0, 0, 0, 0, 0, 0, 0};

static const outside the function

> +    av_log(avctx, AV_LOG_DEBUG, "pcm_bluray_init: p2 = %02x p3 = %02x\n", p2, p3);
> +    /* get the sample depth and derive the sample format that will be used from it */
> +    avctx->bits_per_coded_sample = bits_per_samples[(p3>>  6)&  3];

Nitpick: (p3 >> 6) & 3
To match ffmpeg coding style.

 > [...]
> +
> +    if (buf_size<  4) {

Coding style: if (buf_size < 4)

> +          av_log(avctx, AV_LOG_ERROR, "PCM packet too small\n");
> +          return -1;
> +    }
> +
> +    if (0 == avctx->channels) {
> +        switch(avctx->codec->id) {
> +        case CODEC_ID_PCM_BLURAY:
> +            if (pcm_bluray_init (avctx, src[2], src[3]))
> +                return -1;
> +            break;
> +        default:
> +          return -1;
> +        }
> +    }

Will the PCM_DVD code use the same init function ?
If not the switch is relevant, otherwise, for only one case, it's 
useless IMHO.

Oh, also, thanks for the good work :)

[...]

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org



More information about the ffmpeg-devel mailing list