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