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

[CTS 34] Error with TIFF with colormap #133

Closed
carygravel opened this issue Oct 25, 2020 · 13 comments
Closed

[CTS 34] Error with TIFF with colormap #133

carygravel opened this issue Oct 25, 2020 · 13 comments
Labels
bug something not working to spec, or causes crash

Comments

@carygravel
Copy link
Contributor

In the attached zip, when I run the following code, I get the error message Can't use string ("1845184560") as an ARRAY ref while "strict refs" in use at /usr/share/perl5/PDF/Builder/Resource/XObject/Image/TIFF/File_GT.pm line 154.

I'll give you a pull request to fix this ASAP.

#!/usr/bin/perl
use warnings;
use strict;
use PDF::Builder;

my $width = 1552;
my $height = 1056;
my $pdf = PDF::Builder->new(-file => 'colormap.pdf');
my $page = $pdf->page();
$page->mediabox($width, $height);
my $gfx = $page->gfx();
my $img = $pdf->image_tiff('colormap.tif');
$gfx->image($img, 0, 0, $width, $height);
$pdf->save();
$pdf->end();
@PhilterPaper
Copy link
Owner

That's interesting. I ran it without any messages, although Adobe Reader says the PDF is corrupt and can't open it. The TIFF file is a scan of a diary entry about someone's colostomy (yecch), so that looks OK. None of the TIFF .pm files seem to have been changed for quite a few releases -- you haven't changed anything locally, have you?

@carygravel
Copy link
Contributor Author

The reason I get an error message and you don't is that I am running with Graphics::TIFF and you are not. After some investigation, I see that Graphics::TIFF doesn't pass the colormap properly. I'll get a new release out and then make the appropriate fixes here.

@PhilterPaper
Copy link
Owner

I do have Graphics::TIFF, so that can't be it. But if there's a problem with GT, someone will need to fix it. Are you running with a more recent version (development version) than I am? As I recall, my libtiff is what came with Strawberry Perl 5.26 and it hasn't been updated in almost 3 years or more. I have a ticket in with Graphics::TIFF (RT 132346) suggesting that an Alien::TIFF be created so as to keep libtiff up to date on non-Linux systems.

@carygravel
Copy link
Contributor Author

Please don't confuse libtiff with Graphics::TIFF. Either can be updated without changing the other.

I understand why you like the idea of Alien::TIFF, but I cannot realise it, as I don't have access to a Windows machine. This issue is not a problem with libtiff.

I will solve this problem with a new version of Graphics::TIFF. It will also need small changes to PDF::Builder.

@PhilterPaper
Copy link
Owner

I realize that Graphics::TIFF is a wrapper around libtiff. I have both, and for a long time I have used Graphics::TIFF for TIFF processing (also testing without it). If you want to create an Alien::TIFF, I can't help much with developing/coding, but of course I have a Windows machine to test on. Are you working with Jeffrey Ratcliffe on this? He's the owner of Graphics::TIFF (or are you he?) or are you primarily on the libtiff side?

@PhilterPaper PhilterPaper added the bug something not working to spec, or causes crash label Oct 26, 2020
@PhilterPaper
Copy link
Owner

OK, I merged your PR and cleaned up a few things (LAST_UPDATE version, tiff.t comments). In your new t/tiff.t test, I notice that you were not checking for Graphics::TIFF ($has_GT)... I presume it's needed. I also removed the tiffcp check, as it's not used. It appears in the test that you're satisfied that the test program runs without blowing up, rather than testing against the image as in other tests. Is there a reason for this? I'm concerned, because when I run the Perl code up top, I still get the corrupted PDF error that I reported before. And yes, I am at Graphics::TIFF version 7 (and still a fairly old libtiff). I can send you the files if you want to look at them.

By the way, in Builder.pm I do some tests for Graphics::TIFF with require Graphics::TIFF;. I tried adding version 7 to them (require Graphics::TIFF 7;), as you did in the *_GT.pm files with "use", but Perl didn't like that syntax. Do you know if there's a way to require version 7? As it stands now, Builder.pm can only verify that some version of Graphics::TIFF is loaded; it can't verify that it's version 7 until it's deep in the TIFF code.

@carygravel
Copy link
Contributor Author

carygravel commented Oct 28, 2020

That you still get the corrupt PDF I find worrying. What happens if you enable warnings by running with perl -w?

What version of libtiff do you have?

require isn't great. I would always wrap use in try {} catch {}. You might not want to add the dependency, but it (or something similar) really should be core, because it is standard practice nowadays. Other languages like js and python do exactly the same thing.

I'll look at your changes and add some stuff to the CI to additionally run the tests without Graphics::TIFF.

@PhilterPaper
Copy link
Owner

No difference if I run with perl -w. It seems to take a bit longer to run, but still no messages of any kind, and the resulting PDF is still in error.

I don't think I explicitly installed libtiff.a. I think it came pre-installed with Strawberry Perl 5.26.1, January 2018. I did have to install Graphics::TIFF, in order to use it.

I use require in a couple places in Builder.pm just to test that Graphics::TIFF is available. I did not add Graphics::TIFF, Image::PNG::Libpng, or HarfBuzz::Shaper as prerequisites for installing PDF::Builder, because I didn't want to burden users with adding subsystems that very few of them are likely to use. Also, if the prereq installation fails, they can't use PDF::Builder at all. Are you saying that there should be something available in core Perl that does the job of these packages? Or are you saying that Perl really ought to put these things in core?

@carygravel
Copy link
Contributor Author

I think that something like Try::Tiny should be in core and should be used instead of eval in most cases.

Graphics::TIFF->GetVersion

will tell you which libtiff version you have.

I suspect that your PDF viewer is stricter than mine, meaning the PDF produced by the colormap code is slightly broken, but readable by some viewers. I'll take another look at the colormap code and get back to you.

@PhilterPaper
Copy link
Owner

OK, Graphics::TIFF itself is v7, and

C:\Users\Phil\Desktop>perl -e "use Graphics::TIFF; print Graphics::TIFF->GetVersion;"
LIBTIFF, Version 4.0.7
Copyright (c) 1988-1996 Sam Leffler
Copyright (c) 1991-1996 Silicon Graphics, Inc.

Try::Tiny is not a core module -- are you saying that it ought to be?

I view PDFs with Adobe Acrobat Reader DC (I think it's cloud-based) under Windows.

@carygravel
Copy link
Contributor Author

One tool I've used for looking at the PDF produced from the TIFF with a colormap complains

Syntax Warning: Bad Indexed color space (lookup table stream too short) padding with zeroes

If I add some debugging statements in TIFF.pm and TIFF_GT.pm and compare the size of the stream produce for the colormap for this tiff, I see that with and without Graphics::TIFF, the size of the streams is identical. Indeed, if I create the same PDF without Graphics::TIFF, the above tool makes exactly the same complaint.

The palette code for PNG looks rather different. The resulting image object should be the same, though, irrelevant whether it came from TIFF or PNG, shouldn't it?

If my above assumption is correct, then I'm guessing that TIFF.pm and TIFF_GT.pm are buggy in the same way.

@PhilterPaper
Copy link
Owner

Well, the lookup table (a.k.a. colormap, palette?) is flate compressed... is it supposed to be uncompressed? I'll have to look at the size of the colormap before it's compressed, and see if it's complete. I vaguely recall PNG allowing a short palette if the missing entries weren't used; I'll have to see if the TIFF code is assuming the same thing here (and what the TIFF spec says about it). I don't recall rewriting the palette code when implementing Graphics::TIFF, so it's possible the same bug is in both versions.

Once image_<type>() has taken care of converting an image file, I would think that the image object should be largely the same. I think there may be several different compression schemes, such as DCT-something for JPEG only (avoiding having to uncompress an image and recompress it in a standard method such as flate). Speaking of which, there is a request for JPEG-2000 support (another compression scheme) on the queue.

As you may have guessed by now, you're probably the first one to really exercise the TIFF code, and are finding all sorts of implementation problems that have been around for a decade or more. Compared to JPEG, GIF, and PNG; there's just not that much usage of TIFF images. I do appreciate the interest you're taking, and the help you're giving on this!

@PhilterPaper
Copy link
Owner

Fixed by PR 137. Thanks!

@PhilterPaper PhilterPaper changed the title Error with TIFF with colormap [CTS 34] Error with TIFF with colormap Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something not working to spec, or causes crash
Projects
None yet
Development

No branches or pull requests

2 participants