-
Notifications
You must be signed in to change notification settings - Fork 7k
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
Added support for EXIF orientation transform in read_image for JPEG #8279
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/8279
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit af933cb with merge base bca7a49 (): This comment was automatically generated by Dr. CI and updates every 15 minutes. |
…n in the op registration
a7acc1c
to
4401c2e
Compare
torchvision/csrc/io/image/image.cpp
Outdated
@@ -23,11 +23,11 @@ static auto registry = | |||
torch::RegisterOperators() | |||
.op("image::decode_png", &decode_png) | |||
.op("image::encode_png", &encode_png) | |||
.op("image::decode_jpeg", &decode_jpeg) | |||
.op("image::decode_jpeg(Tensor data, int mode, bool apply_exif_orientation=False) -> Tensor", &decode_jpeg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This helps to make the op BC compatible, otherwise apply_exif_orientation
becomes required and thus break the old code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a ton for working on this @vfdev-5 ! I made a few comments / questions but this looks great
size_t num_entry = get_uint16(exif_data_vec, endianness, offset); | ||
offset += 2; // go to start of tag fields | ||
constexpr size_t tiff_field_size = 12; | ||
for (size_t entry = 0; entry < num_entry; entry++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any risk that this loop can segfault/overflow if the entries are malformed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_uint16 can return -1 in case offset + required size for uint16 go outside the actual buffer size, so we can catch it here and return earlier.
I hope the code is safe but I can't say 100% sure. I have to investigate a bit more.
855d5de
to
5314d43
Compare
5314d43
to
44f4d81
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @vfdev-5 , some minor comments but LGTM
Hey @vfdev-5! You merged this PR, but no labels were added. |
…or JPEG (#8279) Reviewed By: vmoens Differential Revision: D55062774 fbshipit-source-id: 6a75321d8f86630d54cd15ad7433f71d0735659d
Fixes #7977
TODO:
Notes:
cc @gau-nernst @kero-ly