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;

6d91e950c8 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 6d91e950c8:

  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 dbae59281f in
the change log.

Fixes #721
This commit is contained in:
DRC
2023-08-15 11:03:57 -04:00
parent 7b844bfda6
commit 9b704f96b2
2 changed files with 15 additions and 7 deletions

View File

@@ -7,6 +7,11 @@
epilogue so that debuggers and profilers can reliably capture backtraces from epilogue so that debuggers and profilers can reliably capture backtraces from
within the functions. 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 3.0.0
===== =====

View File

@@ -5,7 +5,7 @@
* Copyright (C) 1994-1997, Thomas G. Lane. * Copyright (C) 1994-1997, Thomas G. Lane.
* libjpeg-turbo Modifications: * libjpeg-turbo Modifications:
* Copyright 2009 Pierre Ossman <ossman@cendio.se> for Cendio AB * Copyright 2009 Pierre Ossman <ossman@cendio.se> 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. * Copyright (C) 2015, 2020, Google, Inc.
* For conditions of distribution and use, see the accompanying README.ijg * For conditions of distribution and use, see the accompanying README.ijg
* file. * file.
@@ -431,7 +431,8 @@ decompress_smooth_data(j_decompress_ptr cinfo, _JSAMPIMAGE output_buf)
my_coef_ptr coef = (my_coef_ptr)cinfo->coef; my_coef_ptr coef = (my_coef_ptr)cinfo->coef;
JDIMENSION last_iMCU_row = cinfo->total_iMCU_rows - 1; JDIMENSION last_iMCU_row = cinfo->total_iMCU_rows - 1;
JDIMENSION block_num, last_block_column; 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; JBLOCKARRAY buffer;
JBLOCKROW buffer_ptr, prev_prev_block_row, prev_block_row; JBLOCKROW buffer_ptr, prev_prev_block_row, prev_block_row;
JBLOCKROW next_block_row, next_next_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); (JDIMENSION)access_rows, FALSE);
buffer += 2 * compptr->v_samp_factor; /* point to current iMCU row */ buffer += 2 * compptr->v_samp_factor; /* point to current iMCU row */
} else if (cinfo->output_iMCU_row > 0) { } else if (cinfo->output_iMCU_row > 0) {
access_rows += compptr->v_samp_factor; /* prior iMCU row too */
buffer = (*cinfo->mem->access_virt_barray) buffer = (*cinfo->mem->access_virt_barray)
((j_common_ptr)cinfo, coef->whole_image[ci], ((j_common_ptr)cinfo, coef->whole_image[ci],
(cinfo->output_iMCU_row - 1) * compptr->v_samp_factor, (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]; inverse_DCT = cinfo->idct->_inverse_DCT[ci];
output_ptr = output_buf[ci]; output_ptr = output_buf[ci];
/* Loop over all DCT blocks to be processed. */ /* 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++) { 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]; 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 = prev_block_row =
buffer[block_row - 1] + cinfo->master->first_MCU_col[ci]; buffer[block_row - 1] + cinfo->master->first_MCU_col[ci];
else else
prev_block_row = buffer_ptr; prev_block_row = buffer_ptr;
if (block_row > 1 || cinfo->output_iMCU_row > 1) if (image_block_row > 1)
prev_prev_block_row = prev_prev_block_row =
buffer[block_row - 2] + cinfo->master->first_MCU_col[ci]; buffer[block_row - 2] + cinfo->master->first_MCU_col[ci];
else else
prev_prev_block_row = prev_block_row; 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 = next_block_row =
buffer[block_row + 1] + cinfo->master->first_MCU_col[ci]; buffer[block_row + 1] + cinfo->master->first_MCU_col[ci];
else else
next_block_row = buffer_ptr; next_block_row = buffer_ptr;
if (block_row < block_rows - 2 || if (image_block_row < image_block_rows - 2)
cinfo->output_iMCU_row + 1 < last_iMCU_row)
next_next_block_row = next_next_block_row =
buffer[block_row + 2] + cinfo->master->first_MCU_col[ci]; buffer[block_row + 2] + cinfo->master->first_MCU_col[ci];
else else