jpeg tiff creation fails with gdal and libtiff 3.9.3+

[warmerdam]

Briefly, this is related to the jpegtables getting added late as discussed in ​http://bugzilla.maptools.org/show_bug.cgi?id=2135 and reported by Even on the libtiff list:

I've just tested libtiff 3.9.3 with GDAL autotest suite (and GDAL SVN head), and I'm seeing failures that I do not get with libtiff 3.9.2.

Those new failures are in tiff_write_13(), tiff_write_54() and tiff_write_58() (http://trac.osgeo.org/gdal/browser/trunk/autotest/gcore/tiff_write.py) All 3 are related to reading JPEG compressed TIFF that have just been created.

Basically when I open the image generate by tiff_write_13() I see a black square for the first tile of the image (or a whole black image for stripped images). This can be easily reproduced with running: gdal_translate -of JPEG in.tif out.tif and opening out.tif afterwards

There were a few changes in tif_jpeg.c between 3.9.2 and 3.9.3. I've found that reverting the following change in TIFFInitJPEG() fixes the 3 above failures.

2010-02-22  Lee Howard  <XXX@XXX>

	* libtiff/tif_jpeg.c: Do not generate a JPEGTables tag when creating
	the JPEG TIFF as is is not required in order to prevent it from 
	being unused and filled with invalid data.  (Leave it to be 
	generated by later activity.)
	http://bugzilla.maptools.org/show_bug.cgi?id=2135

The change was to comment the TIFFSetFieldBit(tif, FIELD_JPEGTABLES) call.

/*
The following line assumes incorrectly that all JPEG-in-TIFF files will have
a JPEGTABLES tag generated and causes null-filled JPEGTABLES tags to be 
written
when the JPEG data is placed with TIFFWriteRawStrip.  The field bit should be 
set, anyway, later when actual JPEGTABLES header is generated, so removing it 
here hopefully is harmless.
            TIFFSetFieldBit(tif, FIELD_JPEGTABLES);
*/

Frank got problems with that change (see comment #2 in ​http://bugzilla.maptools.org/show_bug.cgi?id=2135) and found a solution that implied changing TIFFFlush() that has gone in libtiff 4.0.0beta6 (GDAL autotest suite is happy with 4.0.0beta6), but not in 3.9 branch.

Change in 4.0 branch :

2010-03-31  Frank Warmerdam  <XXX@XXX>

	* libtiff/tif_flush.c: Use TIFFRewriteDirectory() when
 flushing directories so previously placed directories will be
 migrated to the end of file if needed.

So I've tested the following change, similar to the one done in trunk :

--- libtiff/tif_flush.c.bak	2010-06-12 13:25:45.000000000 +0200
+++ libtiff/tif_flush.c	2010-06-12 13:25:52.000000000 +0200
@@ -37,7 +37,7 @@
 		if (!TIFFFlushData(tif))
 			return (0);
 		if ((tif->tif_flags & TIFF_DIRTYDIRECT) &&
-		    !TIFFWriteDirectory(tif))
+		    !TIFFRewriteDirectory(tif))
 			return (0);
 	}
 	return (1);

and this fixes the tiff_write_13(), tiff_write_54() and tiff_write_58() failures, but it causes a new failure in tiff_write_72(), which is not critical but a sign that the directory is rewritten a bit too aggressively (leading to larger files than necessary).

But TIFFFlush() in libtiff 4.0 has changed quite significantly in comparison to libtiff 3.X, and it looks like efforts were made to avoid (re)writing the directory when only strip/tile map has been altered.

So at that point, I'm not sure which solution is appropriate for 3.9 branch :

(1) reverting the change in TIFFInitJPEG() --> means "unfixing" ​http://bugzilla.maptools.org/show_bug.cgi?id=2135

(2) replace TIFFWriteDirectory() by TIFFRewriteDirectory() in TIFFFlush() --> produce files a bit larger than before, due to unnecessary directory rewriting

(3) = (2) + porting the other changes done in 4.0 branch in TIFFFlush()

Note that I do not observe any problem when creating JPEG compressed TIFF with tiffcp. By looking at the produced file, it looks like tiffcp only writes the directory at the end of the process, whereas GDAL GTiff driver does an early "crystalization" (GTiffDataset::Crystalize() method) to "make sure that the directory information is written out for a new file, require before writing any imagery data."

Best regards,

Even

[warmerdam]

I feel the proposed change to use TIFFRewriteDirectory() is too radical to impose on the libtiff 3.9.x branch, so instead I have decided to force have GDAL force out a dummy block of imagery in the CreateCopy() function when using libtiff 3.9.x in order to trigger creation of the properly jpegtable. I have committed this change in trunk (r19861) and 1.7 branch (r19862).

While it would be nicer if libtiff was just safe, the use case in question (involving flushing the directory before any imagery is written) is not typical of libtiff use, and the rewrite fix will have side effects I fear in the 3.9 branch.

I have migrated some of the libtiff 4 logic to avoid rewriting the jpeg tables multiple times in libtiff 3.9.x:

diff -r1.50.2.8 tif_jpeg.c
1272,1277c1272,1281
<               if (!prepare_JPEGTables(tif))
<                       return (0);
<               /* Mark the field present */
<               /* Can't use TIFFSetField since BEENWRITING is already set! */
<               TIFFSetFieldBit(tif, FIELD_JPEGTABLES);
<               tif->tif_flags |= TIFF_DIRTYDIRECT;
---
>                 if( sp->jpegtables == NULL
>                     || memcmp(sp->jpegtables,"\0\0\0\0\0\0\0\0\0",8) == 0 )
>                 {
>                         if (!prepare_JPEGTables(tif))
>                                 return (0);
>                         /* Mark the field present */
>                         /* Can't use TIFFSetField since BEENWRITING is already
 set! */
>                         tif->tif_flags |= TIFF_DIRTYDIRECT;
>                         TIFFSetFieldBit(tif, FIELD_JPEGTABLES);
>   

With these changes it should be possible to produce jpeg compressed tiff file with gdal trunk or 1.7 against libtiff 3.9.x.

[Even Rouault]

r20143 /trunk/gdal/frmts/gtiff/geotiff.cpp: Fix regressions with writing JPEG-compressed TIFF with libtiff 3.X by avoiding an unnessary read of the dummy block that has been written to force the creation of the JPEG tables (fix of fix of #3633)

r20144 /branches/1.7/gdal/frmts/gtiff/geotiff.cpp: Fix regressions with writing JPEG-compressed TIFF with libtiff 3.X by avoiding an unnessary read of the dummy block that has been written to force the creation of the JPEG tables (fix of fix of #3633)

[Even Rouault]

r21396 /trunk/gdal/frmts/gtiff/geotiff.cpp: GTiff: apply the same workaround in Create() as we did in CreateCopy() when creating a JPEG-IN-ITFF with 3.9.X (actually X >=3). Fixes remaining issues with tiff_write_54 and tiff_write_58 (continuation of #3633)

(would also be appropriate for 1.7 branch, but I doubt this is worth the risk)