[FFmpeg-devel] [PATCH] OpenEXR decoder rev-13
Tue Sep 8 09:20:59 CEST 2009
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.
>>> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 20972 bytes
Desc: not available
More information about the ffmpeg-devel