[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