Merge lp:~jamesh/thumbnailer/no-fallback-albumart into lp:thumbnailer

Proposed by James Henstridge
Status: Superseded
Proposed branch: lp:~jamesh/thumbnailer/no-fallback-albumart
Merge into: lp:thumbnailer
Diff against target: 68 lines (+11/-12)
2 files modified
plugins/Ubuntu/Thumbnailer/albumartgenerator.cpp (+3/-12)
tests/qml/tst_image_provider.qml (+8/-0)
To merge this branch: bzr merge lp:~jamesh/thumbnailer/no-fallback-albumart
Reviewer Review Type Date Requested Status
James Henstridge Needs Fixing
Jussi Pakkanen (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email:

This proposal has been superseded by a proposal from 2015-06-29.

Commit message

Make the album art image provider return a null QImage on error so that applications can detect the failure and handle fallback themselves.

Description of the change

Rather than providing a fallback image from the albumart ImageProvider, return a null QImage object. When used with the QML Image component, this sets the status to Image.Error which allows applications to handle the fallback themselves.

This was requested by the music-app guys, but I haven't yet verified behaviour in the dash.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:85
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
Executed test runs:

Click here to trigger a rebuild:

review: Needs Fixing (continuous-integration)
Revision history for this message
Jussi Pakkanen (jpakkane) wrote :

Looks good.

review: Approve
Revision history for this message
James Henstridge (jamesh) wrote :

It looks like this does cause problems for the dash: I get an empty space in the carousel where a song without album art sits.

We'll need some kind of fix for this on the Unity8 side before this can be merged.

review: Needs Fixing
Revision history for this message
James Henstridge (jamesh) wrote :

I've filed a bug 1324142 on Unity8 about getting it to handle the error state correctly.

Revision history for this message
Jussi Pakkanen (jpakkane) wrote :

This has lingered for quite a while. What shall we do with this?

86. By James Henstridge

Merge from devel

87. By James Henstridge

Update "no fallback" code to work with QQuickAsyncImageResponse.

88. By James Henstridge

Simplify image response code a bit more. Still doesn't work right due
to Qt bug:

89. By James Henstridge

Include the ID in the errors from album/artist art generators.

90. By James Henstridge

Merge from devel

91. By James Henstridge

Fix up tests for removal of fallback art.

92. By James Henstridge

Remove fallback images.

93. By James Henstridge

Remove the default image override code from tests.

94. By James Henstridge

Remove the thumbnailer-common binary package:
 * Move the apport blacklist file to thumbnailer-service (only listing
   current architecture now, since it is arch specific).
 * Remove the fallback images entirely, since they are no longer used.

95. By James Henstridge

Merge from devel, fixing massive conflicts.

96. By James Henstridge

Fix up QML tests.

97. By James Henstridge

Add direct image provider tests for error conditions.

98. By James Henstridge

Remove obsolete comment from textureFactory() method.

99. By James Henstridge

Remove error logging in requestFinished() method.

We now expose the error message via the errorString() method, which Qt
itself will log.

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/Ubuntu/Thumbnailer/albumartgenerator.cpp'
2--- plugins/Ubuntu/Thumbnailer/albumartgenerator.cpp 2014-03-28 04:08:24 +0000
3+++ plugins/Ubuntu/Thumbnailer/albumartgenerator.cpp 2014-05-14 07:15:45 +0000
4@@ -25,8 +25,6 @@
5 #include <QDBusUnixFileDescriptor>
6 #include <QDBusReply>
8-static const char DEFAULT_ALBUM_ART[] = "/usr/share/unity/icons/album_missing.png";
10 static const char BUS_NAME[] = "com.canonical.Thumbnailer";
11 static const char BUS_PATH[] = "/com/canonical/Thumbnailer";
12 static const char THUMBNAILER_IFACE[] = "com.canonical.Thumbnailer";
13@@ -37,19 +35,12 @@
15 }
17-static QImage fallbackImage(QSize *realSize) {
18- QImage fallback;
19- fallback.load(DEFAULT_ALBUM_ART);
20- *realSize = fallback.size();
21- return fallback;
24 QImage AlbumArtGenerator::requestImage(const QString &id, QSize *realSize,
25 const QSize &requestedSize) {
26 QUrlQuery query(id);
27 if (!query.hasQueryItem("artist") || !query.hasQueryItem("album")) {
28 qWarning() << "Invalid albumart uri:" << id;
29- return fallbackImage(realSize);
30+ return QImage();
31 }
33 const QString artist = query.queryItemValue("artist", QUrl::FullyDecoded);
34@@ -70,7 +61,7 @@
35 GET_ALBUM_ART, artist, album, desiredSize);
36 if (!reply.isValid()) {
37 qWarning() << "D-Bus error: " << reply.error().message();
38- return fallbackImage(realSize);
39+ return QImage();
40 }
42 try {
43@@ -86,5 +77,5 @@
44 qDebug() << "Unknown error when generating image.";
45 }
47- return fallbackImage(realSize);
48+ return QImage();
49 }
51=== modified file 'tests/qml/tst_image_provider.qml'
52--- tests/qml/tst_image_provider.qml 2014-03-28 08:35:04 +0000
53+++ tests/qml/tst_image_provider.qml 2014-05-14 07:15:45 +0000
54@@ -33,6 +33,14 @@
55 comparePixel(ctx, 0, 0, 242, 228, 209, 255);
56 }
58+ function test_notfound() {
59+ image.source = "image://albumart/artist=notfound&album=notfound";
60+ while (image.status == Image.Loading) {
61+ spy.wait();
62+ }
63+ compare(image.status, Image.Error);
64+ }
66 function loadImage(uri) {
67 image.source = uri
68 while (image.status == Image.Loading) {


People subscribed via source and target branches

to all changes: