From 69964e10f46166ccafd0959d796a5476b9d6f516 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 29 Jan 2024 12:49:22 -0500 Subject: [PATCH] LibGfx+Tests: Improve calculation of restart interval JPEGs can store a `restart_interval`, which controls how many minimum coded units (MCUs) apart the stream state resets. This can be used for error correction, decoding parts of a jpeg in parallel, etc. We tried to use u32 i = vcursor * context.mblock_meta.hpadded_count + hcursor; i % (context.dc_restart_interval * context.sampling_factors.vertical * context.sampling_factors.horizontal) == 0 to check if we hit a multiple of an MCU. `hcursor` is the horizontal offset into 8x8 blocks, vcursor the vertical offset, and hpadded_count stores how many 8x8 blocks we have per row, padded to a multiple of the sampling factor. This isn't quite right if hcursor isn't divisible by both the vertical and horizontal sampling factor. Tweak things so that they work. Also rename `i` to `number_of_mcus_decoded_so_far` since that what it is, at least now. For the test case, I converted an existing image to a ppm: Build/lagom/bin/image -o out.ppm \ Tests/LibGfx/test-inputs/jpg/12-bit.jpg Then I resized it to 102x77px in Photoshop and saved it again. Then I turned it into a jpeg like so: path/to/cjpeg \ -outfile Tests/LibGfx/test-inputs/jpg/odd-restart.jpg \ -sample 2x2,1x1,1x1 -quality 5 -restart 3B out.ppm The trick here is to: a) Pick a size that's not divisible by the data size width (8), and that when rounded to a block size (13) still isn't divisible by the subsample factor -- done by picking a width of 102. b) Pick a huffman table that doesn't happen to contain the bit pattern for a restart marker, so that reading a restart marker from the bitstream as data causes a failure (-quality 5 happens to do this) c) Pick a restart interval where we fail to skip it if our calculation is off (-restart 3B) Together with #22987, fixes #22780. --- Tests/LibGfx/TestImageDecoder.cpp | 9 +++++++++ Tests/LibGfx/test-inputs/jpg/odd-restart.jpg | Bin 0 -> 1121 bytes .../Libraries/LibGfx/ImageFormats/JPEGLoader.cpp | 8 +++++--- 3 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 Tests/LibGfx/test-inputs/jpg/odd-restart.jpg diff --git a/Tests/LibGfx/TestImageDecoder.cpp b/Tests/LibGfx/TestImageDecoder.cpp index f0ccfe6170..a6e2b5e9e0 100644 --- a/Tests/LibGfx/TestImageDecoder.cpp +++ b/Tests/LibGfx/TestImageDecoder.cpp @@ -324,6 +324,15 @@ TEST_CASE(test_jpeg_sof0_several_scans) TRY_OR_FAIL(expect_single_frame_of_size(*plugin_decoder, { 592, 800 })); } +TEST_CASE(test_odd_mcu_restart_interval) +{ + auto file = TRY_OR_FAIL(Core::MappedFile::map(TEST_INPUT("jpg/odd-restart.jpg"sv))); + EXPECT(Gfx::JPEGImageDecoderPlugin::sniff(file->bytes())); + auto plugin_decoder = TRY_OR_FAIL(Gfx::JPEGImageDecoderPlugin::create(file->bytes())); + + TRY_OR_FAIL(expect_single_frame_of_size(*plugin_decoder, { 102, 77 })); +} + TEST_CASE(test_jpeg_rgb_components) { auto file = TRY_OR_FAIL(Core::MappedFile::map(TEST_INPUT("jpg/rgb_components.jpg"sv))); diff --git a/Tests/LibGfx/test-inputs/jpg/odd-restart.jpg b/Tests/LibGfx/test-inputs/jpg/odd-restart.jpg new file mode 100644 index 0000000000000000000000000000000000000000..8f1e410a0bd06e80bae4657b58ab2e6e3340d71c GIT binary patch literal 1121 zcmex=@o0Ye@`1w#)IrvTYK3{611g<%zty^rAo z;{-+)h7SyPfbaw3E5~2}~}G|Co}PBABdzv;mVEQv=fl<{8X9Kzapp8q+4A zhyk+#ixaaJa~ktD<|{00ECMWSOgYRJq?U}9tj`ihyAm4yYUx)vzUz#_;hq-f~KCLEZ^u2d*u)Hrb= zhqBYggQ7tfKd2Zd6*X~kiHS={N~x-;YiMejn3|beSXw!|xVpJ}czOkggocGjL`Eg2 zq^6~3WM&nYl$MoOR8}>&w6?W(baqXeJZ0*%=`&`|TC{k{(q+q6tX#Ee^OmjKw(r=v z>(JpNM~@vpaq`rq%U7;myME*5t%r{uKY9A>`HPpYK7RWAZc#u=sIOv0D zQqe^&F%^@CsvkjK1N)3Pk2R6yGq}eP{<_P+!od9h76T76BQOe?1sUuaW_4*SV1i=) z{}&KkhXZq%y&dHLUjz#^IY^p>6qkKjkQv+0BfW5`3g6Zk8JVu|+~c>Vo_cEk|I!SB zs=z?kxmPt6cxJI(oe>x)>7VHDe)r&miv>@US5BHgZNAU!cl~P_w%h-|obRx#pbunphe5WyPLt=L~}x8@mLa z9O}|o=J@{_$O{~)zrVPDKWgWqQ>4+$VQJF6Ys0Uu#1p2vENl`cC;iU+zn)=XxnRzu z%(s%v%Cj=UY*+c_ZI|T_4!WfER$|FyYl~IJRU4g|3OJUodc0mVp=62KntJ5$(pbO` Z4J&wPUYerRJV7&KQ3IF8q6Yi_Hvy}kxpe>l literal 0 HcmV?d00001 diff --git a/Userland/Libraries/LibGfx/ImageFormats/JPEGLoader.cpp b/Userland/Libraries/LibGfx/ImageFormats/JPEGLoader.cpp index efdabd9376..c5c08dd465 100644 --- a/Userland/Libraries/LibGfx/ImageFormats/JPEGLoader.cpp +++ b/Userland/Libraries/LibGfx/ImageFormats/JPEGLoader.cpp @@ -798,12 +798,14 @@ static ErrorOr decode_huffman_stream(JPEGLoadingContext& context, Vectorhuffman_stream; if (context.dc_restart_interval > 0) { - if (i != 0 && i % (context.dc_restart_interval * context.sampling_factors.vertical * context.sampling_factors.horizontal) == 0) { + if (number_of_mcus_decoded_so_far != 0 && number_of_mcus_decoded_so_far % context.dc_restart_interval == 0) { reset_decoder(context); // Restart markers are stored in byte boundaries. Advance the huffman stream cursor to @@ -823,7 +825,7 @@ static ErrorOr decode_huffman_stream(JPEGLoadingContext& context, Vector