Created attachment 168246 [details] [review]
The patch

Review of attachment 168246 [details] [review]:

nice patch, would you mind fixing the missing points? (shouldnt take you long)

::: configure.ac
@@ +175,3 @@
+  zeitgeist-1.0 >= $LIBZEITGEIST_REQUIRED,
+  has_libzg=yes,
+  AC_MSG_WARN([zeitgeist-1.0 not found. Zeitgeist support disabled.])

didnt try this, but it would be cool to have an option

--disable-zeitgeist and
--enable-zeitgeist

::: src/cheese-window.vala
@@ +590,3 @@
                                  null);
     this.camera.take_photo (file_name);
+    this.push_to_zeitgeist (file_name, true);

missing #if HAVE_LIBZG

@@ +646,3 @@
+        string filename = fileutil.get_new_media_filename (this.current_mode);
+        camera.start_video_recording (filename);
+        this.push_to_zeitgeist (filename, false);

missing #if HAVE_LIBZG

@@ +1073,3 @@
+#if HAVE_LIBZG
+  private Zeitgeist.Log zg_log = null;
+#endif

better: one big 	

#if HAVE_LIBZG

around the whole block

@@ +1081,3 @@
+    string interpretation = is_photo ?
+      Zeitgeist.NFO_RASTER_IMAGE : Zeitgeist.VIDEO;
+    string title = "%s taken using Cheese".printf (is_photo ? "Photo":"Video");

shouldnt this string be translated?

@@ +1090,3 @@
+      size_t n = (char*)substr - (char*)uri;
+      origin = uri.ndup (n);
+    }

please use 

fileutil.get_photo_path () 

and

fileutil.get_video_path ()

instead

Created attachment 168342 [details] [review]
Second stab at the patch

Fixes according to the review, though I left the defines in the vala file, as this way it doesn't pollute the original code with the awful #ifs (and calling empty functions doesn't hurt).

Created attachment 168469 [details] [review]
Updated to apply against trunk

(In reply to comment #4)
> Created an attachment (id=168469) [details] [review]
> Updated to apply against trunk

please drop the foo.patch file from the patch

Created attachment 171068 [details] [review]
Updated patch

your patch does not apply to current git master, could you update?

Siegfried Gevatter, ping, did you get time to update patch as per current git master code base ?

Created attachment 186026 [details] [review]
Add Zeitgeist support

Review of attachment 171068 [details] [review]:

This is outdated

Review of attachment 171068 [details] [review]:

This is outdated

Review of attachment 186026 [details] [review]:

I am in the progress of updating the build system, so it is probably best to hold off on any updates to the patch until tomorrow. The changes will require some alterations to the configure.ac and Makefile.am changes.

::: src/Makefile.am
@@ -56,1 +56,5 @@
 	$(CHEESE_GTK_LIBS)
+	
+
+if HAVE_LIBZG
+VALAFLAGS += --define=HAVE_LIBZG --pkg zeitgeist-1.0

HAVE_LIBZG should be output to the configuration header with AC_DEFINE.

::: src/cheese-window.vala
@@ +675,3 @@
                                    null);
       this.camera.take_photo (file_name);
+      this.push_to_zeitgeist (file_name, true);

Needs an #if HAVE_LIBZG.

@@ +763,3 @@
     if (is_start)
     {
       camera.start_video_recording (fileutil.get_new_media_filename (this.current_mode));

I think that you meant to remove this line, as you call start_video_recording() below.

@@ +764,3 @@
     {
       camera.start_video_recording (fileutil.get_new_media_filename (this.current_mode));
+      string filename = fileutil.get_new_media_filename (this.current_mode);

Also needs an #if HAVE_LIBZG.

@@ -1283,2 +1287,4 @@
     camera.play ();
   }
+
+#if HAVE_LIBZG

There should be one #if HAVE_LIBZGaround the whole block (function and declaration), as mentioned in a previous review.

@@ -1285,0 +1289,6 @@
+
+#if HAVE_LIBZG
+  private Zeitgeist.Log zg_log = null;
... 3 more ...

A boolean parameter here is pretty ugly, although Cheese is unlikely to gain the ability to capture other media.  It seems that Zeitgeist already provides constants for those: Zeitgeist.NFO_RASTER_IMAGE and Zeitgeist.NFO_VIDEO (or maybe something better, as those appear to be string constants). Of course, if those constants are used, there must be #if HAVE_LIBZG blocks around any calls to push_to_zeitgeist(), as the constants will not be available if Zeitgeist is not installed.

@@ +1314,3 @@
+                                         subject, null);
+
+    if (zg_log == null) zg_log = new Zeitgeist.Log ();

zg_log = new Zeitgeist.Log (); should be on the line after the if condition.

Comment on attachment 186026 [details] [review]
Add Zeitgeist support

Attaching new patch

Created attachment 200804 [details] [review]
Add Zeitgeist support for Pictures and Videos

Review of attachment 200804 [details] [review]:

This is not a git-formatted patch, please create one before sending an update. The configure.ac changes show that you are basing your patch on the stable branch, which does not have the build system changes that I mentioned in the previous review. Please ensure that you base future updates to the patch on the master branch, Finally, you ignored most of my requests in the previous review. Please either correct the patch based on my comments, or explain your reasoning for not doing so.

Since commit d0cd9db7c92b258aea34e7b4d42ef4f1c7a4958e, Cheese adds tags to saved media, including the application name and device, so I do not think that zeitgeist support is desirable any longer.