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

[Diego Biurrun]

On Tue, Sep 08, 2009 at 01:36:42PM +0200, Jimmy Christensen wrote:
> 
> Thanks for the review.

You are welcome.

> Guess too much vacation makes you forget the 
> basic stuff :)

:)

> --- Changelog	(revision 19793)
> +++ Changelog	(working copy)
> @@ -37,9 +37,9 @@
>  - Bluray (PGS) subtitle decoder
>  - LPCM support in MPEG-TS (HDMV RID as found on Blu-ray disks)
>  - Wmapro decoder
> +- OpenEXR image decoder
>  
>  
> -
>  version 0.5:

Please keep the empty line in place.

> --- libavcodec/exr.c	(revision 0)
> +++ libavcodec/exr.c	(revision 0)
> @@ -0,0 +1,464 @@
> +
> +typedef struct EXRContext {
> +    AVFrame picture;
> +    int compr;
> +
> +    int   red_channel;
> +    int green_channel;
> +    int  blue_channel;
> +    int bits_per_color_id;
> +
> +    int     red_channel_offset;
> +    int   green_channel_offset;
> +    int    blue_channel_offset;
> +
> +} EXRContext;

nit: pointless empty line

> +    if (buf_end - buf >= minimum_length && !strncmp(buf, value_name, strlen(value_name))) {
> +
> +    if (channel_iter == s->red_channel || channel_iter == s->green_channel || channel_iter == s->blue_channel) {
> +
> +                    if (get_rgb_channel(&buf, channel_list_end, s, channel_iter, &current_channel_offset) == -1) {

These lines for example could easily be broken, there are more.

> +    switch (s->bits_per_color_id) {
> +        // 32-bit
> +        case 2:

Indent switch and case at the same depth.

Diego