Add pictures to dive feature issue - EXIF data not read correctly

[sfuchs79]

When trying the picture feature the first time I immediately found a couple of pictures where the EXIF date/time seems to be not evaluated corretly when importing the pictures to a dive. Three examples here with screen shots and files...

Windows explorer:

Subsurface:

Files:

Pictures.zip

To be very open I didn't look at the pictures with any specific EXIF info viewer up to now. So maybe the EXIF info is really incorrect or corrupted. But Windows explorer shows it correctly.

[dirkhh]

Thanks @sfuchs79 -- @atdotde has worked on the EXIF handling in the past... I remember that there were different fields to chose from - but since the one that we are choosing currently clearly contains a timestamp of 0, maybe in that case looking at one of the other fields is the better approach.

Robert, given the files that Stefan posted, is that something you could look at?

Thanks

[mturkia]

Function where the error occurs is picture_get_timestamp. For some reason exif.parseFrom returns an error. The returned error code is:

// No JPEG markers found in buffer, possibly invalid JPEG file
#define PARSE_EXIF_ERROR_NO_JPEG 1982

And when looking into the not ok images, they are indeed missing proper JPEG magic. And indeed, they are in fact TIFF images, not JPEGS. Incorrect file extension...

[sfuchs79]

You're most probably right in a way that the "nok" files don't have a propper JPEG header. I neither have the expertise nor the right tools at the moment to check this.
What is fact is that all the tools I tried under Windows (ACDSee, Gimp,...) and Linux (Thunar, ImageMagic) are more tolerant in two ways: They claim that the pictures are JPEGs and they read the Exif info correctly.
Also Subsurface somehow claims that it can handle TIFF files but in fact we fail because we can't process the Exif inside a TIFF.

Now comes the more interesting part:
If I disable the JPEG sanity checks in the exif.cpp of Subsurface I can read the Exif data correctly also from my "nok" files.
I'm talking about these lines:
https://github.com/Subsurface-divelog/subsurface/blob/master/core/exif.cpp#L420
... down to 439.
Unfortunately with this change we still can't read Exif from a "real" TIFF.
This makes me believe that my "nok" files are closer to JPEG than to TIFF.

Finally I believe that being able to read Exif from any file that can have Exif data and that we support from image format point of view should be our goal and that this may be not too difficult.
I will try to further investigate - any help would be appreciated.

[mturkia]

My initial analysis was incorrect - too tired at the time. Anyway, do you import the sample images properly when you disable the exif sanity check?

If you use 'file' command in Linux, that reports information about the files by analyzing the magic headers. It does mention both JPEG and TIFF for the image data for all files, however.

You can check the start of the file in hex e.g. using the following one-liner:

$ for i in .jpg; do echo $i; xxd $i | head -n 2; done
Test_EXIF_nok_1.jpg
00000000: ffd8 ffe1 46bd 4578 6966 0000 4949 2a00 ....F.Exif..II.
00000010: 0800 0000 0a00 0f01 0200 0800 0000 8600 ................
Test_EXIF_nok_2.jpg
00000000: ffd8 ffe1 32eb 4578 6966 0000 4949 2a00 ....2.Exif..II*.
00000010: 0800 0000 0a00 0f01 0200 0800 0000 8600 ................
Test_EXIF_ok_1.jpg
00000000: ffd8 ffe0 0010 4a46 4946 0001 0101 0048 ......JFIF.....H
00000010: 0048 0000 ffe1 2ae0 4578 6966 0000 4d4d .H....*.Exif..MM

But the problem is actually at the end of the file. There is some additional data after the JPEG end marker. And that is the reason our Exif library claims improper data...

[sfuchs79]

Yes, for my "nok" JPG everything works ok if I disable the sanity check.
I meanwhile did a similar analysis as you @mturkia - just Windows user style by installing a HEX editor ;-)

With one quick check I also think that we can't handle TIFF images at all. Neither read the Exif data nor process the image correctly.

I have these suggestions and would prapare a PR for this but would like to hear some comments first:

• Disable the sanity check in Exif handling for the EOI tag (but keep SOI "ffd8" check) because this imho doesn't bring added value. Existence of correct Exif header is checked good enought later during processing.
• Disable TIFF and PNM handling by changing file filter of file dialog because we can't really process them
• For the case that we again fail to read Exif from JPG give a useful message to user like "No Exif date/time found." instead of printing some 1970 date string.

[atdotde]

Just a quick comment: Except for the exif data analysis, we don't handle images at all. All the lifting is performed by Qt. We can open whatever Qt recognises as an image. And that can be platform dependent (or even be enhanced by plugins), all details are on http://doc.qt.io/qt-5/qimage.html#reading-and-writing-image-files

The correct way to assemble the file filter is to use QImageReader::supportedImageFormats() as we already do in qthelper.cpp. So you can probably simply cut&paste code from there or even better factor out a function that returns an appropriate file filter.

[atdotde]

The exif handling we got from another project. You might want to contact the author of that project if you think there's something wrong with handling those images.

[sfuchs79]

Good point. When doing this I found this:
mayanklahiri/easyexif#17

I'm also going to ask about TIFFs... :-)

[sfuchs79]

Closed via #384