Re: XMP import patch - Best version yet :)

["Cosme Sevestre" <csevestre gmail com>]

Hi

On 7/26/06, Bengt Thuree <bengt thuree com> wrote:
> [...]
> I have uploaded a new patch for the XMP importer, which I think is a
> good step forward.

Thanks,  I think it is taking a pretty good shape too.  I tested the
latest patch and imported successfully all my pictures (jpeg, a few
hundred of them having tags with non-ASCII characters).

I have a few comments/propositions, see below.

> [...]
> Please try it and let me know how it works, and especially if I
>  - am missing some tags

If you can just add the rating tag I discussed about in a previous
email, then F-Spot would retrieve both Urgency and Rating data and
would be ready for bug 326561 [1] :-).  Attached a small patch to
apply after yours - I tested it with a picture rated using Lightroom.

>  - have some fundamental coding problems

It is a more generic feedback and just my humble opinion (so feel free
to ignore it) but I think F-Spot would benefit in a more structured
file organization (now 121 .cs files in src/) and more comments in the
code.

Here is a few propositions specific to the XMP patch:
XmpTagsMetadata.cs
- I am not sure how the variables (creator to captionwriter/rating)
are sorted.  Maybe capturing using comments where those variables come
from (exif, all the xmp stuff with dublin core, xap, raw, photoshop,
etc.) would help.  For example I wasn't sure were to add the rating
stuff (part of the XMP Basic Schema) so just added it to the end...
since there are a lot of possible metadata tags, it might be worth
doing.
- Maybe capture the rational behind the GetSingleRowData and
ProcessItemsList methods.  I think someone new to the code would
wonder why some tags are fetched using the first and others using the
second as the methods name are not very clear.

>  - white space problems
>  - missing whatever

Thanks for your hard work :-)

Cheers,

Cosme

----
[1] http://bugzilla.gnome.org/show_bug.cgi?id=326561

diff -Nur src.orig/XmpTagsImporter.cs src/XmpTagsImporter.cs
--- src.orig/XmpTagsImporter.cs	2006-07-26 17:40:06.000000000 +0530
+++ src/XmpTagsImporter.cs	2006-07-26 17:44:17.000000000 +0530
@@ -187,6 +187,8 @@
 			}
 			if (xmd.Creator != null) {
 			}
+			if (xmd.Rating != null) {
+			}
 
 			if (xmd.City != null)
 				AddTagToPhoto (photo, xmd.City, li_subroot_city);
diff -Nur src.orig/XmpTagsMetadata.cs src/XmpTagsMetadata.cs
--- src.orig/XmpTagsMetadata.cs	2006-07-26 17:40:06.000000000 +0530
+++ src/XmpTagsMetadata.cs	2006-07-26 17:44:17.000000000 +0530
@@ -68,6 +68,7 @@
 		private string isospeed;		public string ISOSpeed		{get { return isospeed; }	}
 		private string lightsource;		public string LightSource	{get { return lightsource; }	}
 		private string captionwriter;		public string CaptionWriter	{get { return captionwriter; }	}
+		private string rating;			public string Rating		{get { return rating; }	}
 
 		public static XmpTagsMetadata ForPhoto (string path, string orig_path)
 		{
@@ -232,6 +233,9 @@
 				case "CaptionWriter" :
 					captionwriter = stmt_obj_str;
 					break;
+				case "Rating" :
+					rating = stmt_obj_str;
+					break;
 
 				default :
 					break;
@@ -480,6 +484,7 @@
 			Console.WriteLine ("Have isospeed = >>{0}<<", isospeed==null ? "null" : isospeed );	
 			Console.WriteLine ("Have bitspersample = >>{0}<<", bitspersample==null ? "null" : bitspersample );	
 			Console.WriteLine ("Have captionwriter = >>{0}<<", captionwriter==null ? "null" : captionwriter );	
+			Console.WriteLine ("Have rating = >>{0}<<", rating==null ? "null" : rating );	
 	
 #endif