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

[Jimmy Christensen]

On 2009-09-08 09:20, Jimmy Christensen wrote:
> On 2009-07-29 15:22, Reimar D?ffinger wrote:
>> On Wed, Jul 29, 2009 at 03:09:55PM +0200, Jimmy Christensen wrote:
>>> On 2009-07-29 15:06, Reimar D?ffinger wrote:
>>>> On Wed, Jul 29, 2009 at 02:39:08PM +0200, Jimmy Christensen wrote:
>>>>>> That's what functions are there for. I think in quite a few places
>>>>>> readability
>>>>>> could be improved quite a bit by creating a well named function,
>>>>>> but I think
>>>>>> you so far ignored all my suggestions in that regard.
>>>>>
>>>>> It's not as much as I ignored them. I tried making them into functions
>>>>> but since it needs to exit with a return -1, I didn't know how to make
>>>>> the function make the main function return -1.
>>>>
>>>> E.g.
>>>> If (get_value(...)< 0)
>>>> return -1;
>>>> That still duplicates the if, but should still be less code, at least
>>>> the av_log is centralized.
>>>> You could then consider if its reasonable to wrap even that into a
>>>> macro.
>>>
>>> This was also pretty much the conclusion that I got to. For most of the
>>> functions it wouldn't make the code much less.
>>
>> But it will also give the code a name and a doxygen description.
>> Also, it will help you avoid doing it one way in five place and a
>> different
>> way in a another (as you have done a few times), leaving in a few
>> years everyone
>> scared to change the code because the can't know if that case was done
>> differently
>> on purpose.
>
> Been a long time since I submitted an update on this so here goes :
>
> Re-designed a few things and made more things into functions. Don't know
> what you guys think of the added exr.h header, but I beleive it's the
> right thing to do for eg. future support for compression types.
>

Made some small fixes inside get_rgb_channel()...

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenEXR-rev14.diff
Type: text/x-patch
Size: 21037 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090908/4e6ed5af/attachment.bin>