2012.07.05 21:54 "[Tiff] Crash in raw_decode with libjpeg 8d (was Re: tiff 4.0.2 raw_decode test failing on ppc64)", by Even Rouault

Bob,

What version of libjpeg are you testing with?

Not speaking for others, but I suspect that old-timers like me still use the old good libjpeg 6b :-)

Nevertheless, I tried with libjpeg 8d and reproduced the same crash too.

diff -uw -U 10 jpeg-6b/libjpeg.doc jpeg-8d/libjpeg.txt shows (among other differences):

"""
The procedure for compression of raw data is basically the same as normal
compression, except that you call jpeg_write_raw_data() in place of
jpeg_write_scanlines(). Before calling jpeg_start_compress(), you must do
the following:

Set cinfo->raw_data_in to TRUE. (It is set FALSE by jpeg_set_defaults().) This notifies the library that you will be supplying raw data. + Furthermore, set cinfo->do_fancy_downsampling to FALSE if you want to use + real downsampled data. (It is set TRUE by jpeg_set_defaults().)
Ensure jpeg_color_space is correct --- an explicit jpeg_set_colorspace() call is a good idea. Note that since color conversion is bypassed, in_color_space is ignored, except that jpeg_set_defaults() uses it to choose the default jpeg_color_space setting.
Ensure the sampling factors, cinfo->comp_info[i].h_samp_factor and cinfo->comp_info[i].v_samp_factor, are correct. Since these indicate the dimensions of the data you are supplying, it's wise to set them explicitly, rather than assuming the library's defaults are what you

want.
"""

I followed the advice and tried the following patch:

$ cvs diff -u -U 10 libtiff/tif_jpeg.c
Index: libtiff/tif_jpeg.c

=================================================================== RCS file: /cvs/maptools/cvsroot/libtiff/libtiff/tif_jpeg.c,v retrieving revision 1.109

diff -u -U 10 -r1.109 tif_jpeg.c

--- libtiff/tif_jpeg.c 3 Jul 2012 00:57:50 -0000 1.109
+++ libtiff/tif_jpeg.c 5 Jul 2012 21:35:31 -0000

@@ -1131,20 +1131,21 @@
                sp->cinfo.d.jpeg_color_space = JCS_UNKNOWN;
                sp->cinfo.d.out_color_space = JCS_UNKNOWN;
                if (td->td_planarconfig == PLANARCONFIG_CONTIG &&
                    (sp->h_sampling != 1 || sp->v_sampling != 1))
                        downsampled_output = TRUE;
                /* XXX what about up-sampling? */
        }
        if (downsampled_output) {
                /* Need to use raw-data interface to libjpeg */
                sp->cinfo.d.raw_data_out = TRUE;
+ sp->cinfo.d.do_fancy_upsampling = FALSE;
                tif->tif_decoderow = DecodeRowError;
                tif->tif_decodestrip = JPEGDecodeRaw;
                tif->tif_decodetile = JPEGDecodeRaw;
        } else {
                /* Use normal interface to libjpeg */
                sp->cinfo.d.raw_data_out = FALSE;
                tif->tif_decoderow = JPEGDecode;
                tif->tif_decodestrip = JPEGDecode;
                tif->tif_decodetile = JPEGDecode;
        }

Which fixes the crash with 8b and make valgrind happy.

The ./raw_decode test still passes fine with jpeg 6b, but generates the following errors with 8b (after commenting the early exit in case of failures)

$ cvs diff -u -U 5 raw_decode.c
Index: raw_decode.c

=================================================================== RCS file: /cvs/maptools/cvsroot/libtiff/test/raw_decode.c,v retrieving revision 1.2

diff -u -U 5 -r1.2 raw_decode.c

--- raw_decode.c 3 Jul 2012 20:48:11 -0000 1.2
+++ raw_decode.c 5 Jul 2012 21:39:24 -0000

@@ -173,15 +173,19 @@
                         "Did not get expected result code from TIFFReadEncodedTile()
(%d instead of %d)\n",
                         (int) szout, (int) sz );
                return 1;
        }

-       if (check_rgb_pixel( 0, 15, 0, 18, buffer )
+    check_rgb_pixel( 0, 15, 0, 18, buffer );
+    check_rgb_pixel( 64, 0, 0, 2, buffer );
+    check_rgb_pixel( 512, 6, 36, 182, buffer );
+
+       /*if (check_rgb_pixel( 0, 15, 0, 18, buffer )

|| check_rgb_pixel( 64, 0, 0, 2, buffer )
|| check_rgb_pixel( 512, 6, 36, 182, buffer ) ) {

exit(1);
- }
+ }*/

free( buffer );

TIFFClose(tif);

@@ -202,10 +206,13 @@

         * Currently TIFFReadRGBATile() just uses JPEGCOLORMODE_RGB so this
         * trivally matches the last results.  Eventually we should actually
         * accomplish it from the YCbCr subsampled buffer ourselves in which
         * case the results may be subtly different but similar.
         */

+    check_rgba_pixel( 0, 15, 0, 18, 255, rgba_buffer );
+    check_rgba_pixel( 64, 0, 0, 2, 255, rgba_buffer );
+    check_rgba_pixel( 512, 6, 36, 182, 255, rgba_buffer );

if (check_rgba_pixel( 0, 15, 0, 18, 255, rgba_buffer )

|| check_rgba_pixel( 64, 0, 0, 2, 255, rgba_buffer )
|| check_rgba_pixel( 512, 6, 36, 182, 255, rgba_buffer ) ) {

exit(1);
}

$ ./raw_decode

Pixel 0 did not match expected results.
Expect:  15   0  18
   Got:  18   0  41
Pixel 64 did not match expected results.
Expect:   0   0   2
   Got:   0   0   0
Pixel 512 did not match expected results.
Expect:   6  36 182
   Got:   5  34 196
Pixel 0 did not match expected results.
Expect:  15   0  18 255
   Got:  18   0  41 255
Pixel 64 did not match expected results.
Expect:   0   0   2 255
   Got:   0   0   0 255
Pixel 512 did not match expected results.
Expect:   6  36 182 255
   Got:   5  34 196 255
Pixel 0 did not match expected results.
Expect:  15   0  18 255
   Got:  18   0  41 255

At first sight, I'd say that the differences might be acceptable, and perhaps the test should be modified to accept them, but my expertise stops there.

>
> Bob