[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