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

[Christian P. Schmidt]

Jai Menon wrote:

> Is there any spec which could be cited instead? Just asking...

Sure, will do. I don't have it, though ;)

>> +static int pcm_mpeg_decode_frame(AVCodecContext *avctx,
>> +                                 void *data, int *data_size,
>> +                                 AVPacket *avpkt)
>> +{
>> +    const uint8_t *src = avpkt->data;
>> +    int buf_size = avpkt->size;
>> +    int num_source_channels, channel, retval;
>> +    int sample_size, num_samples, sample;
>> +
>> +    int32_t *dst = data;
>> +
>> +    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]))
> 
> Since you use src[2], src[3] etc for array lookups, isnt passing these
> on directly without sanity checking here or in the init function kinda
> fragile..

It is checked that these array elements actually exist (see buf_size check just before), and they are fully decoded in the init
function. I don't think more sanity checking is possible at the moment (sanity checking in the init function is done by bailing out
on "unknown" values. But as Baptiste commented, it makes perfect sense to just pass src and get the subscripts out in tht einit
function. Will change it.

>> +                return -1;
>> +            break;
>> +        default:
>> +          return -1;
>> +        }
>> +    }
>> +
>> +    /* eat header */
>> +    src += 4;
>> +    buf_size -= 4;
>> +
>> +    /* There's always an even number of channels in the source */
>> +    num_source_channels = (avctx->channels & 1) ? avctx->channels + 1 : avctx->channels;
>> +
>> +    /* Order of calculation matters: early division by 8 kills the fraction for 20bit samples */
>> +    sample_size = (num_source_channels * avctx->bits_per_coded_sample) / 8;
>> +    num_samples = buf_size / sample_size;
>> +
>> +    av_log(avctx, AV_LOG_DEBUG, "dec: c: %d sc: %d s: %d in: %d\n",
>> +           avctx->channels, num_source_channels, num_samples, buf_size);
>> +
>> +    if (num_source_channels == avctx->channels) {
>> +        switch (avctx->bits_per_coded_sample) {
>> +        case 16:
>> +            for (sample = num_samples*avctx->channels; sample; sample--)
>> +                *dst++ = bytestream_get_be16(&src) << 16;
> 
> bytestream_get_buffer perhaps ?

Not sure. First of all, that would depend on the endianess. Sure, I could easily have the #ifdef for that. Second, memcpy + shift
could be worse than byte copy + shift, I would have to look at the assembler stage to see...

Regards,
Christian