Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No way to debug opj_tcd_init_encode_tile or opj_tcd_init_decode_tile #433

Closed
gcode-importer opened this issue Nov 15, 2014 · 6 comments
Closed
Assignees

Comments

@gcode-importer
Copy link

Originally reported on Google Code with ID 433

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

Reported by mayeut on 2014-11-15 13:44:59

@gcode-importer
Copy link
Author

Reported by mayeut on 2014-11-15 13:45:44

  • Status changed: Verified

- _Attachment: [issue433.patch](https://storage.googleapis.com/google-code-attachments/openjpeg/issue-433/comment-1/issue433.patch)_

@gcode-importer
Copy link
Author

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

Reported by detonin on 2014-11-18 23:22:24

@gcode-importer
Copy link
Author

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/openjp2/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.

Reported by malaterre on 2014-11-19 07:53:20

  • Labels added: Priority-Low
  • Labels removed: Priority-Medium

@gcode-importer
Copy link
Author

@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.

Reported by mayeut on 2014-11-19 19:07:52

@gcode-importer
Copy link
Author

This issue was closed by revision r2931.

Reported by mayeut on 2014-11-19 19:08:22

  • Status changed: Fixed

@gcode-importer
Copy link
Author

Matthieu: thank you for getting rid of those horrible macros! 

Reported by boxerab on 2014-12-15 04:59:09

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants