2016.12.17 20:35 "Re: [Tiff] Thoughts about a proposed patch to fix a heap-based buffer overflow in putcontig8bitYCbCr44tile ?", by Even Rouault

Mickey

Thanks for your thoughts.

I wonder how you managed to create tiled YCbCr tiff. Some time ago I wanted to create some

such images for testing, and couldn't figure out how, as the rgb2ycbcr tool only creates stripped,

and tiffcp and tiffcrop refuse to copy subsampled non-JPEG images.

with the gdal_translate tool from GDAL (www.gdal.org), slightly patched with

Index: frmts/gtiff/geotiff.cpp
===================================================================

--- frmts/gtiff/geotiff.cpp     (revision 36934)
+++ frmts/gtiff/geotiff.cpp     (working copy)

@@ -14384,6 +14384,8 @@

             TIFFSetField( l_hTIFF, TIFFTAG_PHOTOMETRIC, PHOTOMETRIC_YCBCR );
             nSamplesAccountedFor = 3;
+
+ TIFFSetField( l_hTIFF, TIFFTAG_YCBCRSUBSAMPLING, 4, 4 );
         }
         else if( EQUAL( pszValue, "CIELAB" ))
         {

and with libjpeg patched to have C_MAX_BLOCKS_IN_MCU/D_MAX_BLOCKS_IN_MCU = 20 in jpeglib.h (18 would be OK as you noted)

And then:

gdal_translate byte.tif test.tif -b 1 -b 1 -b 1 -co compress=jpeg \ -co photometric=ycbcr -co tiled=yes -co blockxsize=32 -co blockysize=32 \ -outsize 63 63

I've recently re-written a considerable portion of `get_image.c` put* functions because I needed to extract regions with arbitrary `col_offset` and `row_offset` from any kind of image. Although partial support for arbitrary `col_offset` was recently added to libtiff, it's still broken for paletted and subsampled images,

so I had to roll my own. I can turn my modifications into a proper patch, but that's not a priority for me and

I'd very much prefer to do that after libtiff will have migrated to git or mercurial.

Your fixes would be welcome. You can create a patch against this mirror if you wish: https:// github.com/vadz/libtiff

>
> Therefore it is impossible to create or decode a JPEG-compressed TIFF with 4
> x4 subsampling
>
> (which would require 18 blocks in MCU), unless you use a custom-built
> libjpeg with higher limits.
>

> IMO not worth it, totally defeats portability.

Agreed. Was just trying to fix a bug. What surprises me is that the old-JPEG decompression doesn't require a patched libjpeg to handle 4x4 subsampling. Presumably it must use libjpeg in another way than the new JPEG method.

>
> The second reason I think 4x4 subsampling is not worth supporting is that it
> complicates implementation.
>
> But there's a bigger issue. Here's a part of `TIFFScanlineSize64` comment
> from `tif_strip.c`
>
> If (1 <= width % 8 <= 4) then ScanlineSize is not a whole number.

ok, I actually reproduce decoding issues with rgb2ycbcr in the exact conditions you mention (so no need to involve JPEG compression. forgot that YCbCr subsampling can be used in other cases)

Even

--
Spatialys - Geospatial professional services
http://www.spatialys.com