Further jpeg_skip_scanlines() fixes

- Introduce a partial image decompression regression test script that
  validates the correctness of jpeg_skip_scanlines() and
  jpeg_crop_scanlines() for a variety of cropping regions and libjpeg
  settings.

  This regression test catches the following issues:
  #182, fixed in 5bc43c7821
  #237, fixed in 6e95c08649794f5018608f37250026a45ead2db8
  #244, fixed in 398c1e9acc
  #441, fully fixed in this commit

  It does not catch the following issues:
  #194, fixed in 773040f9d9
  #244 (additional segfault), fixed in
       9120a24743

- Modify the libjpeg-turbo regression test suite (make test) so that it
  checks for the issue reported in #441 (segfault in
  jpeg_skip_scanlines() when used with 4:2:0 merged upsampling/color
  conversion.)

- Fix issues in jpeg_skip_scanlines() that caused incorrect output with
  h2v2 (4:2:0) merged upsampling/color conversion.  The previous commit
  fixed the segfault reported in #441, but that was a symptom of a
  larger problem.  Because merged 4:2:0 upsampling uses a "spare row"
  buffer, it is necessary to allow the upsampler to run when skipping
  rows (fancy 4:2:0 upsampling, which uses context rows, also requires
  this.)  Otherwise, if skipping starts at an odd-numbered row, the
  output image will be incorrect.

- Throw an error if jpeg_skip_scanlines() is called with two-pass color
  quantization enabled.  With two-pass color quantization, the first
  pass occurs within jpeg_start_decompress(), so subsequent calls to
  jpeg_skip_scanlines() interfere with the multipass state and prevent
  the second pass from occurring during subsequent calls to
  jpeg_read_scanlines().

CMakeLists.txt

@@ -772,7 +772,7 @@ else()
   set(MD5_PPM_3x2_IFAST fd283664b3b49127984af0a7f118fccd)
   set(MD5_JPEG_420_ISLOW_ARI e986fb0a637a8d833d96e8a6d6d84ea1)
   set(MD5_JPEG_444_ISLOW_PROGARI 0a8f1c8f66e113c3cf635df0a475a617)
-  set(MD5_PPM_420M_IFAST_ARI 72b59a99bcf1de24c5b27d151bde2437)
+  set(MD5_PPM_420M_IFAST_ARI 57251da28a35b46eecb7177d82d10e0e)
   set(MD5_JPEG_420_ISLOW 9a68f56bc76e466aa7e52f415d0f4a5f)
   set(MD5_PPM_420M_ISLOW_2_1 9f9de8c0612f8d06869b960b05abf9c9)
   set(MD5_PPM_420M_ISLOW_15_8 b6875bc070720b899566cc06459b63b7)
@@ -1163,7 +1163,7 @@ foreach(libtype ${TEST_LIBTYPES})
 
   if(WITH_ARITH_DEC)
     # CC: RGB->YCC  SAMP: h2v2 merged  IDCT: ifast  ENT: arith
-    add_bittest(djpeg 420m-ifast-ari "-fast;-ppm"
+    add_bittest(djpeg 420m-ifast-ari "-fast;-skip;1,20;-ppm"
       testout_420m_ifast_ari.ppm ${TESTIMAGES}/testimgari.jpg
       ${MD5_PPM_420M_IFAST_ARI})
 
@@ -1299,6 +1299,11 @@ endforeach()
 add_custom_target(testclean COMMAND ${CMAKE_COMMAND} -P
   ${CMAKE_CURRENT_SOURCE_DIR}/cmakescripts/testclean.cmake)
 
+configure_file(croptest.in croptest @ONLY)
+add_custom_target(croptest
+  COMMAND echo croptest
+  COMMAND ${BASH} ${CMAKE_CURRENT_BINARY_DIR}/croptest)
+
 if(WITH_TURBOJPEG)
   configure_file(tjbenchtest.in tjbenchtest @ONLY)
   configure_file(tjexampletest.in tjexampletest @ONLY)

ChangeLog.md

@@ -7,11 +7,16 @@
 platforms when using any of the YUV encoding/compression/decompression/decoding
 methods in the TurboJPEG Java API.
 
-2. Fixed segfaults or "Corrupt JPEG data: premature end of data segment" errors
+2. Fixed or worked around multiple issues with `jpeg_skip_scanlines()`:
-in `jpeg_skip_scanlines()` that occurred when decompressing 4:2:2 or 4:2:0 JPEG
+
-images using the merged (non-fancy) upsampling algorithms (that is, when
+     - Fixed segfaults or "Corrupt JPEG data: premature end of data segment"
-setting `cinfo.do_fancy_upsampling` to `FALSE`.)  2.0.0[6] was a similar fix,
+errors in `jpeg_skip_scanlines()` that occurred when decompressing 4:2:2 or
-but it did not cover all cases.
+4:2:0 JPEG images using merged (non-fancy) upsampling/color conversion (that
+is, when setting `cinfo.do_fancy_upsampling` to `FALSE`.)  2.0.0[6] was a
+similar fix, but it did not cover all cases.
+     - `jpeg_skip_scanlines()` now throws an error if two-pass color
+quantization is enabled.  Two-pass color quantization never worked properly
+with `jpeg_skip_scanlines()`, and the issues could not readily be fixed.
 
 
 2.0.5

croptest.in

@@ -0,0 +1,95 @@
+#!/bin/bash
+
+set -u
+set -e
+trap onexit INT
+trap onexit TERM
+trap onexit EXIT
+
+onexit()
+{
+	if [ -d $OUTDIR ]; then
+		rm -rf $OUTDIR
+	fi
+}
+
+runme()
+{
+	echo \*\*\* $*
+	$*
+}
+
+IMAGE=vgl_6548_0026a.bmp
+WIDTH=128
+HEIGHT=95
+IMGDIR=@CMAKE_CURRENT_SOURCE_DIR@/testimages
+OUTDIR=`mktemp -d /tmp/__croptest_output.XXXXXX`
+EXEDIR=@CMAKE_CURRENT_BINARY_DIR@
+
+if [ -d $OUTDIR ]; then
+	rm -rf $OUTDIR
+fi
+mkdir -p $OUTDIR
+
+exec >$EXEDIR/croptest.log
+
+echo "============================================================"
+echo "$IMAGE ($WIDTH x $HEIGHT)"
+echo "============================================================"
+echo
+
+for PROGARG in "" -progressive; do
+
+	cp $IMGDIR/$IMAGE $OUTDIR
+	basename=`basename $IMAGE .bmp`
+	echo "------------------------------------------------------------"
+	echo "Generating test images"
+	echo "------------------------------------------------------------"
+	echo
+	runme $EXEDIR/cjpeg $PROGARG -grayscale -outfile $OUTDIR/${basename}_GRAY.jpg $IMGDIR/${basename}.bmp
+	runme $EXEDIR/cjpeg $PROGARG -sample 2x2 -outfile $OUTDIR/${basename}_420.jpg $IMGDIR/${basename}.bmp
+	runme $EXEDIR/cjpeg $PROGARG -sample 2x1 -outfile $OUTDIR/${basename}_422.jpg $IMGDIR/${basename}.bmp
+	runme $EXEDIR/cjpeg $PROGARG -sample 1x2 -outfile $OUTDIR/${basename}_440.jpg $IMGDIR/${basename}.bmp
+	runme $EXEDIR/cjpeg $PROGARG -sample 1x1 -outfile $OUTDIR/${basename}_444.jpg $IMGDIR/${basename}.bmp
+	echo
+
+	for NSARG in "" -nosmooth; do
+
+		for COLORSARG in "" "-colors 256 -dither none -onepass"; do
+
+			for Y in {0..16}; do
+
+				for H in {1..16}; do
+
+					X=$(( (Y*16)%128 ))
+					W=$(( WIDTH-X-7 ))
+					if [ $Y -le 15 ]; then
+						CROPSPEC="${W}x${H}+${X}+${Y}"
+					else
+						Y2=$(( HEIGHT-H ));
+						CROPSPEC="${W}x${H}+${X}+${Y2}"
+					fi
+
+					echo "------------------------------------------------------------"
+					echo $PROGARG $NSARG $COLORSARG -crop $CROPSPEC
+					echo "------------------------------------------------------------"
+					echo
+					for samp in GRAY 420 422 440 444; do
+						$EXEDIR/djpeg $NSARG $COLORSARG -rgb -outfile $OUTDIR/${basename}_${samp}_full.ppm $OUTDIR/${basename}_${samp}.jpg
+						convert -crop $CROPSPEC $OUTDIR/${basename}_${samp}_full.ppm $OUTDIR/${basename}_${samp}_ref.ppm
+						runme $EXEDIR/djpeg $NSARG $COLORSARG -crop $CROPSPEC -rgb -outfile $OUTDIR/${basename}_${samp}.ppm $OUTDIR/${basename}_${samp}.jpg
+						runme cmp $OUTDIR/${basename}_${samp}.ppm $OUTDIR/${basename}_${samp}_ref.ppm
+					done
+					echo
+
+				done
+
+			done
+
+		done
+
+	done
+
+done
+
+echo SUCCESS!

jdapistd.c

@@ -306,16 +306,6 @@ noop_quantize(j_decompress_ptr cinfo, JSAMPARRAY input_buf,
 }
 
 
-/* Dummy postprocessing function used by jpeg_skip_scanlines() */
-LOCAL(void)
-noop_post_process (j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
-                   JDIMENSION *in_row_group_ctr,
-                   JDIMENSION in_row_groups_avail, JSAMPARRAY output_buf,
-                   JDIMENSION *out_row_ctr, JDIMENSION out_rows_avail)
-{
-}
-
-
 /*
  * In some cases, it is best to call jpeg_read_scanlines() and discard the
  * output, rather than skipping the scanlines, because this allows us to
@@ -329,16 +319,12 @@ read_and_discard_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
 {
   JDIMENSION n;
   my_master_ptr master = (my_master_ptr)cinfo->master;
+  JSAMPARRAY scanlines = NULL;
   void (*color_convert) (j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
                          JDIMENSION input_row, JSAMPARRAY output_buf,
                          int num_rows) = NULL;
   void (*color_quantize) (j_decompress_ptr cinfo, JSAMPARRAY input_buf,
                           JSAMPARRAY output_buf, int num_rows) = NULL;
-  void (*post_process_data) (j_decompress_ptr cinfo, JSAMPIMAGE input_buf,
-                             JDIMENSION *in_row_group_ctr,
-                             JDIMENSION in_row_groups_avail,
-                             JSAMPARRAY output_buf, JDIMENSION *out_row_ctr,
-                             JDIMENSION out_rows_avail) = NULL;
 
   if (cinfo->cconvert && cinfo->cconvert->color_convert) {
     color_convert = cinfo->cconvert->color_convert;
@@ -350,23 +336,19 @@ read_and_discard_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
     cinfo->cquantize->color_quantize = noop_quantize;
   }
 
-  if (master->using_merged_upsample && cinfo->post &&
+  if (master->using_merged_upsample && cinfo->max_v_samp_factor == 2) {
-      cinfo->post->post_process_data) {
+    my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample;
-    post_process_data = cinfo->post->post_process_data;
+    scanlines = &upsample->spare_row;
-    cinfo->post->post_process_data = noop_post_process;
   }
 
   for (n = 0; n < num_lines; n++)
-    jpeg_read_scanlines(cinfo, NULL, 1);
+    jpeg_read_scanlines(cinfo, scanlines, 1);
 
   if (color_convert)
     cinfo->cconvert->color_convert = color_convert;
 
   if (color_quantize)
     cinfo->cquantize->color_quantize = color_quantize;
-
-  if (post_process_data)
-    cinfo->post->post_process_data = post_process_data;
 }
 
 
@@ -380,6 +362,12 @@ increment_simple_rowgroup_ctr(j_decompress_ptr cinfo, JDIMENSION rows)
 {
   JDIMENSION rows_left;
   my_main_ptr main_ptr = (my_main_ptr)cinfo->main;
+  my_master_ptr master = (my_master_ptr)cinfo->master;
+
+  if (master->using_merged_upsample && cinfo->max_v_samp_factor == 2) {
+    read_and_discard_scanlines(cinfo, rows);
+    return;
+  }
 
   /* Increment the counter to the next row group after the skipped rows. */
   main_ptr->rowgroup_ctr += rows / cinfo->max_v_samp_factor;
@@ -410,11 +398,16 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
   my_main_ptr main_ptr = (my_main_ptr)cinfo->main;
   my_coef_ptr coef = (my_coef_ptr)cinfo->coef;
   my_master_ptr master = (my_master_ptr)cinfo->master;
+  my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
   JDIMENSION i, x;
   int y;
   JDIMENSION lines_per_iMCU_row, lines_left_in_iMCU_row, lines_after_iMCU_row;
   JDIMENSION lines_to_skip, lines_to_read;
 
+  /* Two-pass color quantization is not supported. */
+  if (cinfo->quantize_colors && cinfo->two_pass_quantize)
+    ERREXIT(cinfo, JERR_NOTIMPL);
+
   if (cinfo->global_state != DSTATE_SCANNING)
     ERREXIT1(cinfo, JERR_BAD_STATE, cinfo->global_state);
 
@@ -472,13 +465,7 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
     main_ptr->buffer_full = FALSE;
     main_ptr->rowgroup_ctr = 0;
     main_ptr->context_state = CTX_PREPARE_FOR_IMCU;
-    if (master->using_merged_upsample) {
+    if (!master->using_merged_upsample) {
-      my_merged_upsample_ptr upsample =
-        (my_merged_upsample_ptr)cinfo->upsample;
-      upsample->spare_full = FALSE;
-      upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
-    } else {
-      my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
       upsample->next_row_out = cinfo->max_v_samp_factor;
       upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
     }
@@ -493,13 +480,7 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
       cinfo->output_scanline += lines_left_in_iMCU_row;
       main_ptr->buffer_full = FALSE;
       main_ptr->rowgroup_ctr = 0;
-      if (master->using_merged_upsample) {
+      if (!master->using_merged_upsample) {
-        my_merged_upsample_ptr upsample =
-          (my_merged_upsample_ptr)cinfo->upsample;
-        upsample->spare_full = FALSE;
-        upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
-      } else {
-        my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
         upsample->next_row_out = cinfo->max_v_samp_factor;
         upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
       }
@@ -537,14 +518,8 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
       cinfo->output_iMCU_row += lines_to_skip / lines_per_iMCU_row;
       increment_simple_rowgroup_ctr(cinfo, lines_to_read);
     }
-    if (master->using_merged_upsample) {
+    if (!master->using_merged_upsample)
-      my_merged_upsample_ptr upsample =
-        (my_merged_upsample_ptr)cinfo->upsample;
       upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
-    } else {
-      my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
-      upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
-    }
     return num_lines;
   }
 
@@ -585,13 +560,8 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
    * bit odd, since "rows_to_go" seems to be redundantly keeping track of
    * output_scanline.
    */
-  if (master->using_merged_upsample) {
+  if (!master->using_merged_upsample)
-    my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample;
     upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
-  } else {
-    my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample;
-    upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline;
-  }
 
   /* Always skip the requested number of lines. */
   return num_lines;

libjpeg.txt

@@ -3,7 +3,7 @@ USING THE IJG JPEG LIBRARY
 This file was part of the Independent JPEG Group's software:
 Copyright (C) 1994-2013, Thomas G. Lane, Guido Vollbeding.
 libjpeg-turbo Modifications:
-Copyright (C) 2010, 2014-2018, D. R. Commander.
+Copyright (C) 2010, 2014-2018, 2020, D. R. Commander.
 Copyright (C) 2015, Google, Inc.
 For conditions of distribution and use, see the accompanying README.ijg file.
 
@@ -750,7 +750,9 @@ multiple rows in the JPEG image.
 
 Suspending data sources are not supported by this function.  Calling
 jpeg_skip_scanlines() with a suspending data source will result in undefined
-behavior.
+behavior.  Two-pass color quantization is also not supported by this function.
+Calling jpeg_skip_scanlines() with two-pass color quantization enabled will
+result in an error.
 
 jpeg_skip_scanlines() will not allow skipping past the bottom of the image.  If
 the value of num_lines is large enough to skip past the bottom of the image,