error/jp2.c/ReadJP2Image/403.

[hotline]

Hello.
I was not able to confirm some picture files by Windows

Code: Select all

>.\identify .\b7.jp2
identify.exe: unable to decode image file `.\b7.jp2' @ error/jp2.c/ReadJP2Image/403.

images are correct and can be opened with various other software.
source image can be downloaded here: http://u92.s172.xrea.com/x/b7.jp2

I'm using Version: ImageMagick-6.8.3-10-Q16-x86-static.exe
but,This problem did not happen on linux same version.

[AndyI]

Confirmed for me, example file here https://github.com/openplanets/jpylyzer ... alloon.jp2

[magick]

Works under Linux. Submit this to the JasPer Project since its a problem with the Jasper delegate library rather than ImageMagick.

[AndyI]

OK, that's interesting.

So I had a look at jas_image_decode(jas_stream_t *in, int fmt, char *optstr) and there's this:

Code: Select all

	/* Is it possible to decode an image represented in this format? */
	if (!(fmtinfo = jas_image_lookupfmtbyid(fmt)))
		goto error;

And in imagemagick, jp2.c:

Code: Select all

jp2_image=jas_image_decode(jp2_stream,-1,0);
  if (jp2_image == (jas_image_t *) NULL)
    {
      (void) jas_stream_close(jp2_stream);
      ThrowReaderException(DelegateError,"UnableToDecodeImageFile");
    }

the format list is constructed in jas_init.c, and the fmtid's start at 0 and increment from there. So will it not always fail this format check and therefore give this error in imagemagick?

[AndyI]

that is to say, instead of hard coding the format to -1 there, should it not do something like this?

Code: Select all

/*
    Initialize JPEG 2000 API.
  */
  jp2_stream=JP2StreamManager(image);
  if (jp2_stream == (jas_stream_t *) NULL)
    ThrowReaderException(DelegateError,"UnableToManageJP2Stream");
  fmt = jas_image_getfmt(jp2_stream);
  jp2_image=jas_image_decode(jp2_stream,fmt,0);
  if (jp2_image == (jas_image_t *) NULL)
    {
      (void) jas_stream_close(jp2_stream);
      ThrowReaderException(DelegateError,"UnableToDecodeImageFile");
    }

[magick]

We'll get your patch into the next point release of ImageMagick. Thanks.

[AndyI]

Having looked at it a bit more, I wouldn't bother

jas_image_decode does that for you when you pass -1 in as the fmt, so I don't think it's relevant.

What is relevant is your original response - this seems to be a problem with JasPer, specifically, jas_stream_tmpfile(). It seems to create a temp file like this:
_snprintf(obj->pathname, L_tmpnam, "%stmp.XXXXXXXXXX", P_tmpdir);
and then open it with O_EXCL.

But that code is called in a loop in jas_image_create() , through jas_image_cmpt_create()

Code: Select all

	/* Create the individual image components. */
	for (cmptno = 0, cmptparm = cmptparms; cmptno < numcmpts; ++cmptno,
	  ++cmptparm) {
		if (!(image->cmpts_[cmptno] = jas_image_cmpt_create(cmptparm->tlx,
		  cmptparm->tly, cmptparm->hstep, cmptparm->vstep,
		  cmptparm->width, cmptparm->height, cmptparm->prec,
		  cmptparm->sgnd, inmem))) {
			jas_image_destroy(image);
			return 0;
		}
		++image->numcmpts_;
	}

and there is this comment in jas_stream_tmpfile() which probably explains what's going on:

Code: Select all

/* Unlink the file so that it will disappear if the program
	terminates abnormally. */
	/* Under UNIX, one can unlink an open file and continue to do I/O
	on it.  Not all operating systems support this functionality, however.
	For example, under Microsoft Windows the unlink operation will fail,
	since the file is open. */
	if (unlink(obj->pathname)) {
		/* We will try unlinking the file again after it is closed. */
		obj->flags |= JAS_STREAM_FILEOBJ_DELONCLOSE;
	}

However, looking at it in a bit more depth, the snprintf() call is implemented a bit weirdly on windows http://msdn.microsoft.com/en-us/library ... 10%29.aspx

Let len be the length of the formatted data string (not including the terminating null). len and count are in bytes for _snprintf, wide characters for _snwprintf.

If len < count, then len characters are stored in buffer, a null-terminator is appended, and len is returned.

If len = count, then len characters are stored in buffer, no null-terminator is appended, and len is returned.

If len > count, then count characters are stored in buffer, no null-terminator is appended, and a negative value is returned.

So, on my windows box, L_tmpnam is 14. obj.pathname is L_tmpnam + 1, so 15. So you end up with a non null terminated path name.
Sticking the null terminator on at least gets me to the point where the unlink fails due to "dos error 32" or "sharing violation", which is what that comment seems to suggest. But if that's the case, I can't see how this is meant to work on windows

[AndyI]

It's not obvious to me why tmpnam() isn't used.

So i've replaced

Code: Select all

_snprintf(obj->pathname, L_tmpnam, "%stmp.XXXXXXXXXX", P_tmpdir);

with

Code: Select all

tmpnam(obj->pathname);

in jas_stream_tmpfile()
and we're cooking with gas now.

[AndyI]

Ah. so the latest version of libjasper does in fact use tmpnam(), so I assume the one included with Imagemagick is well out of date.

[epa]

tmpnam does not seem to be a solution on Windows.
It generates a string like \schg. for the file name.
(the slash in the beginning and the dot in the end are part
of the tmpnam generated name)

The tmpnam documentation on windows seems to imply that the
leading slash is interpreted so that the file is written to the current
directory, but with call to function "open" as done by libjasper, this
does not happen. Instead it tries to write to the current disk root directory.
Even with Windows this does not typically work any more so the whole
operation fails.

So I would say that anything else than plain tmpnam would be better.
(also tmpnam + removing the initial slash on windows should also work)

(also the previous comment on this thread about unlink failing, is that not
the typical Windows behavior of not being able to unlink an open file?
Also the libjasper seems to currently have code for coping with this).

Eero

[dlemstra]

Maybe we should change the method to use 'tmpfile' instead.