2005.10.18 13:14 "Re: [Tiff] Small bug report, tif_fax3.c and tif_row", by Joris Van Damme

Can anyone shed some light on this? Where, in the LibTiff design, is tif->tif_row actually meant to be incremented? How is this supposed to work inside strip/tile decoders?

Upon investigation, it seems that the same problem is there for most decoders. I'm very confused, for these reasons:

Two decoders, tif_jpeg and tif_ojpeg, increment tif_row. The others don't. This is rather weird all by itself.
The 'higher level' read routines, like TIFFReadEncodedStrip, seem to set tif_row on a good value before starting the strip/tile decoder. Working from there, and incrementing it inside the strip/tile decoder, may be the original intended design. However, TIFFReadScanline has code that contradicts this, and that seems to need tif_row to indicate the next row logically retrieved by the app, and not the next row logically decompressed... but then, all decoders doing something like (...'...at scanline %d',tif->tif_row,...) report bad scanline numbers, and that seems to be about all of them.
A report about 'scanline 54', is pretty useless anyway for a tiled tiff. The 55th scanline in what tile horizontally? Plus, it's pretty confusing when there's the orientation tag. Is this scanline 54 before or after applying orientation?
In this case in particular, why should the G4 decoder continue after having failed on a scanline? Every subsequent line is encoded with reference to the previous, and there aren't even any end-of-line markers, so there's absolutely no hope to recover from an error in G4 encoded data. That's why I'm seeing this multitude of warnings in the first place, where a single warning about the first line and no attempt at further decoding the tile/strip would be the correct thing.
For these reasons, it may seem that the best possible cure, is also the only feasable one. Why not try and replace these ('...at scanline %d',tif->tif_row) occurences with ('...at line %d in strip/tile %d',internal_counter,tif->tif_curstrip), with internal_counter being some (new?) variable managed and maintained only inside the scope of the compression dependant tile/strip decoder procedure. As far as I can see, tif_curstrip has no problems, so this might work. It would solve some stuff, resulting in more correct and more unambigiously comprehensible warnings/errors, and, most importantly, we don't have to mess with tif_row and its incrementation, and that's a necessity since some stuff is bound to depend on the current state it's in. No API changes would be involved.
How do people feel about this? If I do this on some decoders, will it end up in CVS?

Joris Van Damme
info@awaresystems.be
http://www.awaresystems.be/
Download your free TIFF tag viewer for windows here:
http://www.awaresystems.be/imaging/tiff/astifftagviewer.html