Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tiff to JPEG with libvips 8.8.0 for some images #1329

Closed
rps01 opened this issue Jun 4, 2019 · 13 comments
Closed

Tiff to JPEG with libvips 8.8.0 for some images #1329

rps01 opened this issue Jun 4, 2019 · 13 comments
Labels

Comments

@rps01
Copy link

rps01 commented Jun 4, 2019

With libvips 8.8.0 , some of tiff assets aren't successfully converting to jpeg

command: vipsthumbnail Asset.tif -s 1280x1280 -o new.jpeg

Logs:

vipsthumbnail: unable to thumbnail Asset.tif
TIFFSetField: Asset.tif: Unknown pseudo-tag 65538
TIFFSetField: Asset.tif: Unknown pseudo-tag 65538
VipsJpeg: Bogus marker length

This issue wasn't there with previous version 8.7.4.

Related older issue: #311
Using below it does works:
command: vipsthumbnail Asset.tif -s 1280x1280 -o new.jpeg[strip]

@jcupitt
Copy link
Member

jcupitt commented Jun 5, 2019

Hello @rps01,

That's interesting. Could you send a sample file that fails? You can mail me privately if that's easier.

@rps01
Copy link
Author

rps01 commented Jun 6, 2019

@jcupitt Won't be possible to share copyright asset. Tiff assets for which there is issue, they have metadata of about 67kb/70kb.

We plan to use [strip] option for now. Do you see any issue if we use [strip] with assets which doesn't even have XMPmetadata.

[strip] option is something i found from the past issue , is there any documentation where such options are mentioned. Thanks.

@jcupitt
Copy link
Member

jcupitt commented Jun 6, 2019

I think XMP over 64kb is difficult for libjpeg, so this is not supported.

The jpeg save docs are here:

https://jcupitt.github.io/libvips/API/current/VipsForeignSave.html#vips-jpegsave

strip is common to all savers and simply removes all metadata.

It will remove ICC profiles as well, so you should convert to sRGB if these are for display. This page has some notes:

https://libvips.github.io/libvips/API/current/Using-vipsthumbnail.md.html

@chregu
Copy link

chregu commented Jul 2, 2019

Have the same issue, also when converting to png. It's somehow random and hardly reproducible but have it in my logs from time to time (from the PHP extension).
What definitely does trigger it, is to call vipsthumbnail with a 2nd not-existing file.

vipsthumbnail pseudoerror.tiff notexisting.jpg

And here's the triggering image:
http://files.chregu.tv/pseudoerror.tiff

@jcupitt
Copy link
Member

jcupitt commented Jul 2, 2019

Thanks for the example @chregu.

I think it is working, eg.:

$ vipsthumbnail pseudoerror.tiff banana.tif

(vipsthumbnail:16029): VIPS-WARNING **: 10:45:23.509: Incompatible type for "RichTIFFIPTC"; tag ignored

[trim]

vipsthumbnail: unable to thumbnail banana.tif
TIFFSetField: pseudoerror.tiff: Unknown pseudo-tag 65538
TIFFSetField: pseudoerror.tiff: Unknown pseudo-tag 65538
TIFFSetField: pseudoerror.tiff: Unknown pseudo-tag 65538
TIFFSetField: pseudoerror.tiff: Unknown pseudo-tag 65538
VipsForeignLoad: file "banana.tif" not found

You see the extra warnings from TIFFSetField because of the error on banana.tif, but the thumbnail of pseudoerror.tif did work (there's a thumbnail).

I'll see if I can see what's causing the TIFFSetField() warnings.

@chregu
Copy link

chregu commented Jul 2, 2019

Ah yes, it works for vipsthumbnail, but in the php extension, does TIFFSetField warnings do seem to throw an exception, which is kind of strange, since it's just a warning.

@chregu
Copy link

chregu commented Jul 2, 2019

Still hard to reproduce, it happens only every few requests. But figured, that the error may come from somewhere else, from an "out of order read" error. The whole message actually in the exception actually is.

TIFFSetField: memory input: Unknown pseudo-tag 65538
tiff2vips: out of order read at line 1503

@jcupitt
Copy link
Member

jcupitt commented Jul 2, 2019

Huh interesting. It's a plane-separated image (R, G and B are stored in separate subimages), perhaps that's a clue?

The Unknown pseudo-tag 65538 messages are because libvips is requesting expansion of jpg output to RGB from YCbCr when the jpg compressor is not being used.

I thought this was harmless, but it seems it makes libtiff fret. I'll add a test.

jcupitt added a commit that referenced this issue Jul 2, 2019
We were setting TIFFTAG_JPEGCOLORMODE == JPEGCOLORMODE_RGB for *all*
images, but libtiff warns if you use it on an image which is not
jpg-compressed.

Only set it for jpg-compressed images.

See #1329
@jcupitt
Copy link
Member

jcupitt commented Jul 2, 2019

I fixed the spurious Unknown tag warning, thanks!

Otherwise, I can't get it to fail here :( I've set this going:

john@yingna ~/pics $ export VIPS_WARNING=1
john@yingna ~/pics $ for i in {1..10000}; do echo $i; vipsthumbnail pseudoerror.tiff || break; done
1
2
3
4
...

I'll leave it running while I work today and see if it falls over.

@chregu
Copy link

chregu commented Jul 2, 2019

Thanks. for that. I also can reproduce with a php-fpm server and hitting reload lots of time. But not with a simple bash script :(

@jcupitt
Copy link
Member

jcupitt commented Jul 2, 2019

Yes, mine hit 10000 iterations and had no problems :(

@jcupitt
Copy link
Member

jcupitt commented Jul 19, 2019

Someone found a sample file that triggers the "bogus marker length" error:

#1372

It may be fixed in 8.8 HEAD and the fix should be in 8.8.2. Would you be able to test this?

@jcupitt
Copy link
Member

jcupitt commented Dec 7, 2019

I think this may be fixed in 8.8.2 and later.

Please open a new issue if this comes up again.

@jcupitt jcupitt closed this as completed Dec 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants