The Sound Menu

Merge lp:~cjcurran/indicator-sound/remote-art-handling into lp:indicator-sound/sound-menu-v2

  1. remote-art-handling
  2. Merge into trunk.0.1
Proposed by Conor Curran
Status: Merged
Merged at revision: 126
Proposed branch: lp:~cjcurran/indicator-sound/remote-art-handling
Merge into: lp:indicator-sound/sound-menu-v2
Diff against target: 463 lines (+261/-32)
7 files modified
src/Makefile.am (+6/-2)
src/fetch-file.vala (+86/-0)
src/metadata-menu-item.vala (+126/-4)
src/metadata-widget.c (+26/-12)
src/player-item.vala (+15/-11)
src/sound-service.c (+1/-2)
vapi/common-defs.vapi (+1/-1)
To merge this branch: bzr merge lp:~cjcurran/indicator-sound/remote-art-handling
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
Review via email: mp+34766@code.launchpad.net

This proposal supersedes a proposal from 2010-09-02.

Description of the change

- Deals with last fm plugin for rhythmbox which uses remote artwork. Dynamically remote artwork will now be fetched asynchronously, rounded and exposed.

To post a comment you must log in.
Revision history for this message
Ted Gould (ted) wrote : Posted in a previous version of this proposal

 * The temporary file needs to be built with mkstemp to create a random directory for the artwork.
 * It should be in the XDG temp directory instead of hardcoded to /tmp
 * You can use g_file_is_native() to determine whether it's local or not, just incase they're using something other than http://

review: Needs Fixing
Revision history for this message
Ted Gould (ted) wrote :

Probably should change "/XXXXXX" to be something that someone can find if they look at the filesystem. I'd suggest "/downloaded-coverart-XXXXXX". This way if someone looks at the directory they'll know what the files are. Otherwise, I'm happy :)

review: Approve
lp:~cjcurran/indicator-sound/remote-art-handling updated
132. By Conor Curran

changed temp name to be more telling of its purpose

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Makefile.am'
2--- src/Makefile.am 2010-08-27 11:25:47 +0000
3+++ src/Makefile.am 2010-09-07 15:31:44 +0000
4@@ -67,7 +67,9 @@
5 player-controller.vala \
6 mpris2-controller.vala \
7 player-item.vala \
8- familiar-players-db.vala
9+ familiar-players-db.vala \
10+ fetch-file.vala
11+
12
13 music_bridge_VALAFLAGS = \
14 --ccode \
15@@ -81,7 +83,9 @@
16 --pkg Dbusmenu-Glib-0.2 \
17 --pkg common-defs \
18 --pkg dbus-glib-1 \
19- --pkg gio-unix-2.0
20+ --pkg gio-unix-2.0 \
21+ --pkg gdk-pixbuf-2.0
22+
23
24 $(MAINTAINER_VALAFLAGS)
25
26
27=== added file 'src/fetch-file.vala'
28--- src/fetch-file.vala 1970-01-01 00:00:00 +0000
29+++ src/fetch-file.vala 2010-09-07 15:31:44 +0000
30@@ -0,0 +1,86 @@
31+/*
32+ * Copyright (C) 2010 Canonical, Ltd.
33+ *
34+ * This library is free software; you can redistribute it and/or modify
35+ * it under the terms of the GNU Lesser General Public License
36+ * version 3.0 as published by the Free Software Foundation.
37+ *
38+ * This library is distributed in the hope that it will be useful,
39+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
40+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
41+ * GNU Lesser General Public License version 3.0 for more details.
42+ *
43+ * You should have received a copy of the GNU Lesser General Public
44+ * License along with this library. If not, see
45+ * <http://www.gnu.org/licenses/>.
46+ *
47+ * Authors
48+ * Gordon Allott <gord.allott@canonical.com>
49+ * Conor Curran <conor.curran@canonical.com>
50+ */
51+
52+public class FetchFile : Object
53+{
54+ /* public variables */
55+ public string uri {get; construct;}
56+ public string intended_property {get; construct;}
57+
58+ /* private variables */
59+ private DataInputStream stream;
60+ private File? file;
61+ private ByteArray data;
62+
63+ /* public signals */
64+ public signal void failed ();
65+ public signal void completed (ByteArray data, string property);
66+
67+ public FetchFile (string uri, string prop)
68+ {
69+ Object (uri: uri, intended_property: prop);
70+ }
71+
72+ construct
73+ {
74+ this.file = File.new_for_uri(this.uri);
75+ this.data = new ByteArray ();
76+ }
77+
78+ public async void fetch_data ()
79+ {
80+ try {
81+ this.stream = new DataInputStream(this.file.read(null));
82+ this.stream.set_byte_order (DataStreamByteOrder.LITTLE_ENDIAN);
83+ } catch (GLib.Error e) {
84+ this.failed ();
85+ }
86+ this.read_something_async ();
87+ }
88+
89+ private async void read_something_async ()
90+ {
91+ ssize_t size = 1024;
92+ uint8[] buffer = new uint8[size];
93+
94+ ssize_t bufsize = 1;
95+ do {
96+ try {
97+ bufsize = yield this.stream.read_async (buffer, size, GLib.Priority.DEFAULT, null);
98+ if (bufsize < 1) { break;}
99+
100+ if (bufsize != size)
101+ {
102+ uint8[] cpybuf = new uint8[bufsize];
103+ Memory.copy (cpybuf, buffer, bufsize);
104+ this.data.append (cpybuf);
105+ }
106+ else
107+ {
108+ this.data.append (buffer);
109+ }
110+ } catch (Error e) {
111+ this.failed ();
112+ }
113+ } while (bufsize > 0);
114+ this.completed (this.data, this.intended_property);
115+ }
116+}
117
118=== modified file 'src/metadata-menu-item.vala'
119--- src/metadata-menu-item.vala 2010-08-24 16:59:15 +0000
120+++ src/metadata-menu-item.vala 2010-09-07 15:31:44 +0000
121@@ -17,18 +17,141 @@
122 with this program. If not, see <http://www.gnu.org/licenses/>.
123 */
124
125-using Dbusmenu;
126 using Gee;
127 using DbusmenuMetadata;
128+using Gdk;
129
130 public class MetadataMenuitem : PlayerItem
131 {
132+ public const string ALBUM_ART_DIR_SUFFIX = "indicators/sound/album-art-cache";
133+
134+ public static string album_art_cache_dir;
135+ private static FetchFile fetcher;
136+ private string previous_temp_album_art_path;
137+
138 public MetadataMenuitem()
139 {
140 Object(item_type: MENUITEM_TYPE);
141 reset(attributes_format());
142 }
143-
144+
145+ construct{
146+ MetadataMenuitem.clean_album_art_temp_dir();
147+ this.previous_temp_album_art_path = null;
148+ this.album_art_cache_dir = MetadataMenuitem.create_album_art_temp_dir();
149+ }
150+
151+ private static void clean_album_art_temp_dir()
152+ {
153+ string path = GLib.Path.build_filename(Environment.get_user_cache_dir(), ALBUM_ART_DIR_SUFFIX);
154+
155+ GLib.File? album_art_dir = GLib.File.new_for_path(path);
156+
157+ if(delete_album_art_contents(album_art_dir) == false)
158+ {
159+ warning("could not remove the temp album art files %s", path);
160+ }
161+ }
162+
163+ private static string? create_album_art_temp_dir()
164+ {
165+ string path = GLib.Path.build_filename(Environment.get_user_cache_dir(), ALBUM_ART_DIR_SUFFIX);
166+ if(DirUtils.create(path, 0700) == -1){
167+ warning("could not create a temp dir for remote album art, it must have been created already");
168+ }
169+ return path;
170+ }
171+
172+ private static bool delete_album_art_contents (GLib.File dir)
173+ {
174+ bool result = true;
175+ try {
176+ var e = dir.enumerate_children (FILE_ATTRIBUTE_STANDARD_NAME,
177+ FileQueryInfoFlags.NOFOLLOW_SYMLINKS,
178+ null);
179+ while (true)
180+ {
181+ var file = e.next_file (null);
182+
183+ debug("file name = %s", file.get_name());
184+
185+ if (file == null)
186+ break;
187+
188+ var child = dir.get_child (file.get_name ());
189+
190+ try {
191+ child.delete (null);
192+ } catch (Error error_) {
193+ warning (@"Unable to delete file '$(child.get_basename ()): $(error_.message)");
194+ result = false;
195+ }
196+ }
197+ } catch (Error error) {
198+ warning (@"Unable to read files from directory '$(dir.get_basename ())': %s",
199+ error.message);
200+ result = false;
201+ }
202+ return result;
203+ }
204+
205+ public void fetch_art(string uri, string prop)
206+ {
207+ File art_file = File.new_for_uri(uri);
208+ if(art_file.is_native() == true){
209+ string path;
210+ try{
211+ path = Filename.from_uri(uri.strip());
212+ this.property_set(prop, path);
213+ }
214+ catch(ConvertError e){
215+ warning("Problem converting URI %s to file path",
216+ uri);
217+ }
218+ // eitherway return, the artwork was local
219+ return;
220+ }
221+ debug("fetch_art -remotely %s", this.album_art_cache_dir);
222+ // If we didn't manage to create the temp dir
223+ // don't bother with remote
224+ if(this.album_art_cache_dir == null){
225+ return;
226+ }
227+ // green light to go remote
228+ this.fetcher = new FetchFile (uri, prop);
229+ this.fetcher.failed.connect (() => { this.on_fetcher_failed ();});
230+ this.fetcher.completed.connect (this.on_fetcher_completed);
231+ this.fetcher.fetch_data ();
232+ }
233+
234+ private void on_fetcher_failed ()
235+ {
236+ warning("on_fetcher_failed -> could not fetch artwork");
237+ }
238+
239+ private void on_fetcher_completed(ByteArray update, string property)
240+ {
241+ try{
242+ PixbufLoader loader = new PixbufLoader ();
243+ loader.write (update.data, update.len);
244+ loader.close ();
245+ Pixbuf icon = loader.get_pixbuf ();
246+ string path = this.album_art_cache_dir.concat("/downloaded-coverart-XXXXXX");
247+ int r = FileUtils.mkstemp(path);
248+ if(r != -1){
249+ icon.save (path, loader.get_format().get_name());
250+ this.property_set(property, path);
251+ if(this.previous_temp_album_art_path != null){
252+ FileUtils.remove(this.previous_temp_album_art_path);
253+ }
254+ this.previous_temp_album_art_path = path;
255+ }
256+ }
257+ catch(GLib.Error e){
258+ warning("Problem creating file from bytearray fetched from the interweb - error: %s",
259+ e.message);
260+ }
261+ }
262
263 public static HashSet<string> attributes_format()
264 {
265@@ -38,6 +161,5 @@
266 attrs.add(MENUITEM_ALBUM);
267 attrs.add(MENUITEM_ARTURL);
268 return attrs;
269- }
270-
271+ }
272 }
273
274=== modified file 'src/metadata-widget.c'
275--- src/metadata-widget.c 2010-09-06 17:07:15 +0000
276+++ src/metadata-widget.c 2010-09-07 15:31:44 +0000
277@@ -25,6 +25,7 @@
278 #include "metadata-widget.h"
279 #include "common-defs.h"
280 #include <gtk/gtk.h>
281+#include <glib.h>
282
283 static DbusmenuMenuitem* twin_item;
284
285@@ -69,7 +70,7 @@
286 MetadataWidget* metadata,
287 GdkPixbuf *source);
288
289-
290+static void draw_album_art_placeholder(GtkWidget *metadata);
291
292 G_DEFINE_TYPE (MetadataWidget, metadata_widget, GTK_TYPE_MENU_ITEM);
293
294@@ -178,7 +179,7 @@
295
296 /**
297 * We override the expose method to enable primitive drawing of the
298- * empty album art image (and soon rounded rectangles on the album art)
299+ * empty album art image and rounded rectangles on the album art.
300 */
301 static gboolean
302 metadata_image_expose (GtkWidget *metadata, GdkEventExpose *event, gpointer user_data)
303@@ -186,24 +187,33 @@
304 g_return_val_if_fail(IS_METADATA_WIDGET(user_data), FALSE);
305 MetadataWidget* widget = METADATA_WIDGET(user_data);
306 MetadataWidgetPrivate * priv = METADATA_WIDGET_GET_PRIVATE(widget);
307-
308 if(priv->image_path->len > 0){
309-
310- if(g_string_equal(priv->image_path, priv->old_image_path) == FALSE){
311+ if(g_string_equal(priv->image_path, priv->old_image_path) == FALSE){
312 GdkPixbuf* pixbuf;
313 pixbuf = gdk_pixbuf_new_from_file(priv->image_path->str, NULL);
314- g_debug("metadata_widget_expose, album art update -> pixbuf from %s",
315- priv->image_path->str);
316+ g_debug("metadata_load_new_image -> pixbuf from %s",
317+ priv->image_path->str);
318+ if(GDK_IS_PIXBUF(pixbuf) == FALSE){
319+ g_debug("problem loading the downloaded image just use the placeholder instead");
320+ draw_album_art_placeholder(metadata);
321+ return TRUE;
322+ }
323 pixbuf = gdk_pixbuf_scale_simple(pixbuf,60, 60, GDK_INTERP_BILINEAR);
324 image_set_from_pixbuf (metadata, widget, pixbuf);
325 g_string_erase(priv->old_image_path, 0, -1);
326 g_string_overwrite(priv->old_image_path, 0, priv->image_path->str);
327
328- g_object_unref(pixbuf);
329+ g_object_unref(pixbuf);
330 }
331 return FALSE;
332 }
333-
334+ draw_album_art_placeholder(metadata);
335+ return TRUE;
336+}
337+
338+static void draw_album_art_placeholder(GtkWidget *metadata)
339+{
340+
341 cairo_t *cr;
342 cr = gdk_cairo_create (metadata->window);
343 GtkAllocation alloc;
344@@ -255,8 +265,7 @@
345 g_object_unref(pcontext);
346 g_string_free (string, TRUE);
347 cairo_destroy (cr);
348-
349- return TRUE;
350+
351 }
352
353 /* Suppress/consume keyevents */
354@@ -314,7 +323,12 @@
355 }
356 else if(g_ascii_strcasecmp(DBUSMENU_METADATA_MENUITEM_ARTURL, property) == 0){
357 g_string_erase(priv->image_path, 0, -1);
358- g_string_overwrite(priv->image_path, 0, g_value_get_string (value));
359+ g_string_overwrite(priv->image_path, 0, g_value_get_string (value));
360+ // if its a remote image queue a redraw incase the download took too long
361+ if (g_str_has_prefix(g_value_get_string (value), g_get_user_cache_dir())){
362+ g_debug("the image update is a download so redraw");
363+ gtk_widget_queue_draw(GTK_WIDGET(mitem));
364+ }
365 }
366 }
367
368
369=== modified file 'src/player-item.vala'
370--- src/player-item.vala 2010-08-24 16:59:15 +0000
371+++ src/player-item.vala 2010-09-07 15:31:44 +0000
372@@ -25,7 +25,7 @@
373 public PlayerController owner {get; construct;}
374 public string item_type { get; construct; }
375 private const int EMPTY = -1;
376-
377+
378 public PlayerItem(string type)
379 {
380 Object(item_type: type);
381@@ -42,6 +42,12 @@
382 }
383 }
384
385+ /**
386+ * update()
387+ * Base update method for playeritems, takes the attributes and the incoming updates
388+ * and attmepts to update the appropriate props on the object.
389+ * Album art is handled separately to deal with remote and local file paths.
390+ */
391 public void update(HashTable<string, Value?> data, HashSet<string> attributes)
392 {
393 debug("PlayerItem::update()");
394@@ -59,16 +65,14 @@
395 if (v.holds (typeof (string))){
396 string update = v.get_string().strip();
397 debug("with value : %s", update);
398- // Special case for the arturl URI's.
399- if(property.contains("mpris:artUrl")){
400- try{
401- update = Filename.from_uri(update.strip());
402- }
403- catch(ConvertError e){
404- warning("Problem converting URI %s to file path", update);
405- }
406+ if(property.contains("mpris:artUrl")){
407+ // We know its a metadata instance because thats the only
408+ // object with the arturl prop
409+ MetadataMenuitem metadata = this as MetadataMenuitem;
410+ metadata.fetch_art(update.strip(), property);
411+ continue;
412 }
413- this.property_set(property, update);
414+ this.property_set(property, update);
415 }
416 else if (v.holds (typeof (int))){
417 debug("with value : %i", v.get_int());
418@@ -83,7 +87,6 @@
419 this.property_set_bool(property, v.get_boolean());
420 }
421 }
422-
423 if(this.property_get_bool(MENUITEM_PROP_VISIBLE) == false){
424 this.property_set_bool(MENUITEM_PROP_VISIBLE, true);
425 }
426@@ -101,5 +104,6 @@
427 }
428 return false;
429 }
430+
431 }
432
433
434=== modified file 'src/sound-service.c'
435--- src/sound-service.c 2010-09-07 09:52:23 +0000
436+++ src/sound-service.c 2010-09-07 15:31:44 +0000
437@@ -40,14 +40,13 @@
438 {
439 if (mainloop != NULL) {
440 g_debug("Service shutdown !");
441- // TODO: uncomment for release !!
442+ //TODO: uncomment for release !!
443 close_pulse_activites();
444 g_main_loop_quit(mainloop);
445 }
446 return;
447 }
448
449-
450 /**
451 main:
452 **/
453
454=== modified file 'vapi/common-defs.vapi'
455--- vapi/common-defs.vapi 2010-07-21 09:56:15 +0000
456+++ vapi/common-defs.vapi 2010-09-07 15:31:44 +0000
457@@ -1,6 +1,6 @@
458 /*
459 Copyright 2010 Canonical Ltd.
460-
461+
462 Authors:
463 Conor Curran <conor.curran@canonical.com>
464

Subscribers

People subscribed via source and target branches

to all changes: