[PATCH] gdaladdo repeated on GTiff make image size increase

[Even Rouault]

Repeating the same gdaladdo command line (ex : 'gdaladdo -r average world_dted0.tiff 2') make the image size increase each time of 128x128xsizeof(pixel type).

I propose the following patch in libtiff to fix that (in TIFFAppendToStrip, in tiff_write.c):

--- gdal-1.4.2.ori/frmts/gtiff/libtiff/tif_write.c      2007-07-28 13:59:18.000000000 +0200
+++ gdal-1.4.2/frmts/gtiff/libtiff/tif_write.c  2007-07-29 18:42:10.000000000 +0200
@@ -642,8 +642,8 @@
              * space, so such scheme is not too much effective.
              */
             if (td->td_stripbytecountsorted) {
-                if (strip == td->td_nstrips - 1
-                    || td->td_stripoffset[strip + 1] <
+                if (strip != td->td_nstrips - 1
+                    && td->td_stripoffset[strip + 1] <
                     td->td_stripoffset[strip] + cc) {
                     td->td_stripoffset[strip] =
                         TIFFSeekFile(tif, (toff_t)0,

[warmerdam]

Hi,

Could you explain the rationale of the code change? This is a twitchy area of the libtiff code, and I've obviously botched it before.

Technically libtiff bug reports should go via the libtiff Bugzilla, but I can handle it this way for this problem.

[warmerdam]

This may relate to #1688.

[Even Rouault]

It was the first time (and I hope the last) that I've put my nose in libtiff code... If I understand well, the point of the test is to be sure that one 'strip' doesn't overlap on the already following strips : "td->td_stripoffset[strip + 1] < td->td_stripoffset[strip] + cc". In my opinion (very humble on the subject), it doesn't make any sense to test this for the last strip : it can't overlap on anything following... That's probably the rational for the original test just before, and to make sure that td_stripoffset[strip + 1] is valid. My hypothese is that the logical test is inverted (my experience shows that it's a quite common programming error pattern). I mean

(strip == td->td_nstrips - 1 || expression(td_stripoffset[strip + 1])

prevents from testing td_stripoffset[strip + 1] when strip == td->td_nstrips - 1. But

(strip != td->td_nstrips - 1 && expression(td_stripoffset[strip+1])

too...

I don't know if there are regression tests in libtiff that could show if it introduces a regression.

[Even Rouault]

I don't reproduce the problem anymore with internal libtiff