Yep, "4.1.2" meant the gcc version.

Your suggested fix compiles here, and at least the resulting simd_test utility 
doesn't seem to have any issues (I didn't do other testing beyond that).

John

From: Larry Gritz [mailto:[email protected]]
Sent: Monday, July 20, 2015 4:07 PM
To: John Burnett
Cc: OpenImageIO developers
Subject: Re: [Oiio-dev] tif/exif errors in 1.5.16

When you say "4.1.2", you mean gcc version?

Seems like a shame to turn SSE off entirely for gcc 4.1.

What happens if you change line right before,

#  if defined(__GNUC__)

to

#  if defined(__GNUC__) && (__GNUC__ * 100 + __GNUC_MINOR__ >= 404)

Thus, merely not including this one file for older gcc versions. Does that make 
it ok, and still ok to use at least SSE2?



On Jul 20, 2015, at 1:21 PM, John Burnett 
<[email protected]<mailto:[email protected]>> wrote:

Ok, back at my desk....  the patch works fine (I commented on the issue).

Digging into the SSE stuff - it's not finding x86intrin.h (via 
src/include/OpenImageIO/simd.h, line 65 on master).  We're currently using oiio 
1.4.8 and not setting any SSE flags in the build config.  For 1.5.x we'll 
probably start out with USE_SIMD=sse2.   Eyeballing gcc history makes it look 
like that file doesn't exist for 4.1.2, and probably first showed up in the 4.4 
series.  Would it make sense to have a conditional that sets OIIO_NO_SSE if 
compiling on 4.1?

John


From: Larry Gritz [mailto:[email protected]]
Sent: Monday, July 20, 2015 11:29 AM
To: John Burnett
Cc: OpenImageIO developers
Subject: Re: [Oiio-dev] tif/exif errors in 1.5.16

I'll wait for your ok to merge, since you've already pulled in the patch (the 
crux of my question was really about how much trouble it would be for you to 
pull the patch, versus only being able to work from a published branch).

As for the SSE stuff, which header is not found? What SSE flags do you 
ordinarily use, and do you know what kind of machine/CPU you are intending to 
build for?



On Jul 20, 2015, at 11:24 AM, John Burnett 
<[email protected]<mailto:[email protected]>> wrote:

Up to you - I pulled in the patch this morning and it's currently building as I 
type.

As a total sidenote - it looks like something after 1.5.16 made things break on 
gcc41 here.   I'm afk atm, but from memory, include/imagebufalgo.h, line 56 is 
including an SSE header that doesn't exist.  I worked around it by just 
disabling SSE in the interim to test the patch.

John

(sent from mobile. sry for grmmar)

On Jul 20, 2015, at 11:15 AM, Larry Gritz 
<[email protected]<mailto:[email protected]>> wrote:
John, just to be clear -- did you want me to speculatively merge now, or were 
you proposing that you will incorporate the PR patch on your end to test and 
then I'll merge into master after I get the ok from you?


On Jul 18, 2015, at 10:43 AM, John Burnett 
<[email protected]<mailto:[email protected]>> wrote:

Great, thanks!  I'll try the build out early next week at work to make sure 
prman is happy there...

(sent from mobile. sry for grmmar)

On Jul 17, 2015, at 3:02 PM, Larry Gritz 
<[email protected]<mailto:[email protected]>> wrote:
OK, that sounds reasonable. I will add that to the current patch under review.



On Jul 17, 2015, at 2:21 PM, John Burnett 
<[email protected]<mailto:[email protected]>> wrote:

Ah, got it - makes sense.  Thanks for digging in!  As for the defaults, I'd 
lean towards making the --prman option imply "OIIO is doing everything it can 
to make prman happy with the resulting tx file".  In that context, I'd 
definitely vote for having --prman imply tiff:write_exif=0 (especially since 
even the latest prman is using libtiff 3.8.2).

On Thu, Jul 16, 2015 at 5:13 PM, Larry Gritz 
<[email protected]<mailto:[email protected]>> wrote:
This pull request I think allows you to fix this: 
https://github.com/OpenImageIO/oiio/pull/1185

As explained in the PR, it requires an extra argument on the maketx command 
line to set the special metadata that controls whether the Exif metadata is 
suppressed. (In particular, you will add to the maketx arguments: --attrib 
"tiff:write_exif" 0)

I would entertain requests to make this happen automatically any time the 
--prman option to maketx is used. But so far it doesn't happen by default.



> On Jul 16, 2015, at 4:41 PM, Larry Gritz 
> <[email protected]<mailto:[email protected]>> wrote:
>
> OK, I think I understand what's going on here. Certain older versions of 
> libtiff did not properly understand the Exif-containing metadata. Our code 
> checks the TIFF version it's compiled against, and doesn't use this if its 
> libtiff is too old to get it right. But it doesn't really consider the case 
> of OIIO (with modern libtiff) writing the texture file which is destined to 
> be read by a different application that uses a much older libtiff (in this 
> case, PRMan).
>
> I think perhaps the right thing to do is to make an option to suppress output 
> of the Exif data when writing TIFF, in order to make a file that's safe for 
> apps with older libtiff.
>
> Question: Do you think this should happen automatically whenever you use the 
> --prman flag to maketx, or should it be a completely separate argument to 
> maketx?
>
>
>
>> On Jun 17, 2015, at 7:40 PM, John Burnett 
>> <[email protected]<mailto:[email protected]>> wrote:
>>
>> We are running into metadata issues with tif files created in oiio 1.5.16 
>> (after upgrading from 1.4.8).  In both cases, we are building against 
>> libtiff 4.0.3.
>>
>> As a repro, I started with a plain exr created with oiio 1.4.8:
>>
>>  oiio-1.4.8/oiiotool --pattern checker 128x128 4 --ch R,G,B,=1.0 -d half -o 
>> checker.exr
>>
>> And then created tx files with both 1.5.16 and 1.4.8 as a baseline:
>>
>>  oiio-1.4.8/maketx --prman -o oiio-1.4.8.tx checker.exr
>>  oiio-1.5.16/maketx --prman -o oiio-1.5.16.tx checker.exr
>>
>> Using tiffinfo built with libtiff 4.0.3 on oiio-1.5.16.tx image shows the 
>> following printed to stderr:
>>
>>  tiffinfo oiio-1.5.16.tx > /dev/null
>>  TIFFFetchDirectory: Sanity check on directory count failed, zero tag 
>> directories not supported.
>>  TIFFReadCustomDirectory: Failed to read custom directory at offset 28686.
>>
>> ...and oiio-1.4.8.tx image shows no such issues.
>>
>> Lastly, using the tiffinfo that ships with prman 19 shows the following (I'm 
>> not sure what version of libtiff prman is using, but the message makes it 
>> look like it's 3.x):
>>
>>  prman/bin/tiffinfo oiio-1.5.16.tx > /dev/null
>>  TIFFReadDirectory: Warning, oiio-1.5.16.tx: wrong data type 13 for 
>> "EXIFIFDOffset"; tag ignored.
>>
>> ...which is the base problem here - renders are spitting out the above 
>> warning for all texture reads.
>>
>> tiffdump shows the two tx files are largely the same, except the 1.5.16 tx 
>> has an extra TIFFTAG_EXIFIFD tag at the end of the 0th directory.  The 
>> biggest relevant source diff I can see between the two oiio versions is 
>> TIFFOutput::write_exif_data.  I'm half wondering if the 
>> TIFFSetDirectory/TIFFSetField calls at the bottom of that method are failing 
>> in some way, but this is literally the second time I've ever looked at the 
>> tiff format and source, so it's a shot in the dark :).
>>
>> I have the above three image files I can mail over if that would be helpful 
>> (checker.exr, oiio-1.4.8.tx, and oiio-1.5.16.tx).
>>
>> John
>>
>> _______________________________________________
>> Oiio-dev mailing list
>> [email protected]<mailto:[email protected]>
>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
>
> --
> Larry Gritz
> [email protected]<mailto:[email protected]>
>
>
> _______________________________________________
> Oiio-dev mailing list
> [email protected]<mailto:[email protected]>
> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org

--
Larry Gritz
[email protected]<mailto:[email protected]>


_______________________________________________
Oiio-dev mailing list
[email protected]<mailto:[email protected]>
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org

_______________________________________________
Oiio-dev mailing list
[email protected]<mailto:[email protected]>
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org

--
Larry Gritz
[email protected]<mailto:[email protected]>

_______________________________________________
Oiio-dev mailing list
[email protected]<mailto:[email protected]>
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
_______________________________________________
Oiio-dev mailing list
[email protected]<mailto:[email protected]>
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org

--
Larry Gritz
[email protected]<mailto:[email protected]>


--
Larry Gritz
[email protected]<mailto:[email protected]>

--
Larry Gritz
[email protected]<mailto:[email protected]>

_______________________________________________
Oiio-dev mailing list
[email protected]
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org

Reply via email to