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

[Jimmy Christensen]

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.

>>> Pointless () around buf_end - buf, also I still think this check is duplicated
>>> all over the place and could be factored out at worst by using a macro.
>>
>> AFAIK Michael Niedermayer hates macros :)
>
> I don't think so at all. But macros avoid code duplication only at the source
> level, not the code level and have some usability issues that make a static inline
> function often preferable over a macro (or for non-speed-critical code like here
> even without the inline).

Will make macros of these then. Perhaps hate was a strong word :)