amitkumar3968 / openjpeg

Automatically exported from code.google.com/p/openjpeg
Other
0 stars 0 forks source link

No way to debug opj_tcd_init_encode_tile or opj_tcd_init_decode_tile #433

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The macro used for those functions prevents proper debugging.

I propose the following patch replacing the macro by a function which will 
allow debugging (& also syntax highlighting, code completion,... for various 
IDE)

The patch as been verified against the whole test suit with no regressions

Original issue reported on code.google.com by m.darb...@gmail.com on 15 Nov 2014 at 1:44

GoogleCodeExporter commented 9 years ago

Original comment by m.darb...@gmail.com on 15 Nov 2014 at 1:45

Attachments:

GoogleCodeExporter commented 9 years ago
@Mathieu Malaterre: could you have a look to this one ? Is there any drawback 
of replacing a MACRO with a (long) INLINE function ? Thks

Original comment by anto...@gmail.com on 18 Nov 2014 at 11:22

GoogleCodeExporter commented 9 years ago
technically since openjpeg is in C only, it is pretty straightforward to create 
a compatible C file, see comments:

https://code.google.com/p/openjpeg/source/detail?r=2571&path=/trunk/src/lib/open
jp2/tcd.c

I believe VS (maybe others) do not support inline keyword (neither public not 
private extension), since this is C99 (not C89).

Feels like a cosmetic patch though, if there is an underlying bug, then feel 
free to apply if this helps debugging. Reducing priority.

Original comment by mathieu.malaterre on 19 Nov 2014 at 7:53

GoogleCodeExporter commented 9 years ago
@mathieu,

This indeed helps debugging (cosmetic would be my last worry right now - some 
new foxit issues are somewhere in those functions & step-by-step debugging 
while not necessary is certainly much appreciated). There's little code 
duplication compared to the macro version  (it's in the inner loop) which could 
be eliminated but I think there's more performance risk doing this (as 
code_block for encode/decode shall share the beginning of their definition to 
avoid code duplication -> member reorder == potential performance penalty).
The inline function uses the INLINE keyword from openjpeg.h which is well 
defined for VS (if you mean Visual Studio). Even if the function doesn't get 
inlined, I guess that the performance penalty will be near to nil.

Original comment by m.darb...@gmail.com on 19 Nov 2014 at 7:07

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r2931.

Original comment by m.darb...@gmail.com on 19 Nov 2014 at 7:08

GoogleCodeExporter commented 9 years ago
Matthieu: thank you for getting rid of those horrible macros! 

Original comment by boxe...@gmail.com on 15 Dec 2014 at 4:59