[Digikam-devel] Review Request 109087: Patch that enable digikam to write face tags into xmp metadata [Experimental]

[Veaceslav Munteanu]

> On Feb. 23, 2013, 11:10 a.m., Marcel Wiesweg wrote:
> > Thanks a lot for your work. I am reviewing the metadata part for now, including your kexiv2 commit.
> > 
> > 1) setXmpTagString(const char* xmpTagName, const QString& value, int type, bool setProgramName)
> >  - Is there a difference between calling the new method with 0, to calling the normal setXmpTagString?
> >  - Is there a difference between calling the new method with 1, to calling setXmpTagStringBag?
> >  - Looking at http://www.exiv2.org/doc/xmpsample_8cpp-example.html and the way a structure is added there (videoFrameSize, CreatorContactInfo), is it necessary at all to add the parent key of the structure by setting the type setXmpStruct? It seems one can directly add the members of the structure.
> > 
> > 2) IIRC correctly, MWG stores the regions referring to the unrotated picture. Is this correct? In digikam, we store the regions referring to the readily rotated picture. 
> > - This problem can be solved when writing for non-RAW formats by reading the rotation metadata field, need to add the required 2D rotation
> > - this problem is currently unsolvable for RAW files, see my comment 1 on 309341.
> > - How do Picasa and Microsoft write the regions? In particular, does Picasa correctly implement the specification? Would need to test with an image in portrait orientation which is rotated upright only by Exif tag.
> 
> Veaceslav Munteanu wrote:
>     Thank you Marcel for reviewing my patch.
>     
>     1.1 There is not difference between calling new method with 0 and original setXmpTagString. I can remove this part, I let it just to preserve the original name (setXmpTagString). At you wish I can add an enumeration {normal, bag, struct} instead of 0,1,2 ...
>     1.2 Yes, there is a difference between calling new method with 1, compared to setXmpStringBag. setXmpStringBag set the given char* as bag and then adds all strings form QStringList. This one simply set a char* as bag without adding anything to it. Later I add more complex structures, that's why I need only to set up an empty bag.
>     1.3 It works for first structure, but if I try to create structure-in-a-structure, it doesn't work anymore. That's why I need to set it as structure by hand.
>     
>     2. This one is a little tricky, because I'm not that familiar about how Digikam store faces(now your comment makes everything more clear), after applying this patch https://bugs.kde.org/show_bug.cgi?id=314509 I saw that rotation force metadata re-read and garbage tags are added into database. My idea is read exif rotation value, and always adjust rectangle and read/store it only in one way. After this, rectangle should be rotation independent. It's a good idea?
>
> 
> Marcel Wiesweg wrote:
>     1) Ok, so essentially this method is about adding tags of specified nature without value. I would recommend to make this clear in the API, something like 
>     enum TagType { NormalyTag, ArrayLangAltTag, ArrayBagTag, ArraySeqTag, StructureTag }; addXmpTag(const char* key, TagType type, ...)
>     and provide API docs for which purposes this method is intended. If you need another method to add values or subkeys, whatever, dont hesitate to add them.
>     KDE git is open for development at the moment.
>     
>     2) Unfortunately, we have two years ago defined that the regions in digikam's database are stored relative to the rotated picture, and MWG have defined the opposite. I dont know what Microsoft defined for their face tags. So there's not much we can do about that than write conversion routines. There is a longer text in the MWG guidance about this problem, they have invested though for their solution.

Ok, I updated setXmpTagString to use XmpTagType. Of course this patch won't compile anymore.. Now I'm trying to figure out how to do the conversion and will come back soon with another patch.


- Veaceslav


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109087/#review27923
-----------------------------------------------------------


On Feb. 21, 2013, 9:21 p.m., Veaceslav Munteanu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109087/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2013, 9:21 p.m.)
> 
> 
> Review request for Digikam, Gilles Caulier and Marcel Wiesweg.
> 
> 
> Description
> -------
> 
> This patch allows digikam to write face tags using Picassa/MetadataWorkingGroup format into image's xmp. This patch is still experimental, do not test with you usual photo collection. 
> Even if It shouldn't do anything bad, I don't want to mess your collection.
> 
> This patch depends on libkexiv2 new methods that I added. It's based on this commit:
> 
> http://quickgit.kde.org/?p=libkexiv2.git&a=commit&h=f2c50255227852bcb83115f13bfc4d4394a780ed
> 
> 
> This addresses bug https://bugs.kde.org/show_bug.cgi?id=277429.
>     http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=277429
> 
> 
> Diffs
> -----
> 
>   digikam/fileaction/metadatahub.cpp ea74590 
>   libs/database/core/tagregion.h 436960f 
>   libs/database/core/tagregion.cpp 4fda6af 
>   libs/dmetadata/dmetadata.h fb9a617 
>   libs/dmetadata/dmetadata.cpp 8eccb45 
>   libs/dmetadata/metadatasettingscontainer.h 02d35d2 
>   libs/dmetadata/metadatasettingscontainer.cpp 3e6cd93 
>   utilities/setup/setupmetadata.cpp f73870c 
> 
> Diff: http://git.reviewboard.kde.org/r/109087/diff/
> 
> 
> Testing
> -------
> 
> I tested so far with my limited collection and setting and deleting tags works well. Tested with exiv2 -pa to see if tags are set or removed.
> 
> 
> Thanks,
> 
> Veaceslav Munteanu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/digikam-devel/attachments/20130225/0fd2d435/attachment.html>