[FFmpeg-devel] [PATCH] Add DPX decoder rev-25

[Jimmy Christensen]

On 2009-06-11 00:51, Michael Niedermayer wrote:
> On Wed, Jun 10, 2009 at 06:17:00PM +0200, Jimmy Christensen wrote:
>> On 2009-06-10 16:48, Diego Biurrun wrote:
>>> On Wed, Jun 10, 2009 at 02:36:58PM +0200, Jimmy Christensen wrote:
>>>>
>>>> I was wondering how close this DPX patch is to get accepted. Is the
>>>> general quality good enough or does it need to be approved a lot more?
>>>
>>> Close.
>>>
>>
>> Thanks. Have started looking into the 3D lut filter for doing log/lin
>> conversion. A bit challenging and slow as hell but have already gotten some
>> promising results.
>>
>>>> --- libavcodec/dpx.c	(revision 0)
>>>> +++ libavcodec/dpx.c	(revision 0)
>>>> @@ -0,0 +1,227 @@
>>>> +static unsigned int read32(const uint8_t **ptr, int is_big)
>>>> +{
>>>> +    unsigned int temp;
>>>> +    if(is_big)
>>>
>>> if (
>>>
>>
>> Fixed.
>>
> [...]
>
>> +    /* Check if the files "magic number" is "SDPX" which means it uses
>> +     * big-endian or XPDS which is for little-endian files */
>> +    if (magic_num == AV_RL32("SDPX"))
>> +        endian = 0;
>> +    else if (magic_num == AV_RB32("SDPX"))
>> +        endian = 1;
>> +    else {
>
> please add {} here they need no extra lines and they make future patches
> smaller

Added, although in the beginning I was told to remove more or less those 
exact {} by Diego :)

 From Diego :
>> > +	if(magic_num == 0x53445058) {
>> > +		endian = 0x0;
>> > +	}
> useless {}

But does that go for all one line if's or just that pair?

>
> [...]
>> +    switch (bits_per_color) {
>> +        case 8:
>> +            if (elements == 4)
>> +                avctx->pix_fmt = PIX_FMT_RGBA;
>> +            else
>> +                avctx->pix_fmt = PIX_FMT_RGB24;
>> +            source_packet_size = elements;
>> +            target_packet_size = elements;
>> +            break;
>> +        case 10:
>> +            avctx->pix_fmt = PIX_FMT_RGB48;
>> +            target_packet_size = 6;
>> +            source_packet_size = elements * 2;
>> +            break;
>> +        case 12:
>> +        case 16:
>> +            if (endian)
>> +                avctx->pix_fmt = PIX_FMT_RGB48BE;
>> +            else
>> +                avctx->pix_fmt = PIX_FMT_RGB48LE;
>> +            target_packet_size = 6;
>> +            source_packet_size = elements * 2;
>> +            break;
>> +        default:
>
>> +            av_log(avctx, AV_LOG_ERROR, "Unsupported color depth\n");
>
> the bits_per_color could be printed too
>

Done.

>
>> +            return -1;
>
>> +            break;
>
> unreachable
>

Removed.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpegDPX-rev25.diff
Type: text/x-patch
Size: 10441 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090611/73dfb501/attachment.bin>