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

Thoughts about openjpeg API current issues and potential improvements #1007

Open
rouault opened this issue Aug 28, 2017 · 0 comments
Open

Thoughts about openjpeg API current issues and potential improvements #1007

rouault opened this issue Aug 28, 2017 · 0 comments

Comments

@rouault
Copy link
Collaborator

rouault commented Aug 28, 2017

The OpenJPEG API is confusing in a number of areas

  • the opj_stream_t * parameter is passed together with the opj_codec_t* parameter to a number of functions like opj_read_header(), opj_decode(), opj_get_decoded_tile(), opj_read_tile_header(). If you don't pass the same pair at each occurence, then things will not propely. It would probably wiser to attach once (probably at opj_setup_decoder() time) the stream object and never again
  • similarly the opj_image_t* parameter is returned by opj_read_header(), and then passed to opj_set_decode_area(), opj_decode() and opj_get_decoded_tile() (but not to opj_set_decode_area()). It is unclear if the user is allowed to manually set the values of the fields of the image after opj_read_header() has been called. In practice, it seems that he could do it between opj_read_header() and opj_decode() and that would work, but this is dangerous. A probably better approach would be that the image would be returned just opj_read_header() and be explicitly forbidden to be manually modified by the user (ie bounds, factor or allocating image data). Another advantage of such solution is that it should be possible to simplify j2k.c/j2k.h to avoid having just one opj_image_t* in the opj_j2k_t structure, instead of the 2 confusing members m_private_image and m_output_image, and all the games in recopying their header between them and the opj_image_t* argument of some functions
  • there are several ways of doing the same thing. For example settings parameters.cp_reduce before calling opj_setup_decoder(), or calling opj_set_decoded_resolution_factor(). This makes maintenance and testing of the API much more complex (see opj_set_decoded_resolution_factor(): bad interaction with opj_set_decode_area() and/or opj_decode() #1006 for an example of such issue)
  • opj_decode_tile_data() isn't affected by opj_set_decode_area(). This is not documented currently. Hard to say if it is a feature or a bug.
  • the usefulness of opj_decode_tile_data() is doubtful. It could probably be just replaced by opj_set_decode_area() and opj_decode()
  • if we want to reduce the API surface, opj_get_decoded_tile() (the co-existence of opj_get_decoded_tile() and opj_get_decoded_tile_data() is super confusing by the way) could also probably be just done with opj_set_decode_area() and opj_decode()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant