[FFmpeg-devel] [PATCH] LPCM in MPEG-TS, next iteration

Christian P. Schmidt schmidt
Fri Aug 14 16:53:21 CEST 2009


Hi all,

Apart from what's mentioned below I made a mistake in one of the dprintf
statements.

Benjamin Larsson wrote:

>> +    /* 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;
>>   
> 
> use shift instead

I'm probably trusting compilers too much... done.

>> +    num_samples = buf_size / sample_size;
>> +
>> +    dprintf(avctx, "pcm_bluray_decode_frame: c: %d sc: %d s: %d in:
>> %d ds: %d\n",
>> +            avctx->channels, num_source_channels, num_samples, buf_size,
>> +            *data_size);
>> +
>> +    switch(avctx->channel_layout) {
>> +    case CH_LAYOUT_STEREO:  ///< same number of source and coded
>> channels
>> +    case CH_LAYOUT_4POINT0:
>> +    case CH_LAYOUT_2_2:
>> +        if(16 == avctx->bits_per_coded_sample) {
>>   
> 
> I'd prefer if(avctx->bits_per_coded_sample == 16) but that's just me

I saw it like I used it in other places. It also helps to prevent
wrongfully assigning values instead of doing a comparison.

>> +    if(16 == avctx->bits_per_coded_sample)
>> +        *data_size = 2*num_samples*avctx->channels;
>> +    else
>> +        *data_size = 4*num_samples*avctx->channels;
>>   
> 
> IIRC the 2 and 4 shouldn't be constants. sizeof(int16||int32) or
> something else.

If those were different from 2 and 4 I think the whole code would break,
but anyway, done.

>> +    .long_name = NULL_IF_CONFIG_SMALL("PCM signed 16|20|24-bit
>> big-endian"),
>>   
> 
> Should be a "bluray" somewhere in the long name.

Done.

Regards,
Christian
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: lpcm-mpegts.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090814/e45dde72/attachment.txt>



More information about the ffmpeg-devel mailing list