Created an attachment (id=433) [details]
Problem Tiff file

Created an attachment (id=434) [details]
output file

(From update of attachment 433 [details])

Additinal errors:
Warning, using bottom-left orientation

One of our customers complained about the same issue in FreeImage (uses LiBTIFF
for TIFF handling). They experienced that a JPEG-compressed color TIFF with
orientation bottom left (i.e. rotated 90 degrees clockwise) was always loaded
as mirror image. Unfortunately, their Xerox WorkCentre scanner uses this
orientation by default.

We researched this problem, please see below our suggested fix for function
setorientation in tif_getimage.c. 

TEST IMAGES
When researching the problem, we were hampered by a lack of good test material
for TIFFs with orientation tags. Therefore we created a full set of test
images, that can be downloaded from this location:
http://support.decos.nl/exif-orientation-testimages.zip (4 MB)

The zip contains sets of 8 test images rendered in all 8 possible exif
orientations, in these formats: jpg, tiff-CCITT group 4, tiff-CCITT RLE, tiff-
JPEG compression v6, tiff-JPEG compression v7, tiff-LZW b&w, tiff-LZW color,
tiff-packbits color. The malfunction in LibTIFF occurs only with JPEG
compressed color TIFFs, with 4 out of the 8 possible orientations.


CAUSE OF THE PROBLEM
JPEG-compressed Color TIFFs (either Compression 6 or 7) are loaded using
TIFFReadRGBAImage. TIFFReadRGBAImage calls TIFFReadRGBAImageOriented for the
default orientation ORIENTATION_BOTLEFT. If no orientation tag is present, the
orientation in the tiff defaults to ORIENTATION_TOPLEFT, which is the native
scanline order for a Tiff.

During processing, function setorientation in tif_getimage.c is called, with
the detected orientation for the tiff in img->orientation and
ORIENTATION_BOTLEFT (4) in img->req_orientation. This function is supposed to
determine whether a horizontal or vertical flip (or both) is needed between the
two orientations.

For the default orientation 1, setorientation returns FLIP_VERICALLY. That is
correct, so without orientation tag, the image looks fine. However, if the
input value is ORIENTATION_LEFTBOT (8), setorientation returns 0, indicating
that no flip is required. I believe that is a bug. Please consider this web
page showing visual samples of exif rotations:
http://sylvana.net/jpegcrop/exif_orientation.html

If you compare 8 and 4, you'll see that a rotation *and* a flip (either
horizontal or vertical) is needed to correctly transform between the two
orientations.

Function setorientation assumes that the pairs
(TOPLEFT,LEFTTOP),(TOPRIGHT,RIGHTTOP), (BOTRIGHT,RIGHTBOT) and
(BOTLEFT,LEFTBOT) require the same transformation. That is not correct. If you
look at the exif_orientation.html web page, you'll see that these pairs are in
fact pairs of mirror images, and thus cannot possibly have the same flip
transformation requirement.


SUGGESTED FIX
Our suggested fix is the following replacement code for setorientation in
tif_getimage.c:

////>>>> Declarations near the top of the file: ROTATE_90DEGREES is added:

/* 
 * Helper constants used in Orientation tag handling
 */
#define FLIP_VERTICALLY 0x01
#define FLIP_HORIZONTALLY 0x02
#define ROTATE_90DEGREES 0x04 // Berend web/6242 20110701

////>>>> Replacement code for setorientation

/*
 * Return value will have FLIP_VERTICALLY (1), FLIP_HORIZONTALLY (2) or 
 * ROTATE_90DEGREES (4) bits set if any of these transformations are 
 * needed to transform  from img->orientation to img->req_orientation.
 *
 * Because LibTIFF does not contain image rotation code, images with
 * orientation > 4 will be loaded rotated by 90 degrees.
 */
static int 
setorientation(TIFFRGBAImage* img)
{
  const uint16 flipbits[8] = {0,2,3,1,6,7,5,4};
  uint16 flip_orient = flipbits[img->orientation - 1];
  uint16 flip_req_or = flipbits[img->req_orientation - 1];
  return (flip_orient & ~flip_req_or) | (~flip_orient & flip_req_or);

  //          TL TR BR BL LT RT RB LB <- Source orientation
  // flipbits  0  2  3  1  6  7  5  4
  //        +  -  -  -  -  -  -  -  -
  // TL  0  |  0  2  3  1  6  7  5  4
  // TR  2  |  2  0  1  3  4  5  7  6
  // BR  3  |  3  1  0  2  5  4  6  7
  // BL  1  |  1  3  2  0  7  6  4  5
  // LT  6  |  6  4  5  7  0  1  3  2
  // RT  7  |  0  5  4  6  1  0  2  3
  // RB  5  |  5  7  6  4  3  2  0  1
  // LB  4  |  3  6  7  5  2  3  1  0 | 
  //                                - return values
  // ^ Target orientation

}

Because the new implementation uses matrix logic, it is much more compact thatn
the current implmentation (4 lines of code instead of 56 lines). I believe it
is also more robust. With both our test images and the problem scan we
received,. we could confirm it fixes the bug :-)

Note that FreeImage uses a version of LibTIFF with slightly different type
definitions for the primitive data types. uint16 is intended to be a 16 bit
unsigned integer. If this gives you compilation errors in the standard version
of LibTIFF, just replace "uint16" by "unsigned" in the code fragment.

Hope this helps,
With kind regards,


Berend Engelbrecht
Oegstgeest, The Netherlands

Created an attachment (id=463) [details]
Replacement code for tif_getimage.c, based on LibTIFF 4.0 beta 7

(From update of attachment 433 [details])
tiffexposedtofeds

(From update of attachment 433 [details])
null

The attached file is a copy of tif_getimage.c instead of a patch.  So it's
difficult to know what I should commit.

Bugzilla is no longer used for tracking libtiff issues. Remaining open tickets,
such as this one, have been migrated to the libtiff GitLab instance at
https://gitlab.com/libtiff/libtiff/issues .

The migrated tickets have their summary prefixed with [BZ#XXXX] where XXXX is
the initial Bugzilla issue number.