[FFmpeg-devel] [PATCH] LPCM in MPEG-TS, next iteration (Cosmetic changes for coding style)

[Christian P. Schmidt]

Christian P. Schmidt wrote:
> 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
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel

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