Potential fix for #2037 #51
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
wasm generation had difficulty with the use of function pointer for the stream skipping function with BufferedStream.hpp.
This change resolves the wasm issue with the function pointer by 1) replacing all of the function pointers with explicit calls that depend on the type of stream (file or buffer), and 2) using C rather than C++ to match the language of cio. The change also moves BufferStream into openjp2 core functions, which enables the
direct call from cio.c. But in doing so, the change does compromise the abstraction of the stream types away from the core functions. One can view the BufferedStream as a core stream type as rationale for moving it into the core.
cio.c functions are 'C', and so the change makes BufferedStream C rather than C++ by using '.c' rather than '.hpp
The issue only appears with wasm and not C/C++, so the changes does retain the function pointers and their setters for
possible future stream types besides file streams although no known stream types are known a this time.
To minimize changes to the core of cio, there are near-equivalent functions to the function pointers for skip, read, write, and seek. You see this simple replacement for example where p_stream->m_write_fn is replaced with write_stream.
The codecs repo uses submodules which slightly complicates the PR.
This PR will likely capture the repo out of John-Skinner.
The only submodule that needed any change is the openjpeg repo.
So that pull request is integral to the overall change.
For codec, this change updates the build-native.sh to produce an install; but that is optional for this fix.
The packages/openjpeg/test/cpp/CMakeLists.txt is updated for the include directories to go one level up.
I only ran testing on decode from browser javascript.
I can expand my testing with a little guidance where additional tests are.