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

feat(decode): add tolerantDecoding option #57

Merged
merged 3 commits into from Apr 21, 2020
Merged

Conversation

Dewep
Copy link
Contributor

@Dewep Dewep commented Jan 7, 2020

closes #56

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

thanks very much for raising this and fixing @Dewep!

is the way you arrived at a broken JPEG a reproducible process we could go through with a public image to have a test for this?

lib/decoder.js Outdated Show resolved Hide resolved
lib/decoder.js Outdated Show resolved Hide resolved
lib/decoder.js Outdated
@@ -980,15 +984,18 @@ module.exports = decode;
function decode(jpegData, opts) {
var defaultOpts = {
useTArray: false,
colorTransform: true
colorTransform: true,
tolerantDecoding: true
Copy link
Collaborator

Choose a reason for hiding this comment

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

to avoid any future breaking change based on tolerantDecoding, how do you feel about this being off by default?

@Dewep
Copy link
Contributor Author

Dewep commented Jan 7, 2020

I will have enabled the default option since this is the behavior of other readers, otherwise the lib will crash in all cases. But I understand your hesitation, it is not disturbing to turn it off.

I changed the default value, and made the small corrections you suggested.

I've searched the internet for images that would have the same issue, without success. And I'm not able to make this picture myself (I clearly don't know enough JPEG for that). The private images in my possession that have this concern all seem to have a name in the form SKMBT_C22419122709450_0018.jpg. I did find some public images with a similar name, but they all work.

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

LGTM thanks @Dewep!

@patrickhulce patrickhulce merged commit 69f6415 into jpeg-js:master Apr 21, 2020
zed-0xff pushed a commit to zed-0xff/jpeg-js that referenced this pull request Feb 24, 2023
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.

DecodeBlock: Cannot read property '0' of undefined
2 participants