Add 10-12-14bit (integer) TIFF decoding support by chacha21 · Pull Request #21701 · opencv/opencv

chacha21 · 2022-03-08T15:59:34Z

Merge with extra: opencv/opencv_extra#962

Proposal for #21700

A (slow) unpacking step is inserted when the native bpp is not equal to the dst_bpp

Currently, I do not know if there can be several packing flavours in TIFF data.

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
The PR is proposed to the proper branch
There is a reference to the original bug report and related work

An (slow) unpacking step is inserted when the native bpp is not equal to the dst_bpp Currently, I do not know if there can be several packing flavours in TIFF data.

alalek · 2022-03-10T19:24:37Z

modules/imgcodecs/test/test_tiff.cpp

+    cv::Mat tmp;
+    double diff = 0;
+
+    if (!img16UC1.empty())


.empty() is a test failure.

I was unsure about what to do, because the real test with actual images will fail on the buildbot as long as the sample files #962 are not commited

alalek · 2022-03-10T19:27:37Z

modules/imgcodecs/test/test_tiff.cpp

+    if (!img16UC1.empty())
+        img8UC1.convertTo(tmp, img16UC1.type(), (1<<(16-8)));
+    diff = tmp.empty() ? 0. : cv::norm(tmp.reshape(1), img16UC1.reshape(1), cv::NORM_INF);
+    ASSERT_TRUE(img16UC1.empty() || (diff <= maxDiff));


ASSERT_TRUE with complex checks provides almost useless messages about failure.
Use appropriate EXPECT_LE() macro instead.

alalek · 2022-03-10T19:30:52Z

modules/imgcodecs/src/grfmt_tiff.cpp

-            AutoBuffer<uchar> _buffer(buffer_size);
-            uchar* buffer = _buffer.data();
-            ushort* buffer16 = (ushort*)buffer;
+            const size_t src_buffer_bytes_per_row = ((ncn * tile_width0 * bpp + bitsPerByte - 1)/bitsPerByte);


Use divUp() here

alalek · 2022-03-10T19:36:19Z

modules/imgcodecs/src/grfmt_tiff.cpp

    }
 }

+static void _unpack10To16(const uchar* src, const uchar* srcEnd, ushort* dst, ushort* dstEnd, size_t expectedDstElements)


expectedDstElements

We should have both limits here (dst and src) to ensure avoiding out of buffer access.

This is already the case.
expectedDstElements is the width of the currently decoded tile, without any assumptions about src/dst bounds limits. Then, the fullPacketsCount computation ensures that no OOB will occur for the "fullPackets" loop, while the last loop with "remainingElements" performs explicit bounds checking

alalek · 2022-03-10T19:37:25Z

modules/imgcodecs/src/grfmt_tiff.cpp

+    for(size_t i = 0 ; i<fullPacketsCount ; ++i)
+    {
+        for(size_t j = 0 ; j<srcElementsPerPacket ; ++j)
+          buf.u8[srcElementsPerPacket-1-j] = *src++;


we need access limitation with buffer boundary check.

the min() in fullPacketsCountcomputation ensures that no OOB will occur

alalek · 2022-03-10T19:44:07Z

modules/imgcodecs/test/test_tiff.cpp

    // What about 32, 64 bit?
 }

+TEST(Imgcodecs_Tiff, decode_12)


decode_12
12

Below test cases for 10/14 bits too,

alalek · 2022-03-10T19:46:03Z

modules/imgcodecs/test/test_tiff.cpp

+    cv::Mat img10UC1 = imread(root + "readwrite/pattern_10uc1.tif", cv::IMREAD_UNCHANGED);
+    cv::Mat img10UC3 = imread(root + "readwrite/pattern_10uc3.tif", cv::IMREAD_UNCHANGED);
+    cv::Mat img10UC4 = imread(root + "readwrite/pattern_10uc4.tif", cv::IMREAD_UNCHANGED);
+    cv::Mat img12UC1 = imread(root + "readwrite/pattern_12uc1.tif", cv::IMREAD_UNCHANGED);
+    cv::Mat img12UC3 = imread(root + "readwrite/pattern_12uc3.tif", cv::IMREAD_UNCHANGED);
+    cv::Mat img12UC4 = imread(root + "readwrite/pattern_12uc4.tif", cv::IMREAD_UNCHANGED);


Please interleave reading and checks (group code of dedicated cases).

Add 10-12-14bit (integer) TIFF decoding support * Add 12bit (integer) TIFF decoding support An (slow) unpacking step is inserted when the native bpp is not equal to the dst_bpp Currently, I do not know if there can be several packing flavours in TIFF data. * added tests * move sample files to opencv_extra * added 10b and 14b unpacking * fix compilation for non MSVC compilers by using more standard typedefs * yet another typdef usage change to fix buildbot Mac compilation * fixed unpacking of partial packets * fixed warnings returned by buildbot * modifications as suggested by reviewer

chacha21 added 3 commits March 8, 2022 16:57

Add 12bit (integer) TIFF decoding support

bd63fb1

An (slow) unpacking step is inserted when the native bpp is not equal to the dst_bpp Currently, I do not know if there can be several packing flavours in TIFF data.

added tests

f34f277

move sample files to opencv_extra

0a819e4

chacha21 mentioned this pull request Mar 8, 2022

add sample files for #21701 opencv/opencv_extra#962

Merged

chacha21 added 2 commits March 10, 2022 09:26

added 10b and 14b unpacking

a9b2dec

fix compilation for non MSVC compilers by using more standard typedefs

d303e86

chacha21 changed the title ~~Add 12bit (integer) TIFF decoding support~~ Add 10-12-14bit (integer) TIFF decoding support Mar 10, 2022

chacha21 added 4 commits March 10, 2022 09:37

yet another typdef usage change to fix buildbot Mac compilation

7f8612d

fixed unpacking of partial packets

d0a7a42

fixed warnings returned by buildbot

44eff2c

Merge remote-tracking branch 'upstream/master' into tiff_10_12_14

3358425

alalek reviewed Mar 10, 2022

View reviewed changes

modifications as suggested by reviewer

7d5f567

alalek approved these changes Mar 11, 2022

View reviewed changes

alalek merged commit 6390b50 into opencv:4.x Mar 11, 2022

chacha21 mentioned this pull request Mar 13, 2022

parallelize some core loop of TIFF decoding #21713

Closed

6 tasks

opencv-pushbot mentioned this pull request Apr 23, 2022

(5.x) Merge 4.x #21903

Merged

alalek mentioned this pull request Oct 15, 2022

(4.x) Merge 3.4 #22642

Merged

Uh oh!

Conversation

chacha21 commented Mar 8, 2022 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chacha21 Mar 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chacha21 Mar 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chacha21 commented Mar 8, 2022 •

edited by alalek

Loading

chacha21 Mar 10, 2022 •

edited

Loading

chacha21 Mar 10, 2022 •

edited

Loading