ayatana-commits team mailing list archive

[Conor Curran <conor.curran@xxxxxxxxxxxxx>]

Conor Curran has proposed merging lp:~cjcurran/indicator-sound/remote-art-handling into lp:indicator-sound.

Requested reviews:
  Ted Gould (ted)
Related bugs:
  #627505 Crashes if Rhythmbox is playing from Last.FM
  https://bugs.launchpad.net/bugs/627505


- Deals with last fm plugin for rhythmbox which uses remote artwork. Dynamically remote artwork will now be fetched asynchronously, rounded and exposed. 
-- 
https://code.launchpad.net/~cjcurran/indicator-sound/remote-art-handling/+merge/34766
Your team ayatana-commits is subscribed to branch lp:indicator-sound.

=== modified file 'src/Makefile.am'
--- src/Makefile.am	2010-08-27 11:25:47 +0000
+++ src/Makefile.am	2010-09-07 15:14:40 +0000
@@ -67,7 +67,9 @@
 	player-controller.vala \
 	mpris2-controller.vala \
 	player-item.vala \
-	familiar-players-db.vala
+	familiar-players-db.vala \
+	fetch-file.vala
+
 
 music_bridge_VALAFLAGS = \
   --ccode \
@@ -81,7 +83,9 @@
   --pkg Dbusmenu-Glib-0.2 \
   --pkg common-defs \
 	--pkg dbus-glib-1 \
-	--pkg gio-unix-2.0
+	--pkg gio-unix-2.0 \
+	--pkg gdk-pixbuf-2.0 
+
 	 
  $(MAINTAINER_VALAFLAGS)
 

=== added file 'src/fetch-file.vala'
--- src/fetch-file.vala	1970-01-01 00:00:00 +0000
+++ src/fetch-file.vala	2010-09-07 15:14:40 +0000
@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2010 Canonical, Ltd.
+ *
+ * This library is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License
+ * version 3.0 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License version 3.0 for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Authors
+ * 			Gordon Allott <gord.allott@xxxxxxxxxxxxx>
+ *			Conor Curran <conor.curran@xxxxxxxxxxxxx>
+ */
+
+public class FetchFile : Object
+{
+  /* public variables */
+  public string uri {get; construct;}
+	public string intended_property {get; construct;}
+
+  /* private variables */
+  private DataInputStream stream;
+  private File? file;
+  private ByteArray data;
+
+  /* public signals */
+  public signal void failed ();
+  public signal void completed (ByteArray data, string property);
+
+  public FetchFile (string uri, string prop)
+  {
+    Object (uri: uri, intended_property: prop);
+  }
+
+  construct
+  {
+    this.file = File.new_for_uri(this.uri);
+    this.data = new ByteArray ();
+  }
+
+  public async void fetch_data ()
+  {
+    try {
+      this.stream = new DataInputStream(this.file.read(null));
+      this.stream.set_byte_order (DataStreamByteOrder.LITTLE_ENDIAN);
+    } catch (GLib.Error e) {
+      this.failed ();
+    }
+    this.read_something_async ();
+  }
+
+  private async void read_something_async ()
+  {
+    ssize_t size = 1024;
+    uint8[] buffer = new uint8[size];
+
+    ssize_t bufsize = 1;
+    do {
+      try {
+        bufsize = yield this.stream.read_async (buffer, size, GLib.Priority.DEFAULT, null);
+        if (bufsize < 1) { break;}
+
+        if (bufsize != size)
+          {
+            uint8[] cpybuf = new uint8[bufsize];
+            Memory.copy (cpybuf, buffer, bufsize);
+            this.data.append (cpybuf);
+          }
+        else
+          {
+            this.data.append (buffer);
+          }
+      } catch (Error e) {
+        this.failed ();
+      }
+    } while (bufsize > 0);
+    this.completed (this.data, this.intended_property);
+  }
+}

=== modified file 'src/metadata-menu-item.vala'
--- src/metadata-menu-item.vala	2010-08-24 16:59:15 +0000
+++ src/metadata-menu-item.vala	2010-09-07 15:14:40 +0000
@@ -17,18 +17,141 @@
 with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
-using Dbusmenu;
 using Gee;
 using DbusmenuMetadata;
+using Gdk;
 
 public class MetadataMenuitem : PlayerItem
 {
+	public const string ALBUM_ART_DIR_SUFFIX = "indicators/sound/album-art-cache"; 
+	
+	public static string album_art_cache_dir;
+	private static FetchFile fetcher;
+	private string previous_temp_album_art_path;
+	
 	public MetadataMenuitem()
   {
 		Object(item_type: MENUITEM_TYPE); 	
 		reset(attributes_format());
 	}
-
+	
+	construct{
+		MetadataMenuitem.clean_album_art_temp_dir();
+		this.previous_temp_album_art_path = null;		
+		this.album_art_cache_dir = MetadataMenuitem.create_album_art_temp_dir();
+	}
+
+	private static void clean_album_art_temp_dir()
+	{
+		string path = GLib.Path.build_filename(Environment.get_user_cache_dir(), ALBUM_ART_DIR_SUFFIX);
+
+		GLib.File? album_art_dir = GLib.File.new_for_path(path);
+		
+		if(delete_album_art_contents(album_art_dir) == false)
+		{
+			warning("could not remove the temp album art files %s", path);
+		}
+	}
+
+	private static string? create_album_art_temp_dir()
+	{
+		string path = GLib.Path.build_filename(Environment.get_user_cache_dir(), ALBUM_ART_DIR_SUFFIX);
+		if(DirUtils.create(path, 0700) == -1){
+			warning("could not create a temp dir for remote album art, it must have been created already");
+		}
+		return path;
+	}
+	
+	private static bool delete_album_art_contents (GLib.File dir)
+  {
+		bool result = true;
+    try {
+      var e = dir.enumerate_children (FILE_ATTRIBUTE_STANDARD_NAME,
+                                      FileQueryInfoFlags.NOFOLLOW_SYMLINKS,
+                                      null);
+      while (true)
+        {
+          var file = e.next_file (null);
+
+					debug("file name = %s", file.get_name());
+					
+          if (file == null)					
+            break;
+
+          var child = dir.get_child (file.get_name ());
+
+            try {
+              child.delete (null);
+            } catch (Error error_) {
+              warning (@"Unable to delete file '$(child.get_basename ()): $(error_.message)");
+							result = false;
+            }
+        }
+    } catch (Error error) {
+      warning (@"Unable to read files from directory '$(dir.get_basename ())': %s",
+               error.message);
+			result = false;
+    }
+		return result;
+  }
+
+	public void fetch_art(string uri, string prop)
+	{		
+		File art_file = File.new_for_uri(uri);
+		if(art_file.is_native() == true){
+			string path;		
+			try{
+				path = Filename.from_uri(uri.strip());			
+				this.property_set(prop, path);			
+			}
+			catch(ConvertError e){
+				warning("Problem converting URI %s to file path",
+					      uri); 
+			}
+			// eitherway return, the artwork was local
+			return;			
+		}
+		debug("fetch_art -remotely %s", this.album_art_cache_dir);
+		// If we didn't manage to create the temp dir
+		// don't bother with remote		
+		if(this.album_art_cache_dir == null){
+			return;
+		}
+		// green light to go remote		
+		this.fetcher = new FetchFile (uri, prop);
+		this.fetcher.failed.connect (() => { this.on_fetcher_failed ();});
+		this.fetcher.completed.connect (this.on_fetcher_completed);
+		this.fetcher.fetch_data ();		
+	}
+	
+	private void on_fetcher_failed ()
+	{
+		warning("on_fetcher_failed -> could not fetch artwork");	
+	}
+
+	private void on_fetcher_completed(ByteArray update, string property)
+	{
+		try{
+			PixbufLoader loader = new PixbufLoader ();
+			loader.write (update.data, update.len);
+			loader.close ();
+			Pixbuf icon = loader.get_pixbuf ();				
+ 			string path = this.album_art_cache_dir.concat("/XXXXXX");
+			int r = FileUtils.mkstemp(path);
+			if(r != -1){
+				icon.save (path, loader.get_format().get_name());		
+				this.property_set(property, path);
+				if(this.previous_temp_album_art_path != null){
+					FileUtils.remove(this.previous_temp_album_art_path);
+				}			
+				this.previous_temp_album_art_path = path;				
+			}				
+		}
+	  catch(GLib.Error e){
+			warning("Problem creating file from bytearray fetched from the interweb - error: %s",
+			        e.message);
+		}				
+	}			
 	
 	public static HashSet<string> attributes_format()
 	{
@@ -38,6 +161,5 @@
     attrs.add(MENUITEM_ALBUM);
     attrs.add(MENUITEM_ARTURL);
 		return attrs;
-	}
-		
+	}	
 }

=== modified file 'src/metadata-widget.c'
--- src/metadata-widget.c	2010-09-06 17:07:15 +0000
+++ src/metadata-widget.c	2010-09-07 15:14:40 +0000
@@ -25,6 +25,7 @@
 #include "metadata-widget.h"
 #include "common-defs.h"
 #include <gtk/gtk.h>
+#include <glib.h>
 
 static DbusmenuMenuitem* twin_item;
 
@@ -69,7 +70,7 @@
 												           MetadataWidget* metadata,
 												           GdkPixbuf *source);
 
-
+static void draw_album_art_placeholder(GtkWidget *metadata);
 
 G_DEFINE_TYPE (MetadataWidget, metadata_widget, GTK_TYPE_MENU_ITEM);
 
@@ -178,7 +179,7 @@
 
 /**
  * We override the expose method to enable primitive drawing of the 
- * empty album art image (and soon rounded rectangles on the album art)
+ * empty album art image and rounded rectangles on the album art.
  */
 static gboolean
 metadata_image_expose (GtkWidget *metadata, GdkEventExpose *event, gpointer user_data)
@@ -186,24 +187,33 @@
 	g_return_val_if_fail(IS_METADATA_WIDGET(user_data), FALSE);
 	MetadataWidget* widget = METADATA_WIDGET(user_data);
 	MetadataWidgetPrivate * priv = METADATA_WIDGET_GET_PRIVATE(widget);	
-	
 	if(priv->image_path->len > 0){
-		
-	  if(g_string_equal(priv->image_path, priv->old_image_path) == FALSE){
+	  if(g_string_equal(priv->image_path, priv->old_image_path) == FALSE){					
 			GdkPixbuf* pixbuf;
 			pixbuf = gdk_pixbuf_new_from_file(priv->image_path->str, NULL);
-			g_debug("metadata_widget_expose, album art update -> pixbuf from %s",
-						  priv->image_path->str); 
+			g_debug("metadata_load_new_image -> pixbuf from %s",
+							priv->image_path->str); 
+			if(GDK_IS_PIXBUF(pixbuf) == FALSE){
+				g_debug("problem loading the downloaded image just use the placeholder instead");
+				draw_album_art_placeholder(metadata);
+				return TRUE;				
+			}
 			pixbuf = gdk_pixbuf_scale_simple(pixbuf,60, 60, GDK_INTERP_BILINEAR);
 			image_set_from_pixbuf (metadata, widget, pixbuf);
 			g_string_erase(priv->old_image_path, 0, -1);
 			g_string_overwrite(priv->old_image_path, 0, priv->image_path->str); 
 
-			g_object_unref(pixbuf);			
+			g_object_unref(pixbuf);				
 		}
 		return FALSE;				
 	}
-	
+	draw_album_art_placeholder(metadata);
+	return TRUE;
+}
+
+static void draw_album_art_placeholder(GtkWidget *metadata)
+{
+		
 	cairo_t *cr;	
 	cr = gdk_cairo_create (metadata->window);
 	GtkAllocation alloc;
@@ -255,8 +265,7 @@
 	g_object_unref(pcontext);
 	g_string_free (string, TRUE);
 	cairo_destroy (cr);	
-	
-	return TRUE;
+
 }
 
 /* Suppress/consume keyevents */
@@ -314,7 +323,12 @@
 	}	
 	else if(g_ascii_strcasecmp(DBUSMENU_METADATA_MENUITEM_ARTURL, property) == 0){
 		g_string_erase(priv->image_path, 0, -1);
-		g_string_overwrite(priv->image_path, 0, g_value_get_string (value)); 
+		g_string_overwrite(priv->image_path, 0, g_value_get_string (value));
+		// if its a remote image queue a redraw incase the download took too long
+		if (g_str_has_prefix(g_value_get_string (value), g_get_user_cache_dir())){
+			g_debug("the image update is a download so redraw");
+			gtk_widget_queue_draw(GTK_WIDGET(mitem));
+		}
 	}		
 }
 

=== modified file 'src/player-item.vala'
--- src/player-item.vala	2010-08-24 16:59:15 +0000
+++ src/player-item.vala	2010-09-07 15:14:40 +0000
@@ -25,7 +25,7 @@
 	public PlayerController owner {get; construct;}
 	public string item_type { get; construct; }
 	private const int EMPTY = -1;
-	
+
 	public PlayerItem(string type)
 	{		
 		Object(item_type: type);
@@ -42,6 +42,12 @@
 		}
 	}
 	
+	/**
+	 * update()
+	 * Base update method for playeritems, takes the attributes and the incoming updates
+	 * and attmepts to update the appropriate props on the object. 
+	 * Album art is handled separately to deal with remote and local file paths.
+	 */
 	public void update(HashTable<string, Value?> data, HashSet<string> attributes)
 	{
 		debug("PlayerItem::update()");
@@ -59,16 +65,14 @@
 			if (v.holds (typeof (string))){
 				string update = v.get_string().strip();
 				debug("with value : %s", update);
-				// Special case for the arturl URI's.
-				if(property.contains("mpris:artUrl")){
-					try{
-						update = Filename.from_uri(update.strip());
-					}
-					catch(ConvertError e){
-						warning("Problem converting URI %s to file path", update); 
-					}
+				if(property.contains("mpris:artUrl")){		
+					  // We know its a metadata instance because thats the only
+					  // object with the arturl prop					  
+						MetadataMenuitem metadata = this as MetadataMenuitem;
+					  metadata.fetch_art(update.strip(), property);
+					 	continue;					                     
 				}
-				this.property_set(property, update);
+				this.property_set(property, update);											
 			}			    
 			else if (v.holds (typeof (int))){
 				debug("with value : %i", v.get_int());
@@ -83,7 +87,6 @@
 				this.property_set_bool(property, v.get_boolean());
 			}
 		}
-
 		if(this.property_get_bool(MENUITEM_PROP_VISIBLE) == false){
 			this.property_set_bool(MENUITEM_PROP_VISIBLE, true);
 		}
@@ -101,5 +104,6 @@
 		}
 		return false;
 	}
+
 }
 

=== modified file 'src/sound-service.c'
--- src/sound-service.c	2010-09-07 09:52:23 +0000
+++ src/sound-service.c	2010-09-07 15:14:40 +0000
@@ -40,14 +40,13 @@
 {
   if (mainloop != NULL) {
     g_debug("Service shutdown !");
-    // TODO: uncomment for release !!
+    //TODO: uncomment for release !!
     close_pulse_activites();
     g_main_loop_quit(mainloop);
   }
   return;
 }
 
-
 /**
 main:
 **/

=== modified file 'vapi/common-defs.vapi'
--- vapi/common-defs.vapi	2010-07-21 09:56:15 +0000
+++ vapi/common-defs.vapi	2010-09-07 15:14:40 +0000
@@ -1,6 +1,6 @@
 /*
 Copyright 2010 Canonical Ltd.
-
+ 
 Authors:
     Conor Curran <conor.curran@xxxxxxxxxxxxx>