GNOME Bugzilla – Bug 769820
Cannot enter Iptc information when no metadata is available and fails to write it to file when it is.
Last modified: 2017-07-08 09:47:13 UTC
Created attachment 333203 [details] [review] Fixes iptc issues. Iptc data cannot be entered on any file that doesn't have existing metadata even when you should be able to for example to add licensing or contact information for redistribution. When it is available (the metadata) writing back the iptc data to a file fails to get updated. Looking at the code in metadata.c shows that whole section is chopped out or missing the part about writing the iptc fields back into the image metadata. This patch fixes the issues mentioned.
Created attachment 333211 [details] [review] Removes the need for a write iptc button in the metadata dialog This patch removes the write iptc data button and updates the iptc fields on exit with the close button instead. This patch complements the first patch about.
Thanks, that was a missing TODO for 2.10 :) Now that the plug-in can actually change things, and the weird write button is gone, shouldn't we switch to "Cancel" and "OK" buttons so users can change their mind?
I had to fix it to work on and test the exiv2 code for webp files. Personally i would leave it as is, it updates transparently. If someone else wants those buttons then by all means change it. On a different subject are you ever going to add that other patch for layer mask or did i waste my time.
You didn't waste your time. I hacked up an infrastructure for dialog defaults locally but I'm so short on time currently that GIMP hacking gets little attention. The patch will go in with some changes.
Good it's the only thing in life to really get me angry, time's the only currency you can't get more of. As long as there's a decent way to select which kind of mask to apply is all that's really needed.
I took another look at it (the dialog) and the problem is that ok/cancel doesn't make sense with whats there. There should be two dialogs (editor for iptc fields and one to view the metadata). Also clicking ok or cancel on the first two tabs would feel really weird.
Created attachment 333786 [details] [review] Seperates viewer and editor and adds export of iptc to equivalent xmp New update fixes some issues and separates the editor and viewer components. Also adds export of iptc tags to equivalent xmp so that formats that support one or the other will include all the appropriate metadata, especially as it relates to copyright information.
Cool, that looks like a much better idea! One quick comment: in plug-ins/common, please don't edit Makefile.am, instead, edit plugin-defs.pl and run mkgen.pl.
Yeah that old dialog was a mess. This is neater. Didn't know that.
Plus this way makes it easier to add to the editor in the future.
This needs an import / export feature in the editor part to be able to have various files depending on usage and copyrights. That way you don't waste time filling out fields all the time for each image. Also it probably should be move into some action section of code to make it callable for the day that there is some kind of actions playback / recorder system (like Photoshop actions do). I've been wracking my brains on thinking up of a name such a system that wouldn't use the word action, or recipe, or anything similar lol.
Created attachment 337290 [details] [review] Splits metadata dialog into editor & another one for viewer, adds import/export to file of the metadata. Splits metadata dialog into an editor & another one for theviewer, this patch also adds import/export of the metadata as xml data to file. Still edits the Makefile in common though haven't had a chance to look the gen script, i needed a quick edit today to add the import & export functionality.
Note one caveat it only imports and exports the metadata fields available from the editor.
I've started work on adding iptc extensions and making the editor portion compatible with the fields used in Photoshop (as well as keeping those that already). Unfortunately the fields as currently defined weren't the same or missing and sharing with photoshop users causes problems because of those differences as both won't see the same fields set for the same data entered.
Created attachment 338833 [details] [review] Adds metadata namespace support and gimp_metadata_set_from_iptc Primarily adds namespace support and adds GIMP namespace to use in the format Xmp.GIMP.tagname tagname being whatever string/struct (per xmp spec) you want.
Thanks, looks like that patch fixes bug #727270 :)
*** Bug 727270 has been marked as a duplicate of this bug. ***
Why does the patch add XMP namepaces? Is that somehow related to the IPTC part of the patch?
Because it's going to be needed when i am done with the next editor patch. I figured i'd create a namespace for GIMP specific metadata while i was at it. The DICOM one is required for the DICOM namespace for the editor as well as i am adding that it in. Is related to the iptc, no, it isn't. I wasn't thinking about that, i just noticed there was no function for it so i figured i'd add it in while i was working on all of this metadata business.
Maybe this should be separated into different patches, though?
(In reply to Michael Schumacher from comment #20) > Maybe this should be separated into different patches, though? No not really, it will be needed for the metadata editor extension i am working on. Unfortunately i have't had the time in months to work on it, it's about 60% complete (though i need to update it for the newest git pull) and it will add all the items found in photoshop.
FYI still working on the patch only have the iptcExt fields left to do. If anyones interesting in testing what i do have i can put up a patch but it's definitely still a WIP.
Created attachment 352871 [details] Current screenshots of the new metadata editor. Screenshots to show the status of where i'm at with this. Almost done the callbacks to write the iptc extensions tab.
Finished working on the metadata editor. Adds the ability to edit the same metadata tags as does Photoshop / Lightroom / Bridge, and several extra iptc that were originally used by GIMP, and GPS location data as well.
Created attachment 353946 [details] [review] New metadata editor and viewer code patch
Holy shit :) That looks great! Some comments: - please use g_snprintf() instead of sprintf() - the file is pretty huge, I already see it being split in several files in the future, we should probably move it all to a new plug-ins/metadata/ folder - we have a policy to not use // comments No time for a detailled review of the code yet, but I guess we should simply get this into master asap.
Thanks, that was a lot of painful work, hopefully i haven't missed anything major or too many bugs lol. Taking a break for the next day or so, too much coding in the last few weeks. I'll update the patch accordingly in the coming days. I'll also upload a test image to go along with the patch to test with. Any preferences on format? Agreed on splitting it off to it's own folder. Sounds good about getting it to master. For some reason i can no longer log into git, i remember i did something to fix that last time it happened but can't find the info anymore.
Created attachment 354149 [details] [review] New patch for metadata editor and viewer plugins. @Michael Updated to move the code into separate folders and fixed according to your requirements (hopefully haven't forgotten anything in the process). Also took the time to fix some issues with the import/export code for people who want to create a metadata template file. The data is stored in plain text xml format.
Created attachment 354164 [details] Sample wilber image with full test metadata. Sample image to test the viewer and editor plugins with complete metadata set.
Created attachment 354166 [details] [review] Latest patch to metadata & viewer plugins
Created attachment 354167 [details] [review] Adds fix/update to dicom ui entry fields spacing.
Created attachment 354417 [details] [review] Updated metadata viewer & editor patch. Updated metadata viewer & editor patch, this one also fixes issues with saving some of the xmp tags.
Great :) But I'm afraid I didn't make myself clear... I meant to move them *both* into *one* new plug-ins/metadata/ folder. Do you mind changing that? Otherwise I can simply do it when applying the patch...
I don't have the time just now to do that, probably best that you do the change.
Created attachment 354983 [details] [review] Updated metadata editor & viewer patch, adds Xmp.xmpMM.History support @Mitch I had a few hours this afternoon to spend on your request and added Xmp.xmpMM.History support while at it. Hopefully i haven't forgotten to add anything to this one *smh*
Something is always forgotten :) Regardless, I think the patch is good for git now, do you agree?
Yes i do agree, time to bake this cake lol.
I don't do subtle real well it seems, lol, was that a suggestion for me to commit to git?
If you move the stuff to a common plug-ins/metadata/, sure go ahead :) But I offered to do that, so you can well lean back and watch... I didn't try to be subtle, that never works in the internet ;)
I'll let you do it then i have to work on something else right now. The last patch has everything in the same directory like you just mentioned.
Applied and built the patch, but the .ui files are missing, can you attach them separately please?
Created attachment 355078 [details] [review] Adds missing UI files. I knew i was forgetting something. Here's the addon patch with the missing UI files.
Created attachment 355079 [details] plug-in-metadata-editor.ui file
Created attachment 355080 [details] plug-in-metadata-editor-calendar.ui file
Created attachment 355081 [details] plug-in-metadata-viewer.ui file
Pushed to master pretty much as is. There is some cleanup to be done but I rather do that separately... commit dc9856cfe12d72c6478504ffc09d2135f90d117b Author: Ben Touchette <draekko.software@gmail.com> Date: Wed Jul 5 19:24:54 2017 -0400 Bug 769820 - Cannot enter Iptc information when no metadata is available... ...and fails to write it to file when it is Completely redo metadata editor and viewer code, adds support for Xmp.xmpMM.History and more. configure.ac | 1 + libgimp/gimpimagemetadata.c | 97 +- libgimpbase/gimpmetadata.c | 339 ++- libgimpbase/gimpmetadata.h | 13 + plug-ins/Makefile.am | 1 + plug-ins/common/.gitignore | 2 - plug-ins/common/Makefile.am | 21 - plug-ins/common/gimprc.common | 1 - plug-ins/common/metadata.c | 512 ---- plug-ins/common/plugin-defs.pl | 1 - plug-ins/metadata/.gitignore | 9 + plug-ins/metadata/Makefile.am | 74 + plug-ins/metadata/metadata-editor.c | 5571 +++++++++++++++++++++++++++++++++++++++ plug-ins/metadata/metadata-editor.h | 27 + plug-ins/metadata/metadata-impexp.c | 238 ++ plug-ins/metadata/metadata-impexp.h | 30 + plug-ins/metadata/metadata-misc.h | 70 + plug-ins/metadata/metadata-tags.h | 494 ++++ plug-ins/metadata/metadata-viewer.c | 302 +++ plug-ins/metadata/metadata-xml.c | 1186 +++++++++ plug-ins/metadata/metadata-xml.h | 98 + plug-ins/ui/Makefile.am | 12 +- plug-ins/ui/plug-in-metadata-editor-calendar.ui | 25 + plug-ins/ui/plug-in-metadata-editor.ui | 4953 ++++++++++++++++++++++++++++++++++ plug-ins/ui/plug-in-metadata-viewer.ui | 226 ++ plug-ins/ui/plug-in-metadata.ui | 908 ------- po-plug-ins/POTFILES.in | 3 +- 27 files changed, 13739 insertions(+), 1475 deletions(-)
Sweet, all those months of work are finally pushed to the repo :) I figured there would be some cleanup issues, there always are some :)
And some cleanup, mostly coding style... Let's close as FIXED. Thanks a lot again, this seems very complete :) commit 486e0f3d88fafa28ec1342c947e5e11fb9aa7891 Author: Michael Natterer <mitch@gimp.org> Date: Fri Jul 7 23:54:25 2017 +0200 plug-ins: lots and lots of cleanup in metadata/ plug-ins/metadata/metadata-editor.c | 3606 ++++++++++++++++++++++++++++----------------------- plug-ins/metadata/metadata-tags.h | 8 - plug-ins/metadata/metadata-xml.c | 12 +- 3 files changed, 1973 insertions(+), 1653 deletions(-) commit a10b07e3986805391694656096d14c680c5bb210 Author: Michael Natterer <mitch@gimp.org> Date: Fri Jul 7 19:14:47 2017 +0200 plug-ins: fix some warnings in metadata/ plug-ins/metadata/metadata-editor.c | 2 +- plug-ins/metadata/metadata-viewer.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) commit 747b3768ef636c26209fe58de720ed384c94fec9 Author: Michael Natterer <mitch@gimp.org> Date: Fri Jul 7 19:09:59 2017 +0200 libgimpbase: remove gimp_metadata_register_xmp_namespace[s]() and do the same in gimp_metadata_class_init(). Also fix all compiler warnings and other stuff. libgimp/gimpimagemetadata.c | 2 - libgimpbase/gimpmetadata.c | 366 ++++++++++++++++++++++++++++-------------------------------- libgimpbase/gimpmetadata.h | 5 - 3 files changed, 173 insertions(+), 200 deletions(-) commit 18e2ae4d1303a84ee3177b820374a88e47bb6ba2 Author: Piotr Drąg <piotrdrag@gmail.com> Date: Fri Jul 7 18:40:05 2017 +0200 Update POTFILES.in po-plug-ins/POTFILES.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) commit 945a695f2d6cf216d4cd7659d94a86b2fba680ee Author: Michael Natterer <mitch@gimp.org> Date: Fri Jul 7 18:25:56 2017 +0200 libgimp: clean up metadata patch to not warn and other minor changes libgimp/gimpimagemetadata.c | 54 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 26 deletions(-)
Welcome, i tried to cover all use cases and make it compatible with photoshop's set of metadata.