1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-27 09:37:34 +00:00

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.
This commit is contained in:
Nico Weber 2024-01-29 12:49:22 -05:00 committed by Andreas Kling
parent fb2166f19c
commit 69964e10f4
3 changed files with 14 additions and 3 deletions

View file

@ -324,6 +324,15 @@ TEST_CASE(test_jpeg_sof0_several_scans)
TRY_OR_FAIL(expect_single_frame_of_size(*plugin_decoder, { 592, 800 })); 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) TEST_CASE(test_jpeg_rgb_components)
{ {
auto file = TRY_OR_FAIL(Core::MappedFile::map(TEST_INPUT("jpg/rgb_components.jpg"sv))); auto file = TRY_OR_FAIL(Core::MappedFile::map(TEST_INPUT("jpg/rgb_components.jpg"sv)));

Binary file not shown.

After

Width:  |  Height:  |  Size: 1.1 KiB

View file

@ -798,12 +798,14 @@ static ErrorOr<void> decode_huffman_stream(JPEGLoadingContext& context, Vector<M
{ {
for (u32 vcursor = 0; vcursor < context.mblock_meta.vcount; vcursor += context.sampling_factors.vertical) { for (u32 vcursor = 0; vcursor < context.mblock_meta.vcount; vcursor += context.sampling_factors.vertical) {
for (u32 hcursor = 0; hcursor < context.mblock_meta.hcount; hcursor += context.sampling_factors.horizontal) { for (u32 hcursor = 0; hcursor < context.mblock_meta.hcount; hcursor += context.sampling_factors.horizontal) {
u32 i = vcursor * context.mblock_meta.hpadded_count + hcursor; // FIXME: This is likely wrong for non-interleaved scans.
VERIFY(context.mblock_meta.hpadded_count % context.sampling_factors.horizontal == 0);
u32 number_of_mcus_decoded_so_far = ((vcursor / context.sampling_factors.vertical) * context.mblock_meta.hpadded_count + hcursor) / context.sampling_factors.horizontal;
auto& huffman_stream = context.current_scan->huffman_stream; auto& huffman_stream = context.current_scan->huffman_stream;
if (context.dc_restart_interval > 0) { 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); reset_decoder(context);
// Restart markers are stored in byte boundaries. Advance the huffman stream cursor to // Restart markers are stored in byte boundaries. Advance the huffman stream cursor to
@ -823,7 +825,7 @@ static ErrorOr<void> decode_huffman_stream(JPEGLoadingContext& context, Vector<M
if (result.is_error()) { if (result.is_error()) {
if constexpr (JPEG_DEBUG) { if constexpr (JPEG_DEBUG) {
dbgln("Failed to build Macroblock {}: {}", i, result.error()); dbgln("Failed to build Macroblock {}: {}", number_of_mcus_decoded_so_far, result.error());
dbgln("Huffman stream byte offset {}", context.stream.byte_offset()); dbgln("Huffman stream byte offset {}", context.stream.byte_offset());
} }
return result.release_error(); return result.release_error();