GNOME Bugzilla – Bug 408185
Maker Notes gets trashed with gthumb
Last modified: 2008-04-11 13:10:39 UTC
Please describe the problem: I noticed that the size of the picture decreased when applying the Reset EXIF Orientation tool to some pictures. The difference is big, about 2.5 kb, for pictures from my Nikon camera while only 2 bytes for my Minolta camera. I attach two JPEG:s to reproduce the problem. Checking out the Maker Notes after the tool I see that some values has changed and for the Nikon picture I also get the following warning from exiv2: Warning: Makernote IFD has a next pointer != 0 (942669824). Ignored. Always the same number in the paranthesis might be a hint here. Steps to reproduce: 1. Check the size of the picture 2. Apply the Reset EXIF Orientation tool 3. Check the size again Actual results: Some Maker Note values changes and the size of the picture decreases Expected results: That the size would be the same and the Maker Notes are unaffected. Does this happen every time? yes Other information:
Created attachment 82593 [details] A picture taken with a Nikon Coolpix 950
Created attachment 82594 [details] A picture taken with a Minolta Dimage
I can't reproduce this problem using svn rev 1370. Jef's work in bug 361913 might have affected this. Can you try the svn version and see if the problem is resolved? To use svn: cd ~ svn checkout http://svn.gnome.org/svn/gthumb/trunk cd trunk vim README (to check library requirements) ./autogen.sh make su make install - Mike
Also, make sure that you have the most recent version of libexif. - Mike
I have the latest released version (0.6.13) Do you have the trunk version? I think this gthumb bug is related to this bug in libexif: https://sourceforge.net/tracker/index.php?func=detail&aid=1561926&group_id=12272&atid=112272 There is an interesting explanation about a possible cause of the problem. Some maker notes seems to have offset values between internal structures expressed as absolute offsets from the beginning of the JPEG file (to ease up seek() I guess). The picture uploaded there is a Konica Minolta also causing a 2 byte difference after the reset of EXIF Orientation in gthumb 2.9.1 I will upload my Nikon picture too since it is affected much more.
Created attachment 82644 [details] A picture taken with a Minolta Dimage New Minolta upload, the old one was already trashed by gthumb/libexif
If we are just changing the exif orientation tag, then why would the offset of the maker notes change? If we can establish that, then perhaps we can make this particular problem go away. (The problem remains if we implement generalized exif tag editing in the future.) - Mike
OK, I do see the file size shrinkage. libexif/gThumb doesn't seem to recognize any MakerNotes for the Minolta! It recognizes some, but not all, for the Nikon. - Mike
Adding generic EXIF editing in the future may also include adding and removing tags so I think we need to nail this problem first. I did take a look at libexif and could see that it supported maker notes from some other cameras but not Minolta and Nikon. The libexiv2 on the other hand had extensive support for Nikon and I think for Minolta too. Maybe its easier to move the Maker Notes support into libexif rather than convert it to libexiv2?! I'll take a look at it. Still libexiv2 might be the long term way to go.
It might be easier to move gThumb to exiv2. libexif does not seem to be as well maintained as exiv2, and applications seem to be moving away from it. - Mike
For all of the EXIF functionality it probably is. I am right now looking at moving just the support for Nikon and Minolta maker notes from libexiv2 to libexif to fix this bug for my cameras. I have found the following code in libexiv: switch (exif_data_get_type_maker_note (data)) { case EXIF_DATA_TYPE_MAKER_NOTE_OLYMPUS: data->priv->md = exif_mnote_data_olympus_new (data->priv->mem); break; case EXIF_DATA_TYPE_MAKER_NOTE_PENTAX: data->priv->md = exif_mnote_data_pentax_new (data->priv->mem); break; case EXIF_DATA_TYPE_MAKER_NOTE_CANON: data->priv->md = exif_mnote_data_canon_new (data->priv->mem); break; default: break; } And there is no initialization of priv->md elsewhere what I can see. So I was a bit puzzled about the maker notes that is actually seen in the gthumb EXIF display. After some debugging I found that it seems to detect Olympus instead of Nikon... I dig further into the code, it can't be too hard to fix this...
Mike, I found some code rot in regards of maker notes in libexif from what I can see. There is some Nikon maker notes code in there but it has been spread all over the place and can be found in the Olympus module rather than in its own. Detection is not clean either. This might take some time after all. // Joakim
Joakim, I've been thinking about metadata in general, and the most complete exif/iptc/xmp solution appears to be exiftool. Does exiftool show your maker notes properly? http://www.sno.phy.queensu.ca/~phil/exiftool/ I think I might modify the exif display tool to use exiftool instead of libexif or exiv2. The big benefit of exiftool is that we gain XMP read support immediately. - Mike
exiftool seems good at displaying meta data. Still the lingo seems non standardized. For instance I see the following info about Metering Mode: libexif 0.6.13: matrix libexiv2 0.10: matrix exiftool 6.00: Multi-Segment NikonView 6.2.7: Multi-Pattern I guess the latters are kind of matrix too but it is hard to know if the tag is correctly interpreted. When I tried to set the "DateTime" TAG with exiftool it also silently added a new TAG called "Date/Time Modified" which is correct I guess but not what I asked for. Other than that exiftool is written in Perl and do unexpected things under the hood for you it seems superiour. When use it from the tools menu on massive amounts of pictures I am not sure about the performance compared to libexif and libexiv2. I would abstract the meta data manipulation interface in gthumb to be able to switch between the available solutions. A bit more work initially but much cleaner to maintain. The only thing I am worried about is that a poor Meta manipulation support will trash the Meta data in the original file(s). So as a short term alternative to fix this bug I would like to add a preference checkbox to archive the original file before starting to do stuff to it. Maybe just by prepending it with a '.' or put it in a subfolder called .originals What do you think? Joakim
Joakim, I am sure the performance of exiftool will be slower, but most times we don't need much speed. We can add metadata caching (in local files) where we do need speed. I was planning to do that anyways, because sorting by Exif DateTime is quite slow right now. Regarding the archiving idea: I think it would be better to understand why corruption is occuring and fix that instead. The maker note corruption should only occur if the data is moved. I don't see why changing the orientation tag is making data move. It would be better to fix that issue, rather than adding more complexity in the save process. Any idea why resetting the orientation tag makes the maker notes move? - Mike
(In reply to comment #14) > exiftool seems good at displaying meta data. Still the lingo seems non > standardized. For instance I see the following info about Metering Mode: > > libexif 0.6.13: matrix > libexiv2 0.10: matrix > exiftool 6.00: Multi-Segment > NikonView 6.2.7: Multi-Pattern > > I guess the latters are kind of matrix too but it is hard to know if the tag is > correctly interpreted. The funny thing is that the Exif standard defines this mode as "Pattern", and none of the extraction programs call it that! - Mike
This is what I suspect: The implementations rebuilds all the Meta data tags when saving them, even when the change is of equal size. This makes it easier to add/remove/edit tags but works only as long as all data are interpreted correctly. When a Maker Notes are interpreted errornously or unknown data segments are encountered it may cause data loss or corruption. What you really would like is a exif-patch library here just changing the fixed size data structures in place. Anyway, when resetting the orientation tag the faulty interpretation of the Nikon/Minolta Maker Notes causes the wrong amount of data to be written back to the file. I am pretty sure it is only Maker Note specific data that is lost. I am also seeing change of values with in the Maker Note data which can be explained by the offset problem in the libexif library. That is that some maker notes uses absolute offset values for internal structures that is not corrected when the data is rewritten. This could also be the cause of the lost data. The archive feature is something else I am looking at that I thought could come in handy here with the obviously pretty buggy maker notes support around. // Joakim
I've outlined some metadata ideas at bug 409050. Please take a look. - Mike
The reason why the file size shrinks is ok. I have discovered that libexif removes leading and trailing garbage around ASCII strings resulting in a more packed IFD structure. Still I got values changing within the Maker Notes section when resetting the orientation bit in gthumb, so I continue to nail this down.
As can be seen below the Maker Notes has been both moved and changed. In 5 places below values has been changed and in one of them two bytes are added. Original Maker Notes in Nikon picture 0003a0 4e 69 6b 6f 6e 00 01 00 0b 00 02 00 02 00 06 00 0003b0 00 00 26 04 00 00 03 00 03 00 01 00 00 00 0b 00 0003c0 00 00 04 00 03 00 01 00 00 00 01 00 00 00 05 00 0003d0 03 00 01 00 00 00 00 00 00 00 06 00 03 00 01 00 0003e0 00 00 00 00 00 00 07 00 03 00 01 00 00 00 00 00 0003f0 00 00 08 00 05 00 01 00 00 00 2c 04 00 00 09 00 000400 02 00 14 00 00 00 34 04 00 00 0a 00 05 00 01 00 000410 00 00 48 04 00 00 0b 00 03 00 01 00 00 00 00 00 000420 00 00 00 0f 04 00 1e 00 00 00 50 04 00 00 00 00 000430 00 00 30 38 2e 30 30 00 00 00 00 00 00 00 00 00 000440 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 000450 00 00 00 00 00 00 00 00 64 00 00 00 00 00 00 00 000460 00 00 00 00 00 00 00 01 00 00 00 01 d4 bf 00 00 000470 00 00 00 00 41 09 00 00 00 70 00 00 41 09 00 00 000480 00 70 00 00 00 00 00 00 00 00 0b 0e 00 00 00 00 000490 00 00 02 06 02 54 02 36 02 09 01 ff 8b 84 00 63 0004a0 00 fc 00 00 0a 01 00 00 00 00 00 00 00 00 00 00 0004b0 00 00 00 00 00 00 00 00 00 00 00 02 61 be 00 47 0004c0 64 50 16 1a 18 26 11 01 26 00 ff 00 4f 55 32 0b 0004d0 20 ec 00 00 ff ff ff ff ff ff ff ff ff ff ff ff 0004e0 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff * Maker Notes in Nikon picture after reset of the orientation bit 4e 69 6b 6f 6e 00 01 00 0b 00 02 00 02 00 000270 06 00 00 00 86 00 00 00 03 00 03 00 01 00 00 00 000280 0b 00 00 00 04 00 03 00 01 00 00 00 01 00 00 00 000290 05 00 03 00 01 00 00 00 00 00 00 00 06 00 03 00 0002a0 01 00 00 00 00 00 00 00 07 00 03 00 01 00 00 00 0002b0 00 00 00 00 08 00 05 00 01 00 00 00 8c 00 00 00 0002c0 09 00 02 00 14 00 00 00 94 00 00 00 0a 00 05 00 0002d0 01 00 00 00 a8 00 00 00 0b 00 03 00 01 00 00 00 0002e0 00 00 00 00 00 0f 04 00 1e 00 00 00 b0 00 00 00 0002f0 00 00 30 38 2e 30 30 00 00 00 00 00 00 00 00 00 000300 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 000310 00 00 00 00 00 00 00 00 64 00 00 00 00 00 00 00 000320 00 00 00 00 00 00 00 01 00 00 00 01 d4 bf 00 00 000330 00 00 00 00 41 09 00 00 00 70 00 00 41 09 00 00 000340 00 70 00 00 00 00 00 00 00 00 0b 0e 00 00 00 00 000350 00 00 02 06 02 54 02 36 02 09 01 ff 8b 84 00 63 000360 00 fc 00 00 0a 01 00 00 00 00 00 00 00 00 00 00 000370 00 00 00 00 00 00 00 00 00 00 00 02 61 be 00 47 000380 64 50 16 1a 18 26 11 01 26 00 ff 00 4f 55 32 0b 000390 20 ec 00 00 41 53 43 49 49 00 00 00 20 20 20 20 0003a0 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 *
The libexif bug report is at http://sourceforge.net/tracker/index.php?func=detail&aid=1561926&group_id=12272&atid=112272.
(In reply to comment #19) > The reason why the file size shrinks is ok. I have discovered that libexif > removes leading and trailing garbage around ASCII strings resulting in a more > packed IFD structure. Still I got values changing within the Maker Notes > section when resetting the orientation bit in gthumb, so I continue to nail > this down. Do you mean that the MakerNotes still change, even if you disable the garbage removal feature so that the positions do not change? - Mike
Well 'removes' is a side effect from parsing all metadata into a data structure and then regenerate the meta data when writing it back. As you can see above the trailing FF:s has been changed to 20 after going through libexif. FF:s are potentially faster to burn into flash in the camera but does not matter for other medias. Tonight I did same check for exiv2 and exiftool setting the Exif.Image.DateTime from commandline, so to sum up: libexif - rewrites all metadata saving unused space but trashes maker notes exiv2 - patches just the correct bytes in the metadata exiftool - rewrites all metadata saving unused space, no harm done to Nikon maker notes So just from this little test we can see that exiftool does a pretty good job! // Joakim
Ok I have verified now that the offsets are wrong for all maker note tags of type ascii and rational. These point relative the start of the maker notes not the relative the start of the meta data. According to http://www.exiv2.org/makernote.html it seems that libexif detects the wrong Nikon version, there is no TIFF header preceeding the maker notes. Hope fully I can provide a patch for this to libexif in a couple of days. // Joakim
Created attachment 82976 [details] [review] temporary patch for libexif 0.6.13 to fix problem for Nikon E9xx and D1 cameras This patch fixes the makernote problems that I had with my Nikon Coolpix E950 and probably for the whole family of cameras including D1. Since the code didn't support relative offsets from main TIFF header I change it to *only* support this for maker notes which probably will cause problems for newer cameras like E5400, SQ, D2H, D70, D100, D200 which wants offsets from start off the maker notes like it was before. I supply this fix to the libexif project too for completion of they so would like to add support code for these older Nikon models.
I'm not a libexif developer, but I'll pretend that I am one :-) To really fix this problem, I would suggest that you patch libexif to not regenerate unchanged data (like the MakerNotes). That would avoid the whole problem for all devices. The patch above probably would not be accepted, since it adds support for one class of devices, but breaks it for another. - Mike
Yes, I know, but for people that can add a patch manually it will be helpful if they got one of the cameras that is affected, so thats why I posted it. I have now tested the other two cameras that I have, a Minolta Dimage X31 and my mobilephone, a Sonyericsson K608i and libexif does not support their MakerNotes at all so it just moves the whole area when regenerating the metadata. This is causing a "Possibly incorrect maker notes offsets (fix by -82?)" error in exiftool for the Minolta and for the mobilephone it seems to have removed the whole IFD1 section. Well, what I think is that there is no patch to fix libexif in the way you say because the fix would be bigger than libexif itself. It is simply a first generation r/w exif library that has had its purpose but no longer can keep up with the needs of functionality because of its fundamental design. It has served us well let it rest in peace. I rather propose I small utility lib, libexifpatch that would have some limited functionality, for example: - Only support for pure EXIF tags that is 4 bytes or shorter That would save the situation until an integration of exiftool or libexiv2 is in place. I could give it a stab and see what I find. I place the changes in the libgthumb/gth-exif-utils.c where it will replace only the write functionality, ok? The actual operations are placed in a separate file(s). // Joakim
(In reply to comment #27) > I rather propose I small utility lib, libexifpatch that would have some limited > functionality, for example: > > - Only support for pure EXIF tags that is 4 bytes or shorter Joakim, That might be worth exploring. I'm not sure how feasible it is. Anyway, you would need to look at: libgthumb/gth-exif-utils.c: set_orientation_in_exif_data* libgthumb/jpegutils/jpegtran.c: update_exif_orientation* swap_fields update_exif_dimensions update_exif_thumbnail update_exif_data * pointless duplication here! - Mike
ok, I'll do that. I also just realized that dates are stored as 20 characters long ascii strings but I think I can just check that a valid old date is already there before patching the data to avoid corruption. // Joakim
Some progress, I got a function that can patch IFD0 tags and write them back to the jpeg file without affecting unknown data. I still have some error checking and testing todo but I will probably be able to update my EXIF patch with this code and upload it for review during next week or it will take some weeks since I'll be away travelling.
Great to hear that you are making progress, Joakim! A reminder: the patch will need to work with IFD1 tags too, to be fully effective (since the orientation tag can be present in both IFD0 and IFD1). If you go away before the patch is done, please upload something so that we can understand what you are planning. - Mike
I think the main reason for this type of corruption is the fact that makernotes are stored in a vendor specific way, usually the ifd format (e.g. the same as the normal exif tags). And ifd offsets are always absolute (e.g. from the start of the data). Thus if the makernotes are not supported and therefore copied unchanged, those offsets are now invalid if the size of the exif structure is changed. This is not a problem if we can modify the bytes of the relevant exif tags directly. But that is not how libexif works (as was already mentioned above). I recreates the exif data and also tries to correct invalid data. The first issue is due to its design. For the second issue, there is an api function to disable this behaviour (I don't remember it's name right now). All these problems can be avoided if we don't use libexif for writing tags. Since the number of tags we need to write is relative small (e.g. orientation tag) we could parse the exif data ourself. For displaying tags, there is no problem and libexif can be kept. I was already working on parsing the tags directly (for changing the orientation tag more efficient as part of bug 361913). What I did is incorporating the code from "jpegexiforient" into gthumb. It works, but I encountered another bug in jpegtran, which causes a small problem with my patch. jpegtran writes jpegs with a JFIF marker, which is a violation of the exif standard. PS: For the Nikon makernotes detected as Olympus makernotes: I think the reason for this is that they use the same storage format and thus don't need a separate parser. But I'm not sure on that.
Jef, I'm glad you're following this bug! I've been exploring the idea of using file-based exiftool for general metadata reading and writing in situations where speed doesn't matter (i.e., viewing the metadata of individual photos, editing comments, etc - bug 409050), because it supports so many types of metadata (exif, xmp, icc, iptc, etc...) It would be great if we could have a corruption-free mini-libexif to read/write the in-memory tags used in the speed-sensitive jpegtran-type functions. - Mike
Created attachment 83279 [details] A code snippet to patch exif tags Hi, this is what I got right now. It works for my EXIF change date patch but is generic and allows both big and little endian. I believe I know a simple way how to support also FD1 that I will implement. This function will only support in place patching, never writing new or extended tag values. Let me know what you think, I will complete a patch later and possibly also replace the other gthumb libexif write calls, unless Jef already has something in place that is. // Joakim
Jef, I agree on what your saying, we should not use libexif for writing. For reading values it works fairly well and I also think having a different package for reading is a good sanity check that the writes were correctly applied too. For Maker Notes libexif is not up to date even for reading values as I understand it and the code base is not very easily to extend with new maker note support. Myself I'd like to "patch" in values like in my code snippet, not touching anything that we don't absolutely need to assuring most possible generic support. If this is not satisfactory for all write support in gthumb we need something else. I will go through with this patch utility just for the exercise and the fun of it anyway. // Joakim
Created attachment 83301 [details] [review] Code from jpegexiforient to change orientation tag This is the patch I mentioned in comment 32. It works for standard exif files (e.g. as produced by my digital camera), but not for JFIF files (e.g. as produced by an unpatched libjpeg/jpegtran). Note that the modification is not picked up by gthumb directly. (I don't know why.) The difference between the two formats is easily checked with "file": $ file *.jpg exif.jpg: JPEG image data, EXIF standard 2.2 jfif.jpg: JPEG image data, JFIF standard 1.01
Hi all, I've made a metadata-ideas branch in svn, which uses exiftool for the metadata tag display. It's really, really rough right now (no sorting, grouping, editing), but already it adds XMP, IPTC, ICC, TIFF-EXIF, reliable MakerNotes, and other good stuff. (A useful sample image is posted in bug 409050). We might as well use exiftool for file-based tag writing. What we need is a non-corrupting patch that will work with in-memory jpeg/exif data. Jef or Joakim, if you can provide that we can test it in the new branch. (Jef, you should consider requesting commit-access to svn, based on the quality of your earlier code contributions! I'm sure Paolo would support that...) - Mike
Mike please bare with me, I am not too familiar with Open Source environments, I spend more frustration on how to create a useful patch than I do on the programming part. I can probably find out what svn is, I am used to cvs, but a few explicit pointers on how to get your test branch out of the svn would help. Also I have created a new patch with a exif patcher utility that supports IFD0, IFD1, EXIF and GPS directories for both little and big endian. I upload it below. Let me know if the patch format is ok, I saw that the old patches I made were not. The exif-patch function has been tested to change the date from gthumb interface on images from 5-6 cameras and seems to work well. There is still some debug printouts so you can see what tags is seen correctly. Still TODO: - handle overlapping TAG names in GPS IFD for instance. - Completer error checking BR // Joakim
Created attachment 83343 [details] gthumb 2.9.1 patches adding non-destructive change EXIF date tool option Implements a new function that patches EXIF tags without affecting any other meta data or associated datastructures.
Mike, I got svn swinging, it really rocks compared to cvs! And patches seems to be a breath. I saw your instructions above and it was really easy to get going. Now the patches I submit will be much easier to create. Thanks! // Joakim
Joakim, Yes, svn is a joy to use after cvs :-) Anyway, to try the new branch (where I am experimenting with exiftool), you would use: svn checkout http://svn.gnome.org/svn/gthumb/branches/metadata-ideas This branch uses exiftool to display the exif data for a file. I think it works very nicely. To generate a patch, just use: svn diff > my.patch It's more convenient if you upload patches uncompressed (no .tar.gz required). Then you can see the patch in your browser when you click the link. Some more svn info is available at http://developer.gnome.org/tools/svn.html - Mike
Joakim, I'll look at modifying your patch and adding to metadata-ideas over the next 1-2 weeks. I need to add gnome-vfs support to it. Also, I would prefer to use an exiftool-based routine to write the date tags, rather than making our own file-based routines - it's easier to maintain that way. I plan to add exiftool-based file-write routines to metadata-ideas over the next 1-2 weeks. Jef, do you have any issues with exiftool? - Mike
Mike, roger that, I just needed the EXIF change date tool ahead of time to post date all my scanned images. When cleaning up the gth-exit-utils.c there might be an idea to have a naming convention for all functions showing that they belong to gthumb rather the libexif or some other external API. This confused me a bit in the beginning. I just compiled and tested the metadata-ideas I think it is very cool. Just some formatting things like that the tags are unsorted and such. The maker notes seems to be much more reliable and I got the Minolta maker notes shown for the first time in gthumb (not supported by libexif). // Joakim
(In reply to comment #43) > roger that, I just needed the EXIF change date tool ahead of time When you attach a patch, please make it clear if you see it as just a temporary patch, or as something that should be permanent. It helps everybody understand where things are going. > I just compiled and tested the metadata-ideas I think it is very cool. Just > some formatting things like that the tags are unsorted and such. Yes, the common tags should be sorted, and it would be nice to have a "translation table" to convert the non-standard exiftool tag naming. Does anyone have a good way to structure a bi-directional, constant translation table to do things like convert "IFD0:ModifyDate" <> "IFD0:DateTime" "IFD0:CreateDate" <> "IFD0:DateTimeDigitize" We can add sub-grouping that way too, for example "ExifIFD:ApertureValue" <> "Camera Settings:ApertureValue" It's trivial in Perl, but I'm not so clever in c. Suggestions welcome... - Mike
(In reply to comment #44) > When you attach a patch, please make it clear if you see it as just a temporary > patch, or as something that should be permanent. It helps everybody understand > where things are going. Sorry, I'll do that, I'm on a learning curve here. However, I am prepared to maintain any patch I submit so if you find writing through exiftool to slow or that it doesn't work as expected I think the patch could be used instead. Then I can connect the patch to the other EXIF write functions in gthumb. // Joakim
(In reply to comment #42) > Jef, do you have any issues with exiftool? I have some mixed feelings about using exiftool. It provides a lot of metadata with relative little effort, but on many distributions (e.g. Ubuntu) it is not installed by default. That means many users will not see *any* metadata at all. I think that is worse than the current situation (with libexif). I also think a metadata library (instead of an external tool) is a better choice for this task, but unfortunately a single good library does not exist (as far as I know)...
Created attachment 83393 [details] [review] Trunk SVN patch for write EXIF date dialog SVN generated patch for trunk version of gthumb. It adds an EXIF checkbox in the change date dialog allowing changing the EXIF date. A generic non-destructive write function for EXIF tags is provided.
(In reply to comment #46) > I have some mixed feelings about using exiftool. It provides a lot of metadata > with relative little effort, but on many distributions (e.g. Ubuntu) it is not > installed by default. That means many users will not see *any* metadata at all. I was hoping to make exiftool a mandatory requirement. It's already packaged for Fedora and Ubuntu, so it should be easy for the distro packagers to accommodate this. > I also think a metadata library (instead of an external tool) is a better > choice for this task, but unfortunately a single good library does not exist > (as far as I know)... No it doesn't... exiv2 is the closest, but it is missing a lot of features that are in exiftool, and it's written in c++, which I'm not familiar with. - Mike
> I was hoping to make exiftool a mandatory requirement. Never mind - that is a bad idea. I see a way to use exiftool if present, and fall back to libexif if it isn't. That should keep everyone happy. Joakim, I may need to use your full patch after all, if exiftool is not mandatory. - Mike
Cool, I'll start to work on making it more robust and maybe also add a directory argument so you can specify what IFDs you want to affect. I also found a bug yesterday when there is no matadata at all present that will crash gthumb that I'll will fix too. Any other additional requirements for the patch function? I was also thinking of the ability to add IFD:s and tags at the end of the metadata section so we do not affect the position of TIFF headers and maker note data. Maybe that can wait. // Joakim
Joakim, Why don't you include a fix for the exif reset tool (the original reason for this bug report) in your patch, using your tag-writing code? That's a convenient way to test the tag writing. Some svn advice: run "svn update" at least once a day. I'm committing changes to the metadata-ideas branch frequently. If you want to keep track of what's happening, subscribe to http://mail.gnome.org/mailman/listinfo/svn-commits-list. (The list is for all of gnome, but you can edit the options so it just sends gthumb-related mail only.) - Mike
Mike, I improved the robustness of the code and change the exif reset tool to use it. Since I am not entirely sure what it is supposed to do and for sure do not have any test content I hope that you can test it for me. It do write the value that it is called with and since the value does not change I think I am ok. :-) I upload the diff to trunk below. I changed the write_orientation_field() function and I saw it was called from many other places so maybe more functionality needs to be tested aswell. // Joakim
Created attachment 83501 [details] [review] Trunk SVN patch for change EXIF date dialog and reset EXIF orientation tools This is a more robust version of the EXIF tag writer function. The patch also changes to the Change Date and Reset EXIF Orientation tools to use this function.
Created attachment 83570 [details] [review] Trunk SVN patch for change EXIF date dialog and reset EXIF orientation tools Bugfix: IFD1 support never activated
Created attachment 83571 [details] [review] Trunk SVN patch for change EXIF date dialog and reset EXIF orientation tools
Created attachment 83581 [details] [review] Metadata ideas SVN patch for change EXIF date dialog and reset EXIF orientation tools Changes and Bugfixes - Fixed support for fixed size TAG types - Fixed bug in dynamic sized TAG values when shrinking - Changed to diff against meta ideas branch instead of trunk Sorry for the spam guys...
Created attachment 83638 [details] [review] Updated patch, needs work Joakim, I have modified your patch slightly: - I renamed patch_exif_tag to gth_minimal_exif_tag_write - I added the gnome-vfs support in the date change function - some minor tweaks It still needs some work. I get these compiler warnings: dlg-change-date.c: In function ok_clicked: dlg-change-date.c:173: warning: incompatible implicit declaration of built-in function strlen rotation-utils.c: In function write_orientation_field: rotation-utils.c:65: warning: passing argument 3 of gth_minimal_exif_tag_write from incompatible pointer type The second warning is the more serious one. I don't think the argument-passing of the data is quite right. In gth_minimal_exif_tag_write we expect: const char *data, int size, and in write_orientation_field we pass: &transform, 2 but is "transform" guaranteed to be 2 bytes in size on all architectures, as we claim? I don't think so (correct me if I'm wrong...). We need a cast operator at least, and maybe more to ensure it really is a 2-byte value. - Mike
Created attachment 83648 [details] [review] Improved patch You are right, I missed those warnings in all the compilation outputs. I have now improved robustness of the patch a bit further and also "typified" it to make life easier for the compilers "to do the right thing". Changes: - Overflow check in copy data section - Returns on wrong tag type in IFD rather than continue - Changed data argument to void * and inserted casts where apropriate // Joakim
OK, I'll commit that later today to the metadata-ideas branch - but first, what's your full name, so I can credit the patch properly? - Mike
Cool, my full name is Joakim Larsson. // Joakim
OK, the patch is in! (Well, it's in metadata-ideas, anyway. Paolo has the final say on whether this stuff gets merged back into trunk). To do next: we have to see where we've used set_orientation_in_exif_data instead of write_orientation_field, and see if that needs to be modified to prevent corruption. - Mike
*** Bug 408183 has been marked as a duplicate of this bug. ***
I only found a call in the catalog-web-exporter.c file and it seems to affect the EXIF tags the same way as the other libexif write calls are doing. The EXIF data is copied thus the offsets will probably be wrong here aswell. However this is a copy of the picture, not the original so it might not be prioritized to preserve the Maker Notes at all, rather to replace them with some Gthumb Maker Notes?! Either way, I don't think we can replace the call to set_orientation_in_exif_data in this case since it operates on in memory structures rather on a file. We need to rewrite the exporter to fix the Maker Note corruption there. // Joakim
I've removed set_orientation_in_exif_data. It was redundant with update_exif_orientation. I modified the web exporter to use write_orientation_field (for now, anyway) - this requires two file writes, but that's OK. Does the web exporter still suffer from corruption in a simple copy of the exif data? Also to look at: libgthumb/jpegutils/jpegtran.c: update_exif_orientation* swap_fields update_exif_dimensions update_exif_thumbnail update_exif_data - Mike
Yes it will corrupt some exif data in the destination file since it uses libexif for the copy which means that it parses the source EXIF data into structures and and reconstruct it for the destination copy. It is how libexif works and why it can't cope with unknown IFD formats containing these vendor specific offsets. // Joakim
Inherently all the functions that you mention will be affected by Maker Note corruption as it seems. It is not as simple as patch an exif value since the whole jpeg body is transformed. The exiftool will not help you in this case. The problem needs to be studied a bit further to see if there is a simple way to keep the exif data in place and just replace the jpeg compression data and then patch the values accordingly. Still I am not that deep into JPEG file formats yet that I can for sure say that it will be good enough. Maybe we should open separate bugs for the separate cases? // Joakim
I was afraid of that... oh well. I prefer to keep this bug open for now, it's more convenient that making new bugs. - Mike
Created attachment 84465 [details] [review] Trunk (rev 1843) patch with non destructive EXIF/TIFF support for Change Date and Reset Orientation dialogs Diff against gthumb trunk rev 1483 Features - Non destructive EXIF write function for JPEG and TIFF files - Adds JPEG/TIFF date writing to Change Date dialog - Adds non destructive JPEG/TIFF update of Reset EXIF orientation TODO - JPEG rotation code (Needs non destructive handling of APPn sections)
I am looking at fixing the JPEG rotation code but it will take some time. I think it is possible to preserve the APPn sections even with new data in the JPEG data sections. However I need to check into this a little further. Attached a patch against the trunk just for the minimal exif write functionality that works, because I want to follow the trunk closer. If I get the APPn handling to work I can make a patch against the meta ideas branch later. I have added TIFF support and are now checking file types before attempting to write the new values. // Joakim
Joakim, I plan to merge the minimal exif write stuff into trunk once the 2.10/2.11 branches are opened, which should happen in the next few weeks. Only bug fixes are going into 2.9.x, so that it can become a stable 2.10.0 release. (The minimal exif write code is too new to be considered stable.) In the meantime, please patch against metadata-ideas so that any changes can be tested. The patch in comment 68 seems to be missing some stuff that is already in metadata-ideas - like the 2-byte casting in write_orientation_field. - Mike
Mike, good to know. I will reapply the patch to the metadata-ideas branch together with any additional code for jpeg rotation and upload new patch(es) before your deadline if everything turns out well. // Joakim
Created attachment 84613 [details] [review] Simplyfied copy_exif_from_orig_and_reset_orientation() and removed EXIF parsing Mike, I think I fixed this function so it is not using libexif like parsing anymore for writing the tags. However when I checked the code I found that it did not only copy the exif data but the whole file so I also simplified it a bit... Please check it out and let me know if it works as expected. // Joakim
ok, I got it, I need to "insert" the exif data into the destination image from the source image, not just copy it. Thought it was too easy to be true.... :-) // Joakim
Mike, after some thoughts I want to propose the following. a) Strip off the original meta data when scaling an image b) Preserve the original meta data when rotating an image lossless Its no point in preserving photo time meta data after a downscale since the data does not match the image anymore. Look at the meta data after created thumbnails through the web exporter, it does still tell the size of the original picture. Instead a new set of meta data may be inserted with accurate data about the scaling and version of gthumb etc and maybe some data about the original in the comment field or similar. I feel this is better than preserving data that is not valid to the image anymore. For the rotation I dig into that now, let me know what you think. // Joakim
(In reply to comment #74) > a) Strip off the original meta data when scaling an image That happens now, although I would prefer that it kept the original metadata, even if it is slightly incorrect. See bug 323943. It would be best if we actually updated the exif dimension tags (and date modified tag) of course! There is too much interesting stuff in the metadata (like the original date) to just throw most of it away. People will complain. > b) Preserve the original meta data when rotating an image lossless That is what we do now (updating the orientation tag if required). It "just" needs to be fixed so the MakerNotes don't get corrupted. - Mike
(In reply to comment #73) > ok, I got it, I need to "insert" the exif data into the destination image from > the source image, not just copy it. Thought it was too easy to be true.... :-) > > // Joakim Sorry, I don't follow you - is the patch in comment 72 OK or obsolete? - Mike
(In reply to comment #76) > (In reply to comment #73) > > ok, I got it, I need to "insert" the exif data into the destination image from > > the source image, not just copy it. Thought it was too easy to be true.... > > Sorry, I don't follow you - is the patch in comment 72 OK or obsolete? > Please obsolete the patch in comment #72. If we need to preserve the meta data it needs more work, it does not work correctly. However I don't know how to change the status to obsolete until I get a fixed patch, I think I don't have the right permissions to do that without uploading a new patch?! I will add code to preserve the metadata and update the exif dimensions. I would like to add some info about gthumb version used for the transformations, what do you think? What tag to use? // Joakim
Joakim, Thanks for the clarification! I've marked all the old patches as obsolete. Exif has a "software" tag that would be appropriate for saving gthumb version info. However, changing that tag will mess up the MakerNote offsets for sure, won't it? (The dimension tags and DateTime tags are fixed length, the software tag is variable length.) - Mike
Mike, I think it might be possible to put new variable length tag values *after* the MakerNote data, ie last in APP1, but I need to look into that later and thinks it is a very useful info for people finding manipulated images not knowing where these originates from. // Joakim
Joakim, as promised, Paolo has branched gThumb so we have an unstable branch again (trunk). I have committed your exif-date-change tool and gth_minimal_exif_tag_write patches to trunk (svn rev 1524). Please test! - Mike
Ok. I am also almost done with the catalog_web_exporter.c patch for a non destructive copy of EXIF data to the destination thumbnails. This is done in the metadata-ideas branch. Should I move it to be a trunk patch too? // Joakim
Joakim, If the patch is working well for you and you are satisfied with it, we can put it into trunk now. If the patch is more experimental, we should test it in metadata-ideas first, so that it doesn't get released to real users too soon. - Mike
I got two patches that are mutual exclusive for the catalog_webexporter.c The first I made used mmap() and is supposedly faster to copy data between files. It also only requires two open files instead of three. However I got some out-of-memory problems with it for no obvious reason so I abandoned it and made a second one with a more tradional file io. When I checked out a fresh metadata-ideas branch and applied each of the patches they both worked out well. I need to add some general improvements from the latter patch to the older and test them again before uploading. I think the traditional approach is more portable but if performance is important the mmap version might be the way to go. Both patches preserves APP1 and APP2 and generates images that causes no complains from exiftool about makernote offsets anymore. I will upload both patches shortly and start to look at improved exif support for rotations too. // Joakim
Joakim, Let's use the "safer" method for the web exporter. Performance is nice, but not if we sacrifice stability. Let's try your patch in metadata-ideas for a week or two, then I'll move it to trunk. - Mike
Created attachment 85533 [details] [review] patch for metadata-ideas branch providing non destructive web catalog export So this is the "slow" patch that we should try out. It works by creating a temporary file which it writes selected parts from the original file and the thumbnail file. I believe it to work pretty well but it needs to be tested for different kind of JPEG images. It assumes that APP1 and optionally APP2 resides in the first 129 kB of the original file which seems to be true for all EXIF JPEG:s I've seen so far.
Created attachment 85539 [details] [review] Non destructive web catalog export patch Fixed a memory leak and removed an uncontrolled debug printout. // Joakim
Thanks Joakim, I'll look at it this weekend. In the meantime: what I really need is a fast function that can quickly scan the file headers and return "TRUE" if: 1) An Exif UserComment tag is present. or 2) An IPTC App marker is present. (Or better yet, only return true if an IPTC Caption-Abstract tag is present.) or 3) An XMP App marker is present. (Or better yet, only return true if an XMP UserComment tag is present.) Ideally it would work with any file (jpeg, tiff, png...), but that might be too hard. Just jpeg files would be enough. If I had this function (call it "has_embedded_comments"), I could skip reading comments from files that have no embedded comments. exiftool is very slow, so this would speed things up a lot in certain situations. Do you think you could take a look at that? - Mike
ok, I have a stab at it. I don't know much about the file structures of IPTC and PNG:s yet but I guess that is available at the net somewhere. // Joakim
Some quick thoughts about the patch: 1. copy_exif_from_orig_and_reset_orientation should be moved to libgthumb/gth-exif-utils.c. 2. We should add a call to gdk_pixbuf_get_file_info to retrieve the actual width and height, and update the exif info with the actual dimensions (since the image has probably been resized. 3. Note to myself: the resize tool needs height/width tag updating, too, now that I think about it. - Mike
I have had a look at the user comment tag detection. My conclusion is that it is rather complicated to get a fool proof scan function for this purpose, especially if you want to support more than just EXIF User Comments. To get a fast memory scanner function it needs to assume things and in this case the amount of things are quite many. For instance in the minimal write function the memory scanner assume that if I a TIFF header is found in a JPEG file it has to be the start of an EXIF APP1 section. This is not strictly speaking true but works most of the times and with additional error checks it will not set a tag unless a number of criterias are true. So it works. I think the user comment function based on the same philosophy is going to require quite some work to get working supporting all these other formats. I feel that I rather put in some work on the rotation code to fix that bug right now and let the comment detection mature a bit before we do something about it, if needed, because it is not a quicky to do if you want it fast. // Joakim
Joakim, Thanks for taking a look! I think you are right; it is low priority and probably not worth the effort. I'm still looking at your exporter patch - it takes a while, because I want to understand it. One small point: gthumb already has a "debug" function that you can use, in glib-utils.h. - Mike
Mike, the web exporter patch is essentially inserting the first source APP1 marker section it finds, and optionally also an APP2 marker section, between the destination JPEG SOI and the APP0 created by libjpeg in the scaling process. It does this by copying the apropriate data from the source and the destination files into a temp file and finally replace the destination file with the temp file. I am thinking of stripping out the destination APP0 JFIF section since it seems that it is not compatible with the EXIF format however I fail to find out what harm it can do. Some legacy decoders might have problems with it I guess. For the rotation code I want to replace the jcopy_markers_execute function in jpegutils/transupp.c to do a similar thing. Copy all marker sections from the source file and write them onto the destination file. The current code strips out the APP0 and the APP14 marker sections which probably does not affect the offset to any Maker Notes because I doubt there will be any APP0 or APP14 before an EXIF APP1 marker section since the EXIF standard requires the APP1 section to follow immediatelly after the SOI. Any thoughts on this? // Joakim
(In reply to comment #92) > I am thinking of stripping out the destination APP0 JFIF section since it seems Yes, please remove the JFIF data if EXIF data is present. Jef recently added some patches to remove JFIF sections when they conflict with EXIF, see bug 411861. > For the rotation code I want to replace the jcopy_markers_execute function in > jpegutils/transupp.c to do a similar thing. Copy all marker sections from the This code is Jef's area of expertise - I don't really understand it. If it's OK with Jef, go ahead. Jef, if you are following this, could you comment? Joakim, I'm working on getting your current patch into metadata-ideas today, for testing. - Mike
Joakim, I've committed your patch to metadata-ideas, with various changes: 1) I've added gthumb-style debug code (enable it at run time using GTHUMB_DEBUG=1;export GTHUMB_DEBUG) 2) I've added gthumb-style temporary file creation/removal. 3) I've moved some code into gth-exif-utils.c 4) Other minor stuff. I haven't changed the basic functionality itself. To do: A) The copy_exif_data function should update the x/y dimension tags. B) Strip out JFIF where it conflicts with EXIF. Thanks for the patch! - Mike
Joakim, Please take a look at bug 428725... - Mike
(In reply to comment #92) > the web exporter patch is essentially inserting the first source APP1 marker > section it finds, and optionally also an APP2 marker section, between the > destination JPEG SOI and the APP0 created by libjpeg in the scaling process. It > does this by copying the apropriate data from the source and the destination > files into a temp file and finally replace the destination file with the temp > file. You'll have to take care to output an APP0 JFIF marker if there is no EXIF marker available. > I am thinking of stripping out the destination APP0 JFIF section since it seems > that it is not compatible with the EXIF format however I fail to find out what > harm it can do. Some legacy decoders might have problems with it I guess. I remember reading somewhere unsupported APPn markers should be ignored, so I don't think legacy decoders should have problems with them at all. Many (recent?) digital camera's are already writing true EXIF files (with no JFIF marker). And I've never heard about any problems. > For the rotation code I want to replace the jcopy_markers_execute function in > jpegutils/transupp.c to do a similar thing. Copy all marker sections from the > source file and write them onto the destination file. The current code strips > out the APP0 and the APP14 marker sections which probably does not affect the > offset to any Maker Notes because I doubt there will be any APP0 or APP14 > before an EXIF APP1 marker section since the EXIF standard requires the APP1 > section to follow immediatelly after the SOI. Any thoughts on this? I don't understand why you want to change the jcopy_markers_execute function. That function is already patched to produce true EXIF files (when used together with the jcopy_markers_exif function). The APP0 (and also APP14) marker is not copied by this function on purpose, because the libjpeg code writes this marker itself already (unless explicitly disabled by my patch). If it is also copied from the source, there will be two APP0 markers in the final file. It is perfectly possible to have an APP0 marker before the EXIF APP1. All applications using an unpatched libjpeg library, are creating jpegs like that. But in practice, this violation of the EXIF standard is not causing any problems as far as I know. (In reply to comment #93) > Jef, if you are following this, could you comment? I am following the progress on the exif and jpeg related bugs. But I have less time available than I would like to spend these days :-(
(In reply to comment #96) > (In reply to comment #92) >> <snip> > You'll have to take care to output an APP0 JFIF marker if there is no EXIF > marker available. Yes I do. > > <snip> > I remember reading somewhere unsupported APPn markers should be ignored, so I > don't think legacy decoders should have problems with them at all. > > Many (recent?) digital camera's are already writing true EXIF files (with no > JFIF marker). And I've never heard about any problems. yes, thats my understanding too. > > <snip> > I don't understand why you want to change the jcopy_markers_execute function. > That function is already patched to produce true EXIF files (when used together > with the jcopy_markers_exif function). The APP0 (and also APP14) marker is not > copied by this function on purpose, because the libjpeg code writes this marker > itself already (unless explicitly disabled by my patch). If it is also copied > from the source, there will be two APP0 markers in the final file. As you probably know, this bug is about trashing the MakerNotes by relocation of the APP1 structure. This is a side effect of parsing all EXIF tags and write them back through libexif for instance. The offsets for some MakerNote values are relative the beginning of APP1 which is really stupid, other MakerNotes are relative the start of the MakerNote IFD which causes us less problems. Since libexif does not have an updated support for different MakerNotes some brands, like the Minolta Dimage image attached to this bug, gets its offsets wrong since the MakerNote IFD has moved relative the start of APP1. The rotation code has this problem too. Instead of trying to update the libexif with a lot of MakerNote knowledge which will be a never ending story, my suggestion is to copy the EXIF data without relocation from the source image to the rotated image. This can be done with all APP sections (except duplicate APP0 and APP14 of course). Lastly we patch the correct values into the required tags with the minimal_write function I have submitted for this purpose. This functionality is already submitted for the web exporter, see the patch attached for this bug. To reproduce the problem just rotate the Minolta Dimage image attached to this bug and check it out with exiftool. It will produce a warning like this: Warning : [minor] Possibly incorrect maker notes offsets (fix by -82?) > > It is perfectly possible to have an APP0 marker before the EXIF APP1. All > applications using an unpatched libjpeg library, are creating jpegs like that. > But in practice, this violation of the EXIF standard is not causing any > problems as far as I know. Thats my understanding too, so maybe I just leave it in for now. // Joakim
(In reply to comment #97) I think the confusing comes from the fact that we are talking about two different problems here. First there is the issue of the mutually exclusive APP0 (JFIF) and APP1 (EXIF) markers (which I fixed for jpegtran). And the second issue is the possible corruption of makernotes by libexif (which you are trying to fix). Although both problems appear in jpegtran, they are completely independent. Since EXIF IFD offsets are relative to the start of the EXIF marker, changing the location of *entire* APPn markers (which is exactly what my patch is doing) does not corrupt the makernotes. When you wrote: > For the rotation code I want to replace the jcopy_markers_execute function in > jpegutils/transupp.c to do a similar thing. Copy all marker sections from the > source file and write them onto the destination file. that sounded te me you were going to undo the effect of my patch.
(In reply to comment #98) > (In reply to comment #97) > > I think the confusing comes from the fact that we are talking about two > different problems here. First there is the issue of the mutually exclusive > APP0 (JFIF) and APP1 (EXIF) markers (which I fixed for jpegtran). Yes, that is good. > And the > second issue is the possible corruption of makernotes by libexif (which you are > trying to fix). Although both problems appear in jpegtran, they are completely > independent. Since EXIF IFD offsets are relative to the start of the EXIF > marker, changing the location of *entire* APPn markers (which is exactly what > my patch is doing) does not corrupt the makernotes. The Maker Notes data could be relative its own TIFF header if it has an TIFF IFD structure, or the EXIF APP1 TIFF header, or containing byte offsets from the start of the file (we reallt don't know since it is proprietary). See the quote from Phil you referred me to earlier: https://sourceforge.net/tracker/index.php?func=detail&aid=1561926&group_id=12272&atid=112272 This means that we can't move Maker Notes at all relative the start of the file. It is not sufficient to just keep the APPn markers intact but we also need to keep it in position within the file. I only worry about original images here since the rotation operation may render the Maker Note data useless for "dumb" applications unlike exiftool that both detects and offers to fix offset problems. Camera manufacturers often rely on windows software that we can't fix to interprete Maker Notes that has been offseted by gthumb or other creative tools. I have got a bunch of original images trashed before backup this way and why I try to fix it so it don't happens to others. > > When you wrote: > > For the rotation code I want to replace the jcopy_markers_execute function in > > jpegutils/transupp.c to do a similar thing. Copy all marker sections from the > > source file and write them onto the destination file. > that sounded te me you were going to undo the effect of my patch. Sorry, I didn't mean to offend anyone here. I have not followed the code fully yet but it seems that the Maker Notes are moved relative start of the file. This may be caused by libexif even if it is a 1 to 1 copy of the entire APPn markers(s) because some images has padding bytes between the offseted TAG data after the IFDs which are not restored in the copy thus the offset to the start of the file for subsequent data will change, ie the Maker Notes. My intention here has been to do a "stupid" byte copy of the whole data area upto the end of the APP2 marker to make sure the offset is not changed. An alternative solution I'd prefer would be to not rotate the orginal file at all but only set the orientation tag correctly and rotate the image at display time (in memory) accordingly. Thoughts? // Joakim
(In reply to comment #99) > The Maker Notes data could be relative its own TIFF header if it has an TIFF > IFD structure, or the EXIF APP1 TIFF header, or containing byte offsets from > the start of the file (we reallt don't know since it is proprietary). > > This means that we can't move Maker Notes at all relative the start of the > file. It is not sufficient to just keep the APPn markers intact but we also > need to keep it in position within the file. Are there really makernotes that have offsets to some arbitrary location (like the start of the file)? I am only aware of the following types (see [1] for instance): - makernote offsets relative to the start of the EXIF header - makernote offsets relative to the start of the makernote header This two types of makernotes follow the TIFF standard (with a minor modification for the second case) and are well documentation (the structure, not the values). This type of structure is certainly not damaged by moving entire IFDs. But if the structure is really unknown, there is no bulletproof method to prevent corruption. The only thing you can do is update the orientation tag (if it is present of course) or leaving the file unchanged. I'm afraid there is nothing else we can do in this case. Also keep in mind that libjpeg itself may alter the number of bytes and thus change the location of the makernotes. For instance a JFIF marker is inserted if such marker does not exist in the input. Are there really different types of makernotes or are we trying to solve a non-existing (or even a non-fixable) problem? [1] http://www.ozhiker.com/electronics/pjmt/jpeg_info/makernotes.html > I only worry about original images here since the rotation operation may render > the Maker Note data useless for "dumb" applications unlike exiftool that both > detects and offers to fix offset problems. How do you want to detect the difference between original and modified images? I think we can't do that and we have to treat all images equal. If the makernotes are already corrupted, it is too late and we can't fix them easily (maybe you are lucky and a more specialized tool like exiftool can still do it). BTW, moving the EXIF marker to the frond is not required if the camera writes true EXIF files. In that case the EXIF marker is already the first marker and my patch becomes basically a null operation. Only omitting the JFIF marker remains, but the input file shouldn't contain that marker. > Sorry, I didn't mean to offend anyone here. I have not followed the code fully > yet but it seems that the Maker Notes are moved relative start of the file. > This may be caused by libexif even if it is a 1 to 1 copy of the entire APPn > markers(s) because some images has padding bytes between the offseted TAG data > after the IFDs which are not restored in the copy thus the offset to the start > of the file for subsequent data will change, ie the Maker Notes. My intention > here has been to do a "stupid" byte copy of the whole data area upto the end of > the APP2 marker to make sure the offset is not changed. You didn't offend anyone (certainly not me). I didn't understand why you wanted to change that code again, because it does not corrupt makernotes at all (I did not take into account offsets relative to the start of the file). That's why I wanted to make sure you understand my patch and the side effects of changing it again. Now I know you were trying do deal with offset relative to the start of the file. > An alternative solution I'd prefer would be to not rotate the original file at > all but only set the orientation tag correctly and rotate the image at display > time (in memory) accordingly. That's up to the end user. I also prefer to keep my pictures unmodified (exactly to prevent the kind of problems we are discussing here), but sometimes I need the physical rotation too. For instance my standalone DVD player doesn't do EXIF autorotation.
Jef, I got an insight, see last section before waste energy on arguing ;-) (In reply to comment #100) > (In reply to comment #99) > I am only aware of the following types (see [1] for instance): > > - makernote offsets relative to the start of the EXIF header > - makernote offsets relative to the start of the makernote header > > This two types of makernotes follow the TIFF standard (with a minor > modification for the second case) and are well documentation (the structure, > not the values). This type of structure is certainly not damaged by moving > entire IFDs. They are damaged if the offsets spans IFD:s that contains padding bytes that are removed in the copy process. > > But if the structure is really unknown, there is no bulletproof method to > prevent corruption. The only thing you can do is update the orientation tag (if > it is present of course) or leaving the file unchanged. I'm afraid there is > nothing else we can do in this case. I disagree, we know that Maker Notes are always encapsulated in APP1 and APP2. With this fact we can make sure that these are not moved relative start of the file and know that the offsets are still ok. The EXIF standard defines Maker Notes as 'unknown' of 'any' size but within the 64kb per marker as I interprete it. >Also keep in mind that libjpeg itself may > alter the number of bytes and thus change the location of the makernotes. For > instance a JFIF marker is inserted if such marker does not exist in the input. That is why libjpeg can't be used to build the file. Instead we can cut and paste from the original file and from the processed file like this psudo code: a) Copy original file upto end of last APPn maker. b) Copy processed file from just after the JFIF APP0 marker to EOF c) Paste it into the target file > Are there really different types of makernotes or are we trying to solve a > non-existing (or even a non-fixable) problem? Yes I got the problem myself and this is the only reason I started coding for Gthumb, at least at this moment. > > > I only worry about original images here since the rotation operation may render > > the Maker Note data useless for "dumb" applications unlike exiftool that both > > detects and offers to fix offset problems. > > How do you want to detect the difference between original and modified images? > I think we can't do that and we have to treat all images equal. If the > makernotes are already corrupted, it is too late and we can't fix them easily > (maybe you are lucky and a more specialized tool like exiftool can still do > it). No, I wanted to prevent corruption of original images. Especially since we import directly from camera people will probably be tempted to correct the orientation right away, thus no backup. > Now I know you were trying do deal with offset relative to the start of > the file. No, the padding bytes when offsets are relative the EXIF header is the biggest problem. > > An alternative solution I'd prefer would be to not rotate the original file at > > all but only set the orientation tag correctly and rotate the image at display > > time (in memory) accordingly. > > That's up to the end user. I also prefer to keep my pictures unmodified > (exactly to prevent the kind of problems we are discussing here), but sometimes > I need the physical rotation too. For instance my standalone DVD player doesn't > do EXIF autorotation. Cheezes, you really got me there, you are right! I didn't realize that the physical transform could be turned off even though I'v seen the checkbox for it so many times!! I thought that it worked like it does in "Image Viewer" where you loose the orientation when you leave the session. So, then the only problem as I see it is that it is checked by default causing problems for people who are in a hurry trashing their Maker Notes if they have offsets realtive the EXIF header. Mike, I am not pursuing this issue anymore unless you guys want me to, gthumb does what I need it todo for my workflow now. Thanks for walling this issue with me Jef! // Joakim
(In reply to comment #101) > I got an insight, see last section before waste energy on arguing ;-) The new rotation mode (modifying the EXIF orientation tag instead of the physical transformation) was implemented some time ago (bug 343867). This bug is about the physical transform and all the time I assumed you were aware of that. > I am not pursuing this issue anymore unless you guys want me to, gthumb > does what I need it todo for my workflow now. If you don't mind, I would like to get some additional comments from you (or anyone else of course) to fix this bug also. > (In reply to comment #100) > > (In reply to comment #99) > > I am only aware of the following types (see [1] for instance): > > > > - makernote offsets relative to the start of the EXIF header > > - makernote offsets relative to the start of the makernote header > > > > This two types of makernotes follow the TIFF standard (with a minor > > modification for the second case) and are well documentation (the structure, > > not the values). This type of structure is certainly not damaged by moving > > entire IFDs. > > They are damaged if the offsets spans IFD:s that contains padding bytes that > are removed in the copy process. Sorry, I should have written "APPn markers" instead of "IFDs". That makes a huge difference. I don't move individual IFDs, but the entire APP1 marker block containing them. I'm still convinced (see exif specification [1]) that this operation will leave the offsets intact, even if there is some padding between the IFDs (or sub IFDs). I'm not exactly sure what you mean with "padding". Can you explain? [1] http://www.exif.org/Exif2-2.PDF (section 4.5.4) You can easily verify this by commenting out the call to update_exif_data in jpegtran_internal (libgthumb/jpegutils/jpegtran.c). This will disable all libexif processing (and thus the possible corruption cause by it). I'm quite sure exiftool will not detect offset errors anymore. (Note: gthumb will show the double rotation bug, because the orientation tag is not updated anymore.) Theoretically, the exif data may continue in the APP2 marker, but I don't understand how that is supposed to work and I don't have such images to test with. > Yes I got the problem myself and this is the only reason I started coding for > Gthumb, at least at this moment. Can you provide some example images? > Cheezes, you really got me there, you are right! I didn't realize that the > physical transform could be turned off even though I'v seen the checkbox for it > so many times!! I thought that it worked like it does in "Image Viewer" where > you loose the orientation when you leave the session. That's why there is a large tooltip with more informartion for the checkbox. And it is also mentioned in the manual. :-) > So, then the only problem as I see it is that it is checked by default causing > problems for people who are in a hurry trashing their Maker Notes if they have > offsets realtive the EXIF header. It is the default setting for backwards compatibility with previous versions of gthumb (which do not have the exif transform mode).
(In reply to comment #101) > Cheezes, you really got me there, you are right! I didn't realize that the > physical transform could be turned off even though I'v seen the checkbox for it > so many times!! A lot of work went into that checkbox, as Jef alluded to (the infamous bug 343867)... > Mike, I am not pursuing this issue anymore unless you guys want me to, gthumb > does what I need it todo for my workflow now. In the long term, we need to fix this - corruption after rotation is highly undesirable. It would be helpful if you could re-attach "before" and "after" images, and the exiftool reports from them, that clearly show the corruption that happens to the makernotes after a physical rotate. (There has been so much confusion above, that I think new images would be helpful). Then you, me, Jef, or someone else can come back to this at their leisure and fix it :-) - Mike
I tried the rotation tool with the update_exif_data function disabled (as I described in comment 102). And exiftool does not report any offset warnings. That seems to indicate libexif is the only source for corruption now.
(In reply to comment #104) > I tried the rotation tool with the update_exif_data function disabled (as I > described in comment 102). And exiftool does not report any offset warnings. > That seems to indicate libexif is the only source for corruption now. To backtrack a bit: this bug has always been about libexif corruption (not gThumb flaws), and how to avoid using libexif Joakim indicated that libexif is flawed in its basic design, and it seems to me that libexif is not being well maintained, so Joakim wrote a nice function (gth_minimal_exif_tag_write) that bypasses libexif for simple tag updates, which is now used in write_orientation_field. This works great for simple file-based tag updates, but we have not address the analogous "in-memory" functions. So... I think that the only remaining question is: Can the functionality of update_exif_data be replaced with similar "minimal" tag-updating functions that bypass libexif? Apologies in advance if I'm overlooking anything! - Mike
Created attachment 86958 [details] Comparison between trashed and non trashed image (hexdump) Jef, here is a manual analysis of an image before and after flipping it in gthumb with physical transform enabled. The images was too big for attachments (I saw it too late) but can be downloaded here: http://bildrulle.nu/tech/pines_org.jpg Original picture http://bildrulle.nu/tech/pines_cpy.jpg Flipped picture As can be seen the Maker Notes are moved and not adjusted. This could be ok but it isn't as reported by exiftool: Warning : [minor] Possibly incorrect maker notes offsets (fix by -82?) In addition I have verified that values are read out wrong for some Maker Notes. And yes, we are hunting a short comming in libexif which requires the Maker Notes to be known. The best solution I could come up with was the cut and paste method described above together with the minimal write function I have written. Unfortunality it results in some code you wrote to be deprecated because of libexif. We also looked into adding support for more Maker Notes in libexif but it seems to be too massive amount of work. // Joakim
(In reply to comment #105) > Joakim indicated that libexif is flawed in its basic design, and it seems to me > that libexif is not being well maintained, so Joakim wrote a nice function > (gth_minimal_exif_tag_write) that bypasses libexif for simple tag updates, > which is now used in write_orientation_field. This works great for simple > file-based tag updates, but we have not address the analogous "in-memory" > functions. > > So... I think that the only remaining question is: > > Can the functionality of update_exif_data be replaced with similar "minimal" > tag-updating functions that bypass libexif? Updating tags "in-memory" shouldn't be more difficult then its file-base counterpart. Only the transformation of the embedded thumbnail is more complicated. Since the size of the transformed thumbnail is not necessary equal to the size of the original thumbnail, IFD1 has to be modified in a more complex way. So I think once we know how to update the thumbnail, the rest is only a matter of some variants of already existing code. If I'm reading the specs [1] right, it involves updating the JPEGInterchangeFormat and JPEGInterchangeFormatLength tags in IFD1, the jpeg data stream itself and the APP1 marker length. Am I missing something else? [1] http://www.exif.org/Exif2-2.PDF (section 4.5.5)
(In reply to comment #106) > here is a manual analysis of an image before and after flipping it in gthumb > with physical transform enabled. > > As can be seen the Maker Notes are moved and not adjusted. This could be ok but > it isn't as reported by exiftool: > > Warning : [minor] Possibly incorrect maker notes > offsets (fix by -82?) > > In addition I have verified that values are read out wrong for some Maker > Notes. And yes, we are hunting a short comming in libexif which requires the > Maker Notes to be known. The best solution I could come up with was the cut and > paste method described above together with the minimal write function I have > written. Unfortunality it results in some code you wrote to be deprecated > because of libexif. We also looked into adding support for more Maker Notes in > libexif but it seems to be too massive amount of work. Your analysis clearly shows libexif is flawed, but that is nothing new of course. That issue should be fixed once we introduce the in-memory variant of your gth_minimal_exif_tag_write function into the jpegtran code. That is the reason why I asked to test with the libexif stuff disabled. That would leave the entire exif block intact (e.g. writing an exact copy of the input, like your cut-and-paste method would do). I tried that and it works very well (see comment 104). Only the jpeg data stream is modified (which is unavoidable for an image rotation of course) as expected. But my patch is still required in this case, otherwise a JFIF marker is inserted before the EXIF marker! Once the in-memory patching is coded it can replace the libexif stuff, with no side effects because the size remains unchanged. Only the thumbnail is a possible source for problems, because it can change in size. But I think this can do no harm because the jpeg data stream comes immediately after it. But more on that next. If I take a closer look at the structure of your Minolta Dimage images (exifprobe is a nice tool for that task), I noticed the file contains three images: 1. Primary image (2048x1536) 2. Thumbnail image (640x480) 3. Reduced resolution image (160x120) Image #1 and #3 are normal jpeg images. (I knew jpegs can contain multiple images, because the libjpeg fileloader supports them, but I had never seen them before.) No problem so far. But image #2 is somewhat special because it has an incorrect SOI (0x00d8 instead of 0xffd8). This is recognized and documented [1] by the exifprobe tool. (Note: It is possible that libjpeg is unable to read this image because of the invalid SOI marker. But I don't know for sure.) The other problem is that the makernotes refer to this image using absolute offsets. After rotation, this offset points to an incorrect location, because the primary image has changed (not the exif data!). That means without any interpretation of the makernotes, this type of corruption is unavoidable. Even the cut-and-paste method will have the same problem! [1] http://www.virtual-cafe.com/~dhh/tools.d/exifprobe.d/exifprobe_output_toc.html
Jef, I broadly agree with your comments. Some observations: 1 - Does the embedded thumbnail really have any value? It would be a lot simpler to delete the thumbnail entirely by setting the "Next IFD Offset" tag in the Exif IFD to 0x00000000 (which has the effect of disabling IFD1). Nothing has to move then. (There will be some unused garbage data in the file, but I don't think it would cause any problems other than wasted space.) I never really trust the thumbnails anyway, personally, after seeing how unevenly various viewers use them and remember to update them after changes. 2 - Makernote thumbnails: as you say, the absolute pointers will be corrupted. Even if we corrected the pointers, the thumbnail would be wrong anyway because we don't update it to reflect the transformation! If we really care about this, we could try to identify all "extra" images in the file (keeping in mind the odd SOI markers), and then see if we can locate a single matching Makernote tag that references it (either by a matching absolute or relative pointer), and "remove" that pointer (set the pointer and/or tag id to zero?). But that is a lot of work and hard to test... but not impossible... - Mike
Just a few thoughts: a) If the thumbnails are *after* Maker Note data it would be safe to rotate those as well hence the Maker Note offsets will not be affected. b) if the thumbnails are *before* the Maker Note data it might be safe to place the new thumbnails after maker Note Data and disable the old thumbnail so it is left in place as garbage data hence the Maker Note offsets will not be affected. c) For Maker Note thumbnails I can agree that it would be nice to stripout those if possible but that probably require some knowledge about the Maker Note structure which we don't have infinitelly, just for a few/old models through libexif. The base idea I had was to create something that would work regardless of the format of the Maker Notes but forward references to the primary image would be tough to support. Maybe the best option is to remove the Maker Notes and embedded thumbs altogether when applying a physical rotation? Or to replace it with a Gthumb Maker Note and a fresh thumbnail. // Joakim
Created attachment 91556 [details] [review] Web exporter patch, against trunk Hi Joakim, I'm trying to merge things out of metadata-ideas and into trunk, including the web exporter patch. A cleaned-up version is attached. It doesn't seem to work with some of my images. A sample image is attached below. Could you take a look at it? The patch applies against trunk. - Mike
The sample image is at http://www.avtechpulse.com/downloads/test.jpg - Mike
Never mind about my previous comments. The sample image had both exif and xmp data. - Mike
trunk uses exiv2 now, so I think most of these issues have gone away. Closing as obsolete. Please file new bugs if trunk/exiv2 have issues. - Mike