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
=== modified file 'src/Makefile.am'
--- src/Makefile.am 2010-08-27 11:25:47 +0000
+++ src/Makefile.am 2010-09-07 15:31:44 +0000
@@ -67,7 +67,9 @@
67 player-controller.vala \67 player-controller.vala \
68 mpris2-controller.vala \68 mpris2-controller.vala \
69 player-item.vala \69 player-item.vala \
70 familiar-players-db.vala70 familiar-players-db.vala \
71 fetch-file.vala
72
7173
72music_bridge_VALAFLAGS = \74music_bridge_VALAFLAGS = \
73 --ccode \75 --ccode \
@@ -81,7 +83,9 @@
81 --pkg Dbusmenu-Glib-0.2 \83 --pkg Dbusmenu-Glib-0.2 \
82 --pkg common-defs \84 --pkg common-defs \
83 --pkg dbus-glib-1 \85 --pkg dbus-glib-1 \
84 --pkg gio-unix-2.086 --pkg gio-unix-2.0 \
87 --pkg gdk-pixbuf-2.0
88
85 89
86 $(MAINTAINER_VALAFLAGS)90 $(MAINTAINER_VALAFLAGS)
8791
8892
=== 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:31:44 +0000
@@ -0,0 +1,86 @@
1/*
2 * Copyright (C) 2010 Canonical, Ltd.
3 *
4 * This library is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU Lesser General Public License
6 * version 3.0 as published by the Free Software Foundation.
7 *
8 * This library is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU Lesser General Public License version 3.0 for more details.
12 *
13 * You should have received a copy of the GNU Lesser General Public
14 * License along with this library. If not, see
15 * <http://www.gnu.org/licenses/>.
16 *
17 * Authors
18 * Gordon Allott <gord.allott@canonical.com>
19 * Conor Curran <conor.curran@canonical.com>
20 */
21
22public class FetchFile : Object
23{
24 /* public variables */
25 public string uri {get; construct;}
26 public string intended_property {get; construct;}
27
28 /* private variables */
29 private DataInputStream stream;
30 private File? file;
31 private ByteArray data;
32
33 /* public signals */
34 public signal void failed ();
35 public signal void completed (ByteArray data, string property);
36
37 public FetchFile (string uri, string prop)
38 {
39 Object (uri: uri, intended_property: prop);
40 }
41
42 construct
43 {
44 this.file = File.new_for_uri(this.uri);
45 this.data = new ByteArray ();
46 }
47
48 public async void fetch_data ()
49 {
50 try {
51 this.stream = new DataInputStream(this.file.read(null));
52 this.stream.set_byte_order (DataStreamByteOrder.LITTLE_ENDIAN);
53 } catch (GLib.Error e) {
54 this.failed ();
55 }
56 this.read_something_async ();
57 }
58
59 private async void read_something_async ()
60 {
61 ssize_t size = 1024;
62 uint8[] buffer = new uint8[size];
63
64 ssize_t bufsize = 1;
65 do {
66 try {
67 bufsize = yield this.stream.read_async (buffer, size, GLib.Priority.DEFAULT, null);
68 if (bufsize < 1) { break;}
69
70 if (bufsize != size)
71 {
72 uint8[] cpybuf = new uint8[bufsize];
73 Memory.copy (cpybuf, buffer, bufsize);
74 this.data.append (cpybuf);
75 }
76 else
77 {
78 this.data.append (buffer);
79 }
80 } catch (Error e) {
81 this.failed ();
82 }
83 } while (bufsize > 0);
84 this.completed (this.data, this.intended_property);
85 }
86}
087
=== 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:31:44 +0000
@@ -17,18 +17,141 @@
17with this program. If not, see <http://www.gnu.org/licenses/>.17with this program. If not, see <http://www.gnu.org/licenses/>.
18*/18*/
1919
20using Dbusmenu;
21using Gee;20using Gee;
22using DbusmenuMetadata;21using DbusmenuMetadata;
22using Gdk;
2323
24public class MetadataMenuitem : PlayerItem24public class MetadataMenuitem : PlayerItem
25{25{
26 public const string ALBUM_ART_DIR_SUFFIX = "indicators/sound/album-art-cache";
27
28 public static string album_art_cache_dir;
29 private static FetchFile fetcher;
30 private string previous_temp_album_art_path;
31
26 public MetadataMenuitem()32 public MetadataMenuitem()
27 {33 {
28 Object(item_type: MENUITEM_TYPE); 34 Object(item_type: MENUITEM_TYPE);
29 reset(attributes_format());35 reset(attributes_format());
30 }36 }
3137
38 construct{
39 MetadataMenuitem.clean_album_art_temp_dir();
40 this.previous_temp_album_art_path = null;
41 this.album_art_cache_dir = MetadataMenuitem.create_album_art_temp_dir();
42 }
43
44 private static void clean_album_art_temp_dir()
45 {
46 string path = GLib.Path.build_filename(Environment.get_user_cache_dir(), ALBUM_ART_DIR_SUFFIX);
47
48 GLib.File? album_art_dir = GLib.File.new_for_path(path);
49
50 if(delete_album_art_contents(album_art_dir) == false)
51 {
52 warning("could not remove the temp album art files %s", path);
53 }
54 }
55
56 private static string? create_album_art_temp_dir()
57 {
58 string path = GLib.Path.build_filename(Environment.get_user_cache_dir(), ALBUM_ART_DIR_SUFFIX);
59 if(DirUtils.create(path, 0700) == -1){
60 warning("could not create a temp dir for remote album art, it must have been created already");
61 }
62 return path;
63 }
64
65 private static bool delete_album_art_contents (GLib.File dir)
66 {
67 bool result = true;
68 try {
69 var e = dir.enumerate_children (FILE_ATTRIBUTE_STANDARD_NAME,
70 FileQueryInfoFlags.NOFOLLOW_SYMLINKS,
71 null);
72 while (true)
73 {
74 var file = e.next_file (null);
75
76 debug("file name = %s", file.get_name());
77
78 if (file == null)
79 break;
80
81 var child = dir.get_child (file.get_name ());
82
83 try {
84 child.delete (null);
85 } catch (Error error_) {
86 warning (@"Unable to delete file '$(child.get_basename ()): $(error_.message)");
87 result = false;
88 }
89 }
90 } catch (Error error) {
91 warning (@"Unable to read files from directory '$(dir.get_basename ())': %s",
92 error.message);
93 result = false;
94 }
95 return result;
96 }
97
98 public void fetch_art(string uri, string prop)
99 {
100 File art_file = File.new_for_uri(uri);
101 if(art_file.is_native() == true){
102 string path;
103 try{
104 path = Filename.from_uri(uri.strip());
105 this.property_set(prop, path);
106 }
107 catch(ConvertError e){
108 warning("Problem converting URI %s to file path",
109 uri);
110 }
111 // eitherway return, the artwork was local
112 return;
113 }
114 debug("fetch_art -remotely %s", this.album_art_cache_dir);
115 // If we didn't manage to create the temp dir
116 // don't bother with remote
117 if(this.album_art_cache_dir == null){
118 return;
119 }
120 // green light to go remote
121 this.fetcher = new FetchFile (uri, prop);
122 this.fetcher.failed.connect (() => { this.on_fetcher_failed ();});
123 this.fetcher.completed.connect (this.on_fetcher_completed);
124 this.fetcher.fetch_data ();
125 }
126
127 private void on_fetcher_failed ()
128 {
129 warning("on_fetcher_failed -> could not fetch artwork");
130 }
131
132 private void on_fetcher_completed(ByteArray update, string property)
133 {
134 try{
135 PixbufLoader loader = new PixbufLoader ();
136 loader.write (update.data, update.len);
137 loader.close ();
138 Pixbuf icon = loader.get_pixbuf ();
139 string path = this.album_art_cache_dir.concat("/downloaded-coverart-XXXXXX");
140 int r = FileUtils.mkstemp(path);
141 if(r != -1){
142 icon.save (path, loader.get_format().get_name());
143 this.property_set(property, path);
144 if(this.previous_temp_album_art_path != null){
145 FileUtils.remove(this.previous_temp_album_art_path);
146 }
147 this.previous_temp_album_art_path = path;
148 }
149 }
150 catch(GLib.Error e){
151 warning("Problem creating file from bytearray fetched from the interweb - error: %s",
152 e.message);
153 }
154 }
32 155
33 public static HashSet<string> attributes_format()156 public static HashSet<string> attributes_format()
34 {157 {
@@ -38,6 +161,5 @@
38 attrs.add(MENUITEM_ALBUM);161 attrs.add(MENUITEM_ALBUM);
39 attrs.add(MENUITEM_ARTURL);162 attrs.add(MENUITEM_ARTURL);
40 return attrs;163 return attrs;
41 }164 }
42
43}165}
44166
=== 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:31:44 +0000
@@ -25,6 +25,7 @@
25#include "metadata-widget.h"25#include "metadata-widget.h"
26#include "common-defs.h"26#include "common-defs.h"
27#include <gtk/gtk.h>27#include <gtk/gtk.h>
28#include <glib.h>
2829
29static DbusmenuMenuitem* twin_item;30static DbusmenuMenuitem* twin_item;
3031
@@ -69,7 +70,7 @@
69 MetadataWidget* metadata,70 MetadataWidget* metadata,
70 GdkPixbuf *source);71 GdkPixbuf *source);
7172
7273static void draw_album_art_placeholder(GtkWidget *metadata);
7374
74G_DEFINE_TYPE (MetadataWidget, metadata_widget, GTK_TYPE_MENU_ITEM);75G_DEFINE_TYPE (MetadataWidget, metadata_widget, GTK_TYPE_MENU_ITEM);
7576
@@ -178,7 +179,7 @@
178179
179/**180/**
180 * We override the expose method to enable primitive drawing of the 181 * We override the expose method to enable primitive drawing of the
181 * empty album art image (and soon rounded rectangles on the album art)182 * empty album art image and rounded rectangles on the album art.
182 */183 */
183static gboolean184static gboolean
184metadata_image_expose (GtkWidget *metadata, GdkEventExpose *event, gpointer user_data)185metadata_image_expose (GtkWidget *metadata, GdkEventExpose *event, gpointer user_data)
@@ -186,24 +187,33 @@
186 g_return_val_if_fail(IS_METADATA_WIDGET(user_data), FALSE);187 g_return_val_if_fail(IS_METADATA_WIDGET(user_data), FALSE);
187 MetadataWidget* widget = METADATA_WIDGET(user_data);188 MetadataWidget* widget = METADATA_WIDGET(user_data);
188 MetadataWidgetPrivate * priv = METADATA_WIDGET_GET_PRIVATE(widget); 189 MetadataWidgetPrivate * priv = METADATA_WIDGET_GET_PRIVATE(widget);
189
190 if(priv->image_path->len > 0){190 if(priv->image_path->len > 0){
191 191 if(g_string_equal(priv->image_path, priv->old_image_path) == FALSE){
192 if(g_string_equal(priv->image_path, priv->old_image_path) == FALSE){
193 GdkPixbuf* pixbuf;192 GdkPixbuf* pixbuf;
194 pixbuf = gdk_pixbuf_new_from_file(priv->image_path->str, NULL);193 pixbuf = gdk_pixbuf_new_from_file(priv->image_path->str, NULL);
195 g_debug("metadata_widget_expose, album art update -> pixbuf from %s",194 g_debug("metadata_load_new_image -> pixbuf from %s",
196 priv->image_path->str); 195 priv->image_path->str);
196 if(GDK_IS_PIXBUF(pixbuf) == FALSE){
197 g_debug("problem loading the downloaded image just use the placeholder instead");
198 draw_album_art_placeholder(metadata);
199 return TRUE;
200 }
197 pixbuf = gdk_pixbuf_scale_simple(pixbuf,60, 60, GDK_INTERP_BILINEAR);201 pixbuf = gdk_pixbuf_scale_simple(pixbuf,60, 60, GDK_INTERP_BILINEAR);
198 image_set_from_pixbuf (metadata, widget, pixbuf);202 image_set_from_pixbuf (metadata, widget, pixbuf);
199 g_string_erase(priv->old_image_path, 0, -1);203 g_string_erase(priv->old_image_path, 0, -1);
200 g_string_overwrite(priv->old_image_path, 0, priv->image_path->str); 204 g_string_overwrite(priv->old_image_path, 0, priv->image_path->str);
201205
202 g_object_unref(pixbuf); 206 g_object_unref(pixbuf);
203 }207 }
204 return FALSE; 208 return FALSE;
205 }209 }
206 210 draw_album_art_placeholder(metadata);
211 return TRUE;
212}
213
214static void draw_album_art_placeholder(GtkWidget *metadata)
215{
216
207 cairo_t *cr; 217 cairo_t *cr;
208 cr = gdk_cairo_create (metadata->window);218 cr = gdk_cairo_create (metadata->window);
209 GtkAllocation alloc;219 GtkAllocation alloc;
@@ -255,8 +265,7 @@
255 g_object_unref(pcontext);265 g_object_unref(pcontext);
256 g_string_free (string, TRUE);266 g_string_free (string, TRUE);
257 cairo_destroy (cr); 267 cairo_destroy (cr);
258 268
259 return TRUE;
260}269}
261270
262/* Suppress/consume keyevents */271/* Suppress/consume keyevents */
@@ -314,7 +323,12 @@
314 } 323 }
315 else if(g_ascii_strcasecmp(DBUSMENU_METADATA_MENUITEM_ARTURL, property) == 0){324 else if(g_ascii_strcasecmp(DBUSMENU_METADATA_MENUITEM_ARTURL, property) == 0){
316 g_string_erase(priv->image_path, 0, -1);325 g_string_erase(priv->image_path, 0, -1);
317 g_string_overwrite(priv->image_path, 0, g_value_get_string (value)); 326 g_string_overwrite(priv->image_path, 0, g_value_get_string (value));
327 // if its a remote image queue a redraw incase the download took too long
328 if (g_str_has_prefix(g_value_get_string (value), g_get_user_cache_dir())){
329 g_debug("the image update is a download so redraw");
330 gtk_widget_queue_draw(GTK_WIDGET(mitem));
331 }
318 } 332 }
319}333}
320334
321335
=== 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:31:44 +0000
@@ -25,7 +25,7 @@
25 public PlayerController owner {get; construct;}25 public PlayerController owner {get; construct;}
26 public string item_type { get; construct; }26 public string item_type { get; construct; }
27 private const int EMPTY = -1;27 private const int EMPTY = -1;
28 28
29 public PlayerItem(string type)29 public PlayerItem(string type)
30 { 30 {
31 Object(item_type: type);31 Object(item_type: type);
@@ -42,6 +42,12 @@
42 }42 }
43 }43 }
44 44
45 /**
46 * update()
47 * Base update method for playeritems, takes the attributes and the incoming updates
48 * and attmepts to update the appropriate props on the object.
49 * Album art is handled separately to deal with remote and local file paths.
50 */
45 public void update(HashTable<string, Value?> data, HashSet<string> attributes)51 public void update(HashTable<string, Value?> data, HashSet<string> attributes)
46 {52 {
47 debug("PlayerItem::update()");53 debug("PlayerItem::update()");
@@ -59,16 +65,14 @@
59 if (v.holds (typeof (string))){65 if (v.holds (typeof (string))){
60 string update = v.get_string().strip();66 string update = v.get_string().strip();
61 debug("with value : %s", update);67 debug("with value : %s", update);
62 // Special case for the arturl URI's.68 if(property.contains("mpris:artUrl")){
63 if(property.contains("mpris:artUrl")){69 // We know its a metadata instance because thats the only
64 try{70 // object with the arturl prop
65 update = Filename.from_uri(update.strip());71 MetadataMenuitem metadata = this as MetadataMenuitem;
66 }72 metadata.fetch_art(update.strip(), property);
67 catch(ConvertError e){73 continue;
68 warning("Problem converting URI %s to file path", update);
69 }
70 }74 }
71 this.property_set(property, update);75 this.property_set(property, update);
72 } 76 }
73 else if (v.holds (typeof (int))){77 else if (v.holds (typeof (int))){
74 debug("with value : %i", v.get_int());78 debug("with value : %i", v.get_int());
@@ -83,7 +87,6 @@
83 this.property_set_bool(property, v.get_boolean());87 this.property_set_bool(property, v.get_boolean());
84 }88 }
85 }89 }
86
87 if(this.property_get_bool(MENUITEM_PROP_VISIBLE) == false){90 if(this.property_get_bool(MENUITEM_PROP_VISIBLE) == false){
88 this.property_set_bool(MENUITEM_PROP_VISIBLE, true);91 this.property_set_bool(MENUITEM_PROP_VISIBLE, true);
89 }92 }
@@ -101,5 +104,6 @@
101 }104 }
102 return false;105 return false;
103 }106 }
107
104}108}
105109
106110
=== 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:31:44 +0000
@@ -40,14 +40,13 @@
40{40{
41 if (mainloop != NULL) {41 if (mainloop != NULL) {
42 g_debug("Service shutdown !");42 g_debug("Service shutdown !");
43 // TODO: uncomment for release !!43 //TODO: uncomment for release !!
44 close_pulse_activites();44 close_pulse_activites();
45 g_main_loop_quit(mainloop);45 g_main_loop_quit(mainloop);
46 }46 }
47 return;47 return;
48}48}
4949
50
51/**50/**
52main:51main:
53**/52**/
5453
=== 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:31:44 +0000
@@ -1,6 +1,6 @@
1/*1/*
2Copyright 2010 Canonical Ltd.2Copyright 2010 Canonical Ltd.
33
4Authors:4Authors:
5 Conor Curran <conor.curran@canonical.com>5 Conor Curran <conor.curran@canonical.com>
66

Subscribers

People subscribed via source and target branches

to all changes: