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

Jimmy Christensen jimmy
Tue Apr 27 09:24:44 CEST 2010


On 04/24/2010 09:04 PM, Stefano Sabatini wrote:
> On date Saturday 2010-04-24 17:59:46 +0200, Jimmy Christensen encoded:
> [...]
>> Index: libavcodec/exr.c
>> ===================================================================
>> --- libavcodec/exr.c	(revision 0)
>> +++ libavcodec/exr.c	(revision 0)
>> @@ -0,0 +1,483 @@
>> +/*
>> + * OpenEXR (.exr) image decoder
>
> This is redundant (already in the @file doxy).
>
>> + * Copyright (c) 2009 Jimmy Christensen
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +
>> +/**
>> + * @file libavcodec/exr.c
>
> "@file" alone should be enough
>

Fixed

>> + * OpenEXR decoder
>> + * @author Jimmy Christensen
>> + *
>> + * For more information on the OpenEXR format, visit:
>> + *  http://openexr.com/
>> + *
>> + * exr_flt2uint() and exr_halflt2uint() is credited to  Reimar D??ffinger
>
> D??ffinger =>  unicode issue?
>

Was not sending as UTF-8. Should be fixed now.

>> + */
>> +
>> +#include "avcodec.h"
>> +#include "bytestream.h"
>> +
>> +enum ExrCompr {
>> +    EXR_RAW   = 0,
>> +    EXR_RLE   = 1,
>> +    EXR_ZIP1  = 2,
>> +    EXR_ZIP16 = 3,
>> +    EXR_PIZ   = 4,
>> +    EXR_B44   = 6
>> +};
>> +
>> +typedef struct EXRContext {
>> +    AVFrame picture;
>> +    int compr;
>> +    int bits_per_color_id;
>> +    int8_t channel_offsets[3]; // 0 = red, 1 = green and 2 = blue
>> +} EXRContext;
>> +
>> +/**
>> + * Convert from 32-bit float as uint32_t to uint16_t
>
> "Converts" - please use third person here and below for consistency
> sake.
>

Fixed.

> Also add an empty newline between description and @params, here and
> below, improve readability.
>

Fixed.

>> + * @param v 32-bit float
>> + * @return normalized 16-bit unsigned int
>> + */
>> +static inline uint16_t exr_flt2uint(uint32_t v)
>> +{
>> +    unsigned int exp = v>>  23;
>> +    // "HACK": negative values result in exp<   0, so clipping them to 0
>> +    // is also handled by this condition, avoids explicit check for sign bit.
>> +    if (exp<= 127 + 7 - 24) // we would shift out all bits anyway
>> +        return 0;
>> +    if (exp>= 127)
>> +        return 0xffff;
>> +    v&= 0x007fffff;
>> +    return (v + (1<<  23))>>  (127 + 7 - exp);
>> +}
>> +
>> +/**
>> + * Convert from 16-bit float as uint16_t to uint16_t
>> + * @param v 16-bit float
>> + * @return normalized 16-bit unsigned int
>> + */
>> +static inline uint16_t exr_halflt2uint(uint16_t v)
>> +{
>> +    int exp = v>>  10;
>> +    if (v&  0x8000)
>> +        return 0;
>> +    if (!exp)
>> +        return (v>>  9)&  1;
>> +    if (exp>= 15)
>> +        return 0xffff;
>> +    v<<= 6;
>> +    return (v + (1<<  16))>>  (15 - exp);
>> +}
>> +
>> +/**
>> + * Gets the size of the header variable
>> + * @param **buf the current pointer location in the header where
>> + * the variable data starts
>> + * @param *buf_end pointer location of the end of the buffer
>> + * @return size of variable data
>> + */
>> +static unsigned int get_header_variable_length(const uint8_t **buf,
>> +                                               const uint8_t *buf_end)
>> +{
>> +    unsigned int variable_buffer_data_size = bytestream_get_le32(buf);
>> +    if (variable_buffer_data_size>= buf_end - *buf)
>> +        return 0;
>> +    return variable_buffer_data_size;
>> +}
>> +
>> +/**
>> + * Checks if the variable name corresponds with it's data type
>> + * @param *avctx the AVCodecContext
>> + * @param **buf the current pointer location in the header where
>> + * the variable name starts
>> + * @param *buf_end pointer location of the end of the buffer
>> + * @param *value_name name of the varible to check
>> + * @param *value_type type of the varible to check
>> + * @param minimum_length minimum length of the variable data
>> + * @param variable_buffer_data_size variable length read from the header
>> + * after it's checked
>> + * @return zero if variable is invalid and 1 if good
>> + */
>> +static unsigned int check_header_variable(AVCodecContext *avctx,
>> +                                              const uint8_t **buf,
>> +                                              const uint8_t *buf_end,
>> +                                              const char *value_name,
>> +                                              const char *value_type,
>> +                                              unsigned int minimum_length,
>> +                                              unsigned int *variable_buffer_data_size)
>> +{
>> +    if (buf_end - *buf>= minimum_length&&  !strcmp(*buf, value_name)) {
>> +        *buf += strlen(value_name)+1;
>> +        if (!strcmp(*buf, value_type)) {
>> +            *buf += strlen(value_type)+1;
>> +            *variable_buffer_data_size = get_header_variable_length(buf, buf_end);
>> +            if (!*variable_buffer_data_size)
>> +                av_log(avctx, AV_LOG_ERROR, "Incomplete header\n");
>> +            return 1;
>
> AVERROR_INVALIDDATA;
>

Fixed.

>> +        }
>> +        *buf -= strlen(value_name)+1;
>> +        av_log(avctx, AV_LOG_ERROR, "Unknown data type for header variable %s\n", value_name);
>> +    }
>> +    return 0;
>> +}
>> +
>> +static int decode_frame(AVCodecContext *avctx,
>> +                        void *data,
>> +                        int *data_size,
>> +                        AVPacket *avpkt)
>> +{
>> +    const uint8_t *buf      = avpkt->data;
>> +    unsigned int   buf_size = avpkt->size;
>> +    const uint8_t *buf_end  = buf + buf_size;
>> +
>> +    EXRContext *const s = avctx->priv_data;
>> +    AVFrame *picture  = data;
>> +    AVFrame *const p =&s->picture;
>> +    uint8_t *ptr;
>> +
>> +    int x, y, stride, magic_number, version_flag;
>> +    int w = 0;
>> +    int h = 0;
>> +    unsigned int xmin   = ~0;
>> +    unsigned int xmax   = ~0;
>> +    unsigned int ymin   = ~0;
>> +    unsigned int ymax   = ~0;
>> +    unsigned int xdelta = ~0;
>> +
>> +    unsigned int current_channel_offset = 0;
>> +
>> +    s->channel_offsets[0] = -1;
>> +    s->channel_offsets[1] = -1;
>> +    s->channel_offsets[2] = -1;
>> +    s->bits_per_color_id = -1;
>> +
>
>> +    magic_number = bytestream_get_le32(&buf);
>> +    if (magic_number != 20000630) { // As per documentation of OpenEXR it's supposed to be int 20000630 little-endian
>> +        av_log(avctx, AV_LOG_ERROR, "Wrong magic number %d\n", magic_number);
>> +        return -1;
>> +    }
>> +
>> +    version_flag = bytestream_get_le32(&buf);
>> +    if ((version_flag&  0x200) == 0x200) {
>> +        av_log(avctx, AV_LOG_ERROR, "Tile based images are not supported\n");
>> +        return -1;
>> +    }
>> +
>> +    if (buf_end - buf<  10) {
>> +        av_log(avctx, AV_LOG_ERROR, "Too short header to parse\n");
>> +        return -1;
>
> Please use meaningful AVERROR_ codes here and in the rest of the file.
>

Should be fixed now. Have converted all which were returning to AVERROR_ 
codes.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: openEXR-rev21.diff
Type: text/x-patch
Size: 19907 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100427/83c39fbb/attachment.bin>



More information about the ffmpeg-devel mailing list