Merge lp:~cjcurran/indicator-sound/remote-art-handling into lp:indicator-sound/sound-menu-v2
- remote-art-handling
- 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 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Ted Gould (community) | Approve | ||
Review via email:
|
This proposal supersedes a proposal from 2010-09-02.
Commit message
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
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ted Gould (ted) wrote : Posted in a previous version of this proposal | # |
review:
Needs Fixing
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
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-
review:
Approve
- 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 |
* 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://