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

Merge/aous72 20240802 #149

Closed

Conversation

wayfarer3130
Copy link

This is a straight merge from current upstream as of 2024-08-02
This version appears to be much more stable than earlier versions.

@aous72
Copy link
Owner

aous72 commented Aug 4, 2024

Hi Bill,

Thank you for this PR.

I am putting my thinking/response on the commit; I am assuming that you intend to use the library for WebAssembly.

Kind regards,
Aous.

@@ -74,7 +74,7 @@ namespace ojph {
cur_comp = 0;
cur_line = 0;
cur_tile_row = 0;
resilient = false;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking is that an incomplete file is a catastrophic error.
So the default behavior should be to terminate with an error rather than terminate gracefully.
If the end user accepts this incompleteness, they can use the enable_resilience() function call to enable resilience.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, we do partial decodes all the time for being able to perform streaming decode, and that is enabled by default now, so we always want to allow it.

@@ -1046,7 +1046,8 @@ namespace ojph {
int marker_idx = find_marker(infile, next_markers, 2);
if (marker_idx == -1)
{
OJPH_INFO(0x00030067, "File terminated early");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will get back to you on this one -- I need to check the code.
If you do not want to get any messages, which might be a good idea for WebAssembly, you can use
set_message_level(OJPH_MSG_LEVEL::NO_MSG)

@@ -1,5 +1,5 @@
#!/bin/sh
mkdir -p build
#(cd build && emcmake cmake -DCMAKE_BUILD_TYPE=Debug ..)
(cd build && emcmake cmake ..)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes the default build is debug.
Is that the intention? can you elaborate?

if (file->read(&Rsiz, 2) != 2)
OJPH_ERROR(0x00050043, "error reading SIZ marker");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From this position downwards.
This fine granularity might be useful for a developer; it is of little use to a normal user.
The Rsiz, XSIZ, YSIZ, ... etc. are parameters within the SIZ marker segment. See the following table.

image

@wayfarer3130
Copy link
Author

My apologies - I was trying to merge into a local fork with these changes, not into the master branch.

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

Successfully merging this pull request may close these issues.

3 participants