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: Superseded
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) Needs Fixing
Review via email: mp+34396@code.launchpad.net

This proposal has been superseded by a proposal from 2010-09-07.

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 :

 * 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
lp:~cjcurran/indicator-sound/remote-art-handling updated
126. By Conor Curran

reworked album art handling using gio exclusively with mkstemp

127. By Conor Curran

working nicely

128. By Conor Curran

big refactor

129. By Conor Curran

shifted to use the more appropriate cache dir

130. By Conor Curran

tidy up on the print outs

131. By Conor Curran

remote art url fixed

132. By Conor Curran

changed temp name to be more telling of its purpose

Unmerged revisions

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:12:48 +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:12:48 +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:12:48 +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("/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:12:48 +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:12:48 +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:12:48 +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:12:48 +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: