[FFmpeg-devel] [PATCH] OpenEXR decoder rev-3

[Jimmy Christensen]

On 2009-07-01 15:55, Diego Biurrun wrote:
> On Wed, Jul 01, 2009 at 03:34:19PM +0200, Jimmy Christensen wrote:
>> On 2009-07-01 15:10, Diego Biurrun wrote:
>>> On Wed, Jul 01, 2009 at 02:46:54PM +0200, Jimmy Christensen wrote:
>>>>
>>>> --- libavcodec/exr.c	(revision 0)
>>>> +++ libavcodec/exr.c	(revision 0)
>>>> @@ -0,0 +1,275 @@
>>>> +        if (strcmp(variable_buffer_name, "dataWindow") == 0) {
>>>> +            xmin = AV_RL32(buf);
>>>> +            ymin = AV_RL32(buf+4);
>>>> +            xmax = AV_RL32(buf+8);
>>>> +            ymax = AV_RL32(buf+12);
>>>> +            xdelta = (xmax-xmin)+1;
>>>
>>> Spaces around + would make this more readable.
>>>
>>>> +        if (strcmp(variable_buffer_name, "displayWindow") == 0) {
>>>> +
>>>> +        if (strcmp(variable_buffer_name, "lineOrder") == 0) {
>>>
>>> The '== 0' is unnecessary.
>>
>> Changed all the strcmp to something like this:
>>
>> if (!strcmp(variable_buffer_name, "lineOrder"))
>>
>>>
>>>> +            if(*buf != 0) {
>>>
>>> similar
>>
>> I suppose you mean the spaces thing. I would however like to like to
>> keep the "*buf != 0" part since it makes it a little more descriptive IMHO.
>
>> --- libavcodec/exr.c	(revision 0)
>> +++ libavcodec/exr.c	(revision 0)
>> @@ -0,0 +1,276 @@
>> +
>> +//#include "libavutil/half.h"
>
> Why this commented out #include?

Whoops. From an old approach. Removed along with some other header files 
which were unnecessary.

>
>> +                if (!strcmp(channel_name, "R")) {
>> +                    red_channel = channel_iter;
>> +                    bits_per_color_id = AV_RL32(ptr_tmp);
>> +                }
>> +                if (!strcmp(channel_name, "G")) {
>> +                    green_channel = channel_iter;
>> +                }
>> +                if (!strcmp(channel_name, "B")) {
>> +                    blue_channel = channel_iter;
>> +                }
>
> pointless {}
>
>> +        if (!strcmp(variable_buffer_name, "lineOrder")) {
>> +            if (*buf != 0) {
>> +                av_log(avctx, AV_LOG_ERROR, "Doesn't support this line order : %d\n", *buf);
>> +            }
>> +        }
>
> pointless {}

Corrected.

>
> The '!= 0' is still pointless.

Corrected.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenEXR-rev3.diff
Type: text/x-patch
Size: 12171 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090701/11625e9c/attachment.bin>