JPEG resolution not imported in Windows

[Hans-Karl]

Following this new features :
https://code.launchpad.net/~dawagenaar/inkscape/import-res-daw
https://code.launchpad.net/~dawagenaar/inkscape/import-res-optional

and this bugfixes :
https://bugs.launchpad.net/inkscape/+bug/165952
https://bugs.launchpad.net/inkscape/+bug/1052796

I notice the good result for PNG resolution import, but bad results for JPEG, BMP and TIFF resolution import, in the Windows snapshot from https://skydrive.live.com/?cid=09706d11303fa52a&id=9706D11303FA52A%21217 (R11882)

See picture.
Note JPEG is not loading at the 200 dpi indicated in the original file.

[jazzynico (jazzynico)]

Reproduced on Windows XP, Inkscape trunk revision 11883, files from Bug #165952, comment #42.

Since the devlibs already include libjpeg, adding "#define HAVE_JPEG 1" in build.xml (around line 190) fixes the issue with the file Fiche_03.jpg.

I've also tried with a local version with libexif linked, but libexif doesn't seem to find relevant information in the file. Test with other formats (tiff, gif) in progress.

[jazzynico (jazzynico)]

More tests on Windows XP.

Importing JPEG files with EXIF tags works as expected with libexif-0.6.21 (compiled locally). libexif is very light and could be easily added to the Inkscape win32 devlibs.
Unfortunately, EXIF tags in TIFF files aren't even used (exif_data_new_from_file() returns NULL).

@Daniel - Do you plan to improve your code so that it supports TIFF, BMP, GIF, and other formats?
If not, it wouldn't be to hard to use ImageMagick as a fallback (patch from Bug #165952, comment #12 could be a good start).

Since the JPEG bug can be easy-fixed, there's no risk to include it in 0.49 and close the report.
Other changes could be a bit more risky. Proposing to open a new report to handle other formats support.

[jazzynico (jazzynico)]

> Importing JPEG files with EXIF

Of course, test were done with "#define HAVE_EXIF 1" in build.xml.

[Daniel Wagenaar (dawagenaar)]

I would favor including libexif so that the important jpeg case works.

As far as I know, bmp and gif do not store resolution information. Can
somebody confirm or correct me?

I am surprised that exif tags from tiff files aren't read. I will test
in linux.

ImageMagick fallback support is an interesting idea and shouldn't be too
hard to implement. However, I do see two potential show stoppers: (1)
Using ImageMagick will be slow, since it involves loading the whole
image file twice, and (2) Since it involves running an external binary,
there are security concerns that should be thought through. (And not by
me, because I am not an expert, certainly not on Windows.)

[Alvin Penner (apenner)]

according to : http://en.wikipedia.org/wiki/BMP_file_format
the .bmp file format does store resolution information in the BITMAPINFOHEADER in units of pixels/meter.

[jazzynico (jazzynico)]

> Using ImageMagick will be slow, since it involves loading the whole image file twice

Yes, that's why it would be a fallback.

> there are security concerns that should be thought through

We already use ImageMagick in the Extensions>Raster menu. The only difference is that importing images is a far more common task than using the extensions raster effects and potential issues would affect more users.
On the other hand, we already use lots of external libs (such as libjpeg, libpng, and many others). Is there a reason why ImageMagick would be more dangerous?

[Daniel Wagenaar (dawagenaar)]

> Yes, that's why it would be a fallback.
I see. How common would that case be? Do we have any usage statistics?
> ... we already use lots of external libs (such as libjpeg, libpng, and
> many others). Is there a reason why ImageMagick would be more dangerous?
Using it as a library shouldn't pose any specific dangers. I was under
the impression that you wanted to use the "identify" command externally.

[jazzynico (jazzynico)]

libexif added to the devlibs revision 37.
JPEG and EXIF support activated in the trunk, revision 11925.

[jazzynico (jazzynico)]

ImageMagick fallback reported separately in Bug #1085949 "Use ImageMagick fallback when importing bitmap resolution".

[Hans-Karl (jchbraun)]

Works fine with R11928 for Windows. Thanks!

[Hans-Karl (jchbraun)]

Sorry, I have an other problem with some JPEG pictures : It seems when EXIF fields are empty, resolution is not imported correctly. See screenshot for details.
I join also a problematic JPEG file.

One solution would be to ignore the EXIF ​​data if the fields are empty.
Is it possible?

[Hans-Karl (jchbraun)]

The JPEG file...

[jazzynico (jazzynico)]

@Hans-Karl - The EXIF fields of your JPEG file are not empty, and resolution is set to 72x72 (see capture).
Importing it in Inkscape works as expected, and keeps the EXIF resolution.

[Hans-Karl (jchbraun)]

Very strange...

This picture come from an Epson scanner (and painful configuration), at 300 dpi.
I don't know why the exif command return 72x72 (also on my PC). But Gwenview don't shows any EXIF info, and XnView confirm my file at 300 dpi. See pictures.

Importing NO-EXIF.jpg on Scribus apply a 300 dpi resolution and scale my file at A4 dimensions automatically, as expected (see picture).

Is possible that exif command return 72x72 by default, if no field is marked on EXIF table ?

[Hans-Karl (jchbraun)]

Import in PhotoShop, it shows 300 dpi....

[Hans-Karl (jchbraun)]

Import in Illustrator, at right dimensions...

[Hans-Karl (jchbraun)]

Import in Inkscape R11962, strange result...

Perhaps my file is corrupted, could someone tell me his opinion on the situation ?

[Hans-Karl (jchbraun)]

Oups, the screenshot is here.

[su_v (suv-lp)]

On OS X 10.7.4:
- Apple's Preview.app v5.5.2 recognizes the resolution of 300x300 as well, as does ImageMagick (see attached screenshot).
- Inkscape trunk OTOH does import the image at 72 dpi, as does GIMP 2.8.2 (it reports 72x72 ppi for the resolution).

I don't know how the resolution is stored in the JPEG file, and why Inkscape as well as GIMP don't agree with other image viewers.

[jazzynico (jazzynico)]

The difference between Inkscape and other applications could be that we first try to extract EXIF information before using the dimensions stored in the JPEG format. I've just managed to get a correct resolution (300x300) by inverting the tests (png, jpeg, exif instead of png, exif, jpeg) in src/extension/internal/image-resolution.cpp.

Daniel, ~suv, are you aware of some cases where it could cause problems?

[Daniel Wagenaar (dawagenaar)]

On 12/19/2012 07:08 AM, JazzyNico wrote:
> The difference between Inkscape and other applications could be that we
> first try to extract EXIF information before using the dimensions stored
> in the JPEG format. I've just managed to get a correct resolution
> (300x300) by inverting the tests (png, jpeg, exif instead of png, exif, jpeg) [...]
Good detective work. Does the jpeg standard say anything about which
information should be used in case of conflicts? It is a sad state of
affairs that a jpeg+exif file can store multiple instances of mutually
conflicting resolution information. I suspect that whichever instance we
choose to prioritize, some users will conclude that it is the wrong
behavior, but certainly it is better to use the jpeg info over nonexistent
exif information.

[jazzynico (jazzynico)]

Quoting http://sylvana.net/jpegcrop/exifpatch.html :
"According to the Exif specification, the Exif APP1 marker has to follow immediately after the SOI, just as the JFIF specification requires the same for the JFIF APP0 marker!
Therefore a JPEG file cannot legally be both Exif and JFIF at the same time!"

And checking the file with the 'exiftool' command line utility tells us it has a JFIF marker, and not an EXIF one...

ImageResolution::readexif doesn't check the marker, and thus reads data that may not be exactly what we expect. And since ImageResolution::readjfif doesn't check the marker either, changing the test order doesn't fix the issue.

Therefore I suggest we check the existence of the appropriate marker in readexif and readjfif before accepting the format.

[jazzynico (jazzynico)]

> changing the test order doesn't fix the issue.

Correction: it does fix the issue because readjfif() reads cinfo.density_unit, which only exists with JFIF files.
Patch in progress, with a test on the saw_JFIF_marker field (more obvious than density_unit...) and some coding convention fixes.

[jazzynico (jazzynico)]

Patch committed in the trunk, revision 11992. Also adds support for JFIF resolution unit in dots/cm.

[su_v (suv-lp)]

> Patch committed in the trunk, revision 11992.

Confirmed on OS X 10.7.4 with r11992: (with default Inkscape preferences) the earlier discussed sample JPEG image now imports with the expected resolution / scaled size, and is approximately as big as the A4 page of the default new document.

[Hans-Karl (jchbraun)]

With R11992 for Windows :
I tested several files from different sources and it works fine.
Thanks!