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

[Jimmy Christensen]

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.

-- 
Best Regards
Jimmy Christensen
Developer
Ghost A/S
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenEXR-rev13.diff
Type: text/x-patch
Size: 20972 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090908/35724f93/attachment.bin>