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

Christian P. Schmidt schmidt
Sun Aug 2 12:29:56 CEST 2009


Baptiste Coudurier wrote:
> Hi,
> 
> On 08/02/2009 12:54 AM, Christian P. Schmidt wrote:

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

That's what I originally did. Setting to SAMPLE_FMT_NONE initially caused crashes with either segmentation violation or floating
point exception, initializing with SAMPLE_FMT_S32 and changing it to SAMPLE_FMT_S16 in the pcm_bluray_init audibly kept playing with
32bit samples (only every second sample was played).

> Besides I think it's time to set sample_fmt to SAMPLE_FMT_NONE in init
> and not SAMPLE_FMT_S16 in utils.c
> 
>> [...]
>> +    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

That would clash later on if I would want ro use the same names for the pcm dvd init... Also, they have no meaning outside the scope
of this function.

But well, if you insist, I'm going to move them...

>> +    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)

Weird, my patch has the format you want. Maybe a codepage error?

>> +          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.

It will not, the meaning of the header is different for both. That's exactly what the switch is for.
Maybe I should rename the function to pcm_bluray_parse_header...

> Oh, also, thanks for the good work :)

You're welcome, but I'm selfish... I just want to play my BluRays ;)

Regards,
Christia



More information about the ffmpeg-devel mailing list