Skip to content

Raise proper error when deconding 16-bits jpegs #4101

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

Merged
merged 2 commits into from
Jun 24, 2021

Conversation

cyyever
Copy link
Contributor

@cyyever cyyever commented Jun 23, 2021

After some investigation, I found that #3613 failed because the image is 16 bit. However our decoding code assumes the image is 8-bit. So our allocated tensor size is too small to hold the decoded data and leads to memory overflow.

However, I can't find an easy way to decode 16-bit images without breaking the current decoding logic(we would have to change too many code places). So I simply issue an error.

Note: In order to support 16-bit png images, we should allocate the tensor with torch::kI16 and change

auto datap = data.accessor<unsigned char, 1>().data();

and

auto ptr = tensor.accessor<uint8_t, 3>().data();

and the logic of read_callback. And although #4051 didn't fix this bug, I still think it's useful..

@NicolasHug NicolasHug changed the title fix 3613 Raise proper error when deconding 16-bits jpegs Jun 23, 2021
Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks @cyyever , LGTM!

I can confirm that the offending 16-bits jpeg now fails with the more informative error message rather than a segfault.

@fmassa should we open an issue to keep track of the 16-bits support, or is this not something we want?

@cyyever
Copy link
Contributor Author

cyyever commented Jun 23, 2021

And there may be 32-bit or 64-bit png images. And I'm not sure the case of jpg.

@NicolasHug NicolasHug merged commit 9596668 into pytorch:master Jun 24, 2021
@NicolasHug
Copy link
Member

Thanks @cyyever !

@fmassa
Copy link
Member

fmassa commented Jun 24, 2021

Thanks for the fix @cyyever !

cyyever added a commit to cyyever/vision that referenced this pull request Jun 24, 2021
facebook-github-bot pushed a commit that referenced this pull request Jun 25, 2021
Reviewed By: NicolasHug

Differential Revision: D29369895

fbshipit-source-id: c0203152ccf006578c70e1e9201bb168089ff938
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants