Changes in converttif.c for PPC64

[szukw000]

Type mismatch in 'src/bin/jp2/converttif.c' found using PPC64.
winfried

[szukw000]

Before the change:

bin/opj_compress -i ../N98_0002-16bit-gray.tif -o N98_0002-16bit-gray.tif.jp2

converttif.c:1284:
tiWidth(447) tiHeight(287) tiBps(1048576) tiSf(0) tiSpp(65536) tiPhoto(65536) tiPC(65536)
tiftoimage: Bad value for samples per pixel == 65536.
Aborting.
Unable to load tiff file

After the change:

bin/opj_compress -i ../N98_0002-16bit-gray.tif -o N98_0002-16bit-gray.tif.jp2

converttif.c:1287:
tiWidth(447) tiHeight(287) tiBps(16) tiSf(0) tiSpp(1) tiPhoto(1) tiPC(1)
[INFO] tile number 1 / 1
[INFO] Generated outfile N98_0002-16bit-gray.tif.jp2
encode time: 84 ms

winfried

[rouault]

@szukw000 Is this a 2.2 regression ?

@detonin How should be bugfixes like this one handled ? Only in master, or will there be a openjpeg-2.2 branch for maintenance releases ?

[szukw000]

@rouault ,
what do you mean with 'regression'? It is a bug:

struct _TIFFRGBAImage {
(...)
    uint16 bitspersample;   /* image bits/sample */
    uint16 samplesperpixel; /* image samples/pixel */
    uint16 orientation;     /* image orientation */
    uint16 req_orientation; /* requested orientation */
    uint16 photometric;     /* image photometric interp */
    uint16* redcmap;        /* colormap pallete */
    uint16* greencmap;
    uint16* bluecmap;
(...)
};

A little endian INT container is different from a big endian container.
The type 'uint32' simply is wrong.
winfried

[rouault]

what do you mean with 'regression'?
A bug that was introduced recently and didn't exist before. As it is in code you touched in #975 I'm wondering if it used to work before (I didn't check, hence my question)

[szukw000]

@rouault ,

openjpeg-master-2015-07-31:

int imagetotif(opj_image_t * image, const char *outfile)
{
int width, height;
int bps,adjust, sgnd;
int tiPhoto;

TIFFSetField(tif, TIFFTAG_IMAGEWIDTH, width);
TIFFSetField(tif, TIFFTAG_IMAGELENGTH, height);
TIFFSetField(tif, TIFFTAG_SAMPLESPERPIXEL, numcomps);
TIFFSetField(tif, TIFFTAG_BITSPERSAMPLE, bps);
TIFFSetField(tif, TIFFTAG_ORIENTATION, ORIENTATION_TOPLEFT);
TIFFSetField(tif, TIFFTAG_PLANARCONFIG, PLANARCONFIG_CONTIG);
TIFFSetField(tif, TIFFTAG_PHOTOMETRIC, tiPhoto);
TIFFSetField(tif, TIFFTAG_ROWSPERSTRIP, 1);

}

opj_image_t* tiftoimage(const char *filename, opj_cparameters_t *parameters)
{
unsigned short tiBps, tiPhoto, tiSf, tiSpp, tiPC;
unsigned int tiWidth, tiHeight;

TIFFGetField(tif, TIFFTAG_IMAGEWIDTH, &tiWidth);
TIFFGetField(tif, TIFFTAG_IMAGELENGTH, &tiHeight);
TIFFGetField(tif, TIFFTAG_BITSPERSAMPLE, &tiBps);
TIFFGetField(tif, TIFFTAG_SAMPLEFORMAT, &tiSf);
TIFFGetField(tif, TIFFTAG_SAMPLESPERPIXEL, &tiSpp);
TIFFGetField(tif, TIFFTAG_PHOTOMETRIC, &tiPhoto);
TIFFGetField(tif, TIFFTAG_PLANARCONFIG, &tiPC);

}

I found these types upto openjpeg2-2016-11-30.

In openjpeg2-2016-12-07 I found:

int imagetotif(opj_image_t * image, const char *outfile)
{
uint32 width, height, bps, tiPhoto;

TIFFSetField(tif, TIFFTAG_IMAGEWIDTH, width);
TIFFSetField(tif, TIFFTAG_IMAGELENGTH, height);
TIFFSetField(tif, TIFFTAG_SAMPLESPERPIXEL, (uint32)numcomps);
TIFFSetField(tif, TIFFTAG_BITSPERSAMPLE, bps);
TIFFSetField(tif, TIFFTAG_ORIENTATION, ORIENTATION_TOPLEFT);
TIFFSetField(tif, TIFFTAG_PLANARCONFIG, PLANARCONFIG_CONTIG);
TIFFSetField(tif, TIFFTAG_PHOTOMETRIC, tiPhoto);
TIFFSetField(tif, TIFFTAG_ROWSPERSTRIP, 1);

}

opj_image_t* tiftoimage(const char *filename, opj_cparameters_t *parameters)
{
uint32 tiBps, tiPhoto, tiSf, tiSpp, tiPC, tiWidth, tiHeight;

TIFFGetField(tif, TIFFTAG_IMAGEWIDTH, &tiWidth);
TIFFGetField(tif, TIFFTAG_IMAGELENGTH, &tiHeight);
TIFFGetField(tif, TIFFTAG_BITSPERSAMPLE, &tiBps);
TIFFGetField(tif, TIFFTAG_SAMPLEFORMAT, &tiSf);
TIFFGetField(tif, TIFFTAG_SAMPLESPERPIXEL, &tiSpp);
TIFFGetField(tif, TIFFTAG_PHOTOMETRIC, &tiPhoto);
TIFFGetField(tif, TIFFTAG_PLANARCONFIG, &tiPC);

}

winfried

[rouault]

For the TIFFSetField() part, given that it is a variadic argument, uint16 is automatically promoted to int, which is fine. So I guess this part of your changes is OK, but without effect.
The interesting part is for TIFFGetField(). I was a bit lost in your above analysis, so I looked at the tags, and it appears that 2.1.2 is OK:
https://github.com/uclouvain/openjpeg/blob/v2.1.2/src/bin/jp2/converttif.c#L1220 . So this is indeed a regression and it was introduced by commit 00f4568#diff-a134c5f68030d3b0836765c0cb06901dR1242

Merging in master now. This might be candidate for a v2.2 branch as well

[detonin]

@detonin How should be bugfixes like this one handled ? Only in master, or will there be a openjpeg-2.2 branch for maintenance releases ?

@rouault In master only: as there are no API/ABI break, we don't create a specific openjpeg-2.2 branch.