From 9b704f96b2dccc54363ad7a2fe8e378fc1a2893b Mon Sep 17 00:00:00 2001 From: DRC Date: Tue, 15 Aug 2023 11:03:57 -0400 Subject: [PATCH] Fix block smoothing w/vert.-subsampled prog. JPEGs The 5x5 interblock smoothing implementation, introduced in libjpeg-turbo 2.1, improperly extended the logic from the traditional 3x3 smoothing implementation. Both implementations point prev_block_row and next_block_row to the current block row when processing, respectively, the first and the last block row in the image: if (block_row > 0 || cinfo->output_iMCU_row > 0) prev_block_row = buffer[block_row - 1] + cinfo->master->first_MCU_col[ci]; else prev_block_row = buffer_ptr; if (block_row < block_rows - 1 || cinfo->output_iMCU_row < last_iMCU_row) next_block_row = buffer[block_row + 1] + cinfo->master->first_MCU_col[ci]; else next_block_row = buffer_ptr; 6d91e950c871103a11bac2f10c63bf998796c719 naively extended that logic to accommodate a 5x5 smoothing window: if (block_row > 1 || cinfo->output_iMCU_row > 1) prev_prev_block_row = buffer[block_row - 2] + cinfo->master->first_MCU_col[ci]; else prev_prev_block_row = prev_block_row; if (block_row < block_rows - 2 || cinfo->output_iMCU_row + 1 < last_iMCU_row) next_next_block_row = buffer[block_row + 2] + cinfo->master->first_MCU_col[ci]; else next_next_block_row = next_block_row; However, this new logic was only correct if block_rows == 1, so the values of prev_prev_block_row and next_next_block_row were incorrect when processing, respectively, the second and second to last iMCU rows in a vertically-subsampled progressive JPEG image. The intent was to: - point prev_block_row to the current block row when processing the first block row in the image, - point prev_prev_block_row to prev_block_row when processing the first two block rows in the image, - point next_block_row to the current block row when processing the last block row in the image, and - point next_next_block_row to next_block_row when processing the last two block rows in the image. This commit modifies decompress_smooth_data() so that it computes the current block row's position relative to the whole image and sets the block row pointers based on that value. This commit also restores a line of code that was accidentally deleted by 6d91e950c871103a11bac2f10c63bf998796c719: access_rows += compptr->v_samp_factor; /* prior iMCU row too */ access_rows is merely a sanity check that tells the access_virt_barray() method to generate an error if accessing the specified number of rows would cause a buffer overrun. Essentially, it is a belt-and-suspenders measure to ensure that j*init_d_coef_controller() allocated enough rows for the full-image virtual array. Thus, excluding that line of code did not cause an observable issue. This commit also documents dbae59281fdc6b3a6304a40134e8576d50d662c0 in the change log. Fixes #721 --- ChangeLog.md | 5 +++++ jdcoefct.c | 17 ++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 8bccc1aa..2f3ebd54 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -7,6 +7,11 @@ epilogue so that debuggers and profilers can reliably capture backtraces from within the functions. +2. Fixed two minor issues in the interblock smoothing algorithm that caused +mathematical (but not necessarily perceptible) edge block errors when +decompressing progressive JPEG images exactly two MCU blocks in width or that +use vertical chrominance subsampling. + 3.0.0 ===== diff --git a/jdcoefct.c b/jdcoefct.c index e68f6845..40ce2725 100644 --- a/jdcoefct.c +++ b/jdcoefct.c @@ -5,7 +5,7 @@ * Copyright (C) 1994-1997, Thomas G. Lane. * libjpeg-turbo Modifications: * Copyright 2009 Pierre Ossman for Cendio AB - * Copyright (C) 2010, 2015-2016, 2019-2020, 2022, D. R. Commander. + * Copyright (C) 2010, 2015-2016, 2019-2020, 2022-2023, D. R. Commander. * Copyright (C) 2015, 2020, Google, Inc. * For conditions of distribution and use, see the accompanying README.ijg * file. @@ -431,7 +431,8 @@ decompress_smooth_data(j_decompress_ptr cinfo, _JSAMPIMAGE output_buf) my_coef_ptr coef = (my_coef_ptr)cinfo->coef; JDIMENSION last_iMCU_row = cinfo->total_iMCU_rows - 1; JDIMENSION block_num, last_block_column; - int ci, block_row, block_rows, access_rows; + int ci, block_row, block_rows, access_rows, image_block_row, + image_block_rows; JBLOCKARRAY buffer; JBLOCKROW buffer_ptr, prev_prev_block_row, prev_block_row; JBLOCKROW next_block_row, next_next_block_row; @@ -497,6 +498,7 @@ decompress_smooth_data(j_decompress_ptr cinfo, _JSAMPIMAGE output_buf) (JDIMENSION)access_rows, FALSE); buffer += 2 * compptr->v_samp_factor; /* point to current iMCU row */ } else if (cinfo->output_iMCU_row > 0) { + access_rows += compptr->v_samp_factor; /* prior iMCU row too */ buffer = (*cinfo->mem->access_virt_barray) ((j_common_ptr)cinfo, coef->whole_image[ci], (cinfo->output_iMCU_row - 1) * compptr->v_samp_factor, @@ -539,29 +541,30 @@ decompress_smooth_data(j_decompress_ptr cinfo, _JSAMPIMAGE output_buf) inverse_DCT = cinfo->idct->_inverse_DCT[ci]; output_ptr = output_buf[ci]; /* Loop over all DCT blocks to be processed. */ + image_block_rows = block_rows * cinfo->total_iMCU_rows; for (block_row = 0; block_row < block_rows; block_row++) { + image_block_row = cinfo->output_iMCU_row * block_rows + block_row; buffer_ptr = buffer[block_row] + cinfo->master->first_MCU_col[ci]; - if (block_row > 0 || cinfo->output_iMCU_row > 0) + if (image_block_row > 0) prev_block_row = buffer[block_row - 1] + cinfo->master->first_MCU_col[ci]; else prev_block_row = buffer_ptr; - if (block_row > 1 || cinfo->output_iMCU_row > 1) + if (image_block_row > 1) prev_prev_block_row = buffer[block_row - 2] + cinfo->master->first_MCU_col[ci]; else prev_prev_block_row = prev_block_row; - if (block_row < block_rows - 1 || cinfo->output_iMCU_row < last_iMCU_row) + if (image_block_row < image_block_rows - 1) next_block_row = buffer[block_row + 1] + cinfo->master->first_MCU_col[ci]; else next_block_row = buffer_ptr; - if (block_row < block_rows - 2 || - cinfo->output_iMCU_row + 1 < last_iMCU_row) + if (image_block_row < image_block_rows - 2) next_next_block_row = buffer[block_row + 2] + cinfo->master->first_MCU_col[ci]; else