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

[Diego Biurrun]

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?

> +                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 {}

The '!= 0' is still pointless.

Diego