Merge lp:~jamesh/ubuntu-ui-toolkit/albumart-dbus into lp:ubuntu-ui-toolkit

[James Henstridge]

Add an image://albumart image provider that retrieves album art for a particular (artist, album) pair. Artwork is retrieved via a D-Bus service so it is possible for confined applications to take advantage of a centralised media art cache.

Add an image://albumart image provider that retrieves album art for a particular (artist, album) pair. Artwork is retrieved via a D-Bus service so it is possible for confined applications to take advantage of a centralised media art cache.

The D-Bus service is being added to the thumbnailer here:

https://code.launchpad.net/~jamesh/thumbnailer/dbus-service/+merge/212347

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:973
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/1929/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4167/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/3751/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-ci/877/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/877/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4234/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/3753/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/1929/rebuild

review: Needs Fixing (continuous-integration)

[Jussi Pakkanen (jpakkane)]

Looks good.

review: Approve

[lp:~jamesh/ubuntu-ui-toolkit/albumart-dbus]

974. By James Henstridge on 2014-03-24

Fix up constness of catch() clause.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:974
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/1931/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4169/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/3753/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-ci/879/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/879/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4236/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/3755/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/1931/rebuild

review: Needs Fixing (continuous-integration)

[Tim Peeters (tpeeters)]

Why should an album art generator be part of the UITK? It seems specific for only a few apps to me and I don't like to add dependencies to the UITK because that gives problems with supporting and porting (to precise for example) the UITK. Could this be included in the apps that use it or as a separate module that they can use?

[Jussi Pakkanen (jpakkane)]

The album art image provider is conceptually the same as thumbnailer image provider. The latter says "get me a thumbnail for this file" and the former is "get me an image of this album". There is already code for this in the Dash (and I think duplicated in Unity8) but that is not the correct place for it for obvious reasons.

As far as dependencies go, this does not add any extra dependencies. The UITK part is just a dbus call (the actual generation happens in a service provided by libthumbnailer). If the album art service is not running (as it probably won't on precise), the code returns the default image. The dependency on >=1.1 of thumbnailer and thumbnailer-service is just to declare this and can be changed to just libthumbnailer>=1.0 if that makes supporting precise easier (note that neither AlbumArtGenerator.cpp nor AlbumArtGenerator.h includes anything from libthumbnnailer so they will compile and link just fine).

We proposed this for the UITK because we feel that it should be as widely available as the thumbnailing image provider is. That being said the current users are the shell and the music player app. If you feel that this code has a better location somewhere else then we are glad to work with you to find one. It is a very small change, after all.

[Tim Peeters (tpeeters)]

Yes it is similar to the thumbnailer image provider. I thought there were plans for the thumbnailer also to move it out of the UITK (cf. https://bugs.launchpad.net/ubuntu-ui-toolkit/+bug/1237045/comments/17) but I don't know the status of those plans. I would not mind if the thumbnailer was also moved out of the UITK, that would prevent possible issues with porting such as https://bugs.launchpad.net/ubuntu-ui-toolkit/+bug/1237045/

[Jussi Pakkanen (jpakkane)]

We need a QQuickImageProvider that can be used by all applications, the shell and so directly from their QML files. This is the requirement we were given. Where should this functionality live, then, if it needs to be moved out of uitk?

[James Henstridge (jamesh)]

So what is the way forward here? Do we move the QML image providers to a new QML plugin in thumbnailer? If we do add a thumbnailer QML plugin, should the other image provider be moved to it? What effect does that have on application compatibility?

Also, its worth noting that the "new dependency" here is still part of the thumbnailer source package. It is a new binary package in order to satisfy multi-arch (we only want one copy of the D-Bus service on the system, as opposed to the library, where each architecture needs its own copy).

[James Henstridge (jamesh)]

Wherever the code ends up, the thumbnailer-service dependency should probably be a Recommends:

18:36 <cjwatson> jamesh: what do thumbnailer-service's own Depends look like? I'm concerned that this will cause a problem for cross-installation in development chroots
18:37 <cjwatson> jamesh: and it's pretty questionable whether e.g. the d-bus service should be mandatorily installed during development
18:37 <cjwatson> those are usually run-time facilities that tests would have to mock anyway
18:42 <jamesh> cjwatson: it depends on libthumbnailer + its dependencies (which is already a dep for ubuntu-ui-toolkit)
18:44 <cjwatson> jamesh: right, but in cross-install situations that'll involve following the dep trees on both architectures
18:44 <cjwatson> best avoided if we don't have to
18:44 <jamesh> cjwatson: if the service isn't available, the image provider should present a fallback image and log the problem
18:45 <jamesh> I guess it could be a "Recommends" in that case
18:45 <cjwatson> jamesh: ok, so that sounds like it should be Recommends not Depends, and we should take other measures if necessary to ensure that it's on our images

[Tim Peeters (tpeeters)]

After some discussion with James and Jussi on irc we decided to move the thumbnailer QML bindings out of the UITK into a new package in the thumbnailer proejct.

review: Disapprove

Unmerged revisions

974. By James Henstridge on 2014-03-24

Fix up constness of catch() clause.

973. By James Henstridge on 2014-03-24

Make qtdeclarative5-ubuntu-ui-toolkit-plugin depend on
thumbnailer-service, since the album art image provider has a runtime
dependency on this D-Bus service.

972. By James Henstridge on 2014-03-24

Small cleanups.

971. By James Henstridge on 2014-03-24

Make album art image provider call out to the D-Bus service, and follow
the URI scheme used by Unity.

970. By Jussi Pakkanen on 2014-03-10

Now with xlarge goodness.

969. By Jussi Pakkanen on 2014-03-10

Basic work on album art downloader.

968. By Launchpad Translations on behalf of ubuntu-sdk-team on 2014-03-09

Launchpad automatic translations update.

=== modified file 'debian/control'
--- debian/control	2014-03-17 09:02:58 +0000
+++ debian/control	2014-03-24 10:25:09 +0000
@@ -33,7 +33,7 @@
                libqt5organizer5,
                qtpim5-dev,
                language-pack-en-base,
-               libthumbnailer-dev,
+               libthumbnailer-dev (>=1.1),
                libdbus-1-dev,
                libnih-dbus-dev,
                xvfb,
@@ -56,6 +56,7 @@
          qtdeclarative5-window-plugin,
          qtdeclarative5-qtfeedback-plugin,
          qtdeclarative5-unity-action-plugin (>= 1.1.0),
+         thumbnailer-service,
          ttf-ubuntu-font-family,
          ubuntu-ui-toolkit-theme,
          ${misc:Depends},

=== added file 'modules/Ubuntu/Components/plugin/albumartgenerator.cpp'
--- modules/Ubuntu/Components/plugin/albumartgenerator.cpp	1970-01-01 00:00:00 +0000
+++ modules/Ubuntu/Components/plugin/albumartgenerator.cpp	2014-03-24 10:25:09 +0000
@@ -0,0 +1,90 @@
+/*
+ * Copyright 2014 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors: Jussi Pakkanen <[e-mail]>
+ *          James Henstridge <[e-mail]>
+*/
+
+#include "albumartgenerator.h"
+#include <stdexcept>
+#include <QDebug>
+#include <QFile>
+#include <QUrlQuery>
+#include <QDBusUnixFileDescriptor>
+#include <QDBusReply>
+
+static const char DEFAULT_ALBUM_ART[] = "/usr/share/unity/icons/album_missing.png";
+
+static const char BUS_NAME[] = "com.canonical.Thumbnailer";
+static const char BUS_PATH[] = "/com/canonical/Thumbnailer";
+static const char THUMBNAILER_IFACE[] = "com.canonical.Thumbnailer";
+static const char GET_ALBUM_ART[] = "GetAlbumArt";
+
+AlbumArtGenerator::AlbumArtGenerator()
+    : QQuickImageProvider(QQuickImageProvider::Image, QQmlImageProviderBase::ForceAsynchronousImageLoading),
+      iface(BUS_NAME, BUS_PATH, THUMBNAILER_IFACE) {
+}
+
+static QImage fallbackImage(QSize *realSize) {
+    QImage fallback;
+    fallback.load(DEFAULT_ALBUM_ART);
+    *realSize = fallback.size();
+    return fallback;
+}
+
+QImage AlbumArtGenerator::requestImage(const QString &id, QSize *realSize,
+        const QSize &requestedSize) {
+    QUrlQuery query(id);
+    if (!query.hasQueryItem("artist") || !query.hasQueryItem("album")) {
+        qWarning() << "Invalid albumart uri:" << id;
+        return fallbackImage(realSize);
+    }
+
+    const QString artist = query.queryItemValue("artist", QUrl::FullyDecoded);
+    const QString album = query.queryItemValue("album", QUrl::FullyDecoded);
+
+    QString desiredSize = "original";
+    int size = requestedSize.width() > requestedSize.height() ? requestedSize.width() : requestedSize.height();
+    if (size < 128) {
+        desiredSize = "small";
+    } else if (size < 256) {
+        desiredSize = "large";
+    } else if (size < 512) {
+        desiredSize = "xlarge";
+    }
+
+    // perform dbus call
+    QDBusReply<QDBusUnixFileDescriptor> reply = iface.call(
+        GET_ALBUM_ART, artist, album, desiredSize);
+    if (!reply.isValid()) {
+        qWarning() << "D-Bus error: " << reply.error().message();
+        return fallbackImage(realSize);
+    }
+
+    try {
+        QFile file;
+        file.open(reply.value().fileDescriptor(), QIODevice::ReadOnly);
+        QImage image;
+        image.load(&file, NULL);
+        *realSize = image.size();
+        return image;
+    } catch (const std::exception &e) {
+        qDebug() << "Album art loader failed: " << e.what();
+    } catch (...) {
+        qDebug() << "Unknown error when generating image.";
+    }
+
+    return fallbackImage(realSize);
+}

=== added file 'modules/Ubuntu/Components/plugin/albumartgenerator.h'
--- modules/Ubuntu/Components/plugin/albumartgenerator.h	1970-01-01 00:00:00 +0000
+++ modules/Ubuntu/Components/plugin/albumartgenerator.h	2014-03-24 10:25:09 +0000
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2014 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors: Jussi Pakkanen <[e-mail]>
+ *          James Henstridge <[e-mail]>
+*/
+
+#ifndef ALBUMART_GENERATOR_H
+#define ALBUMART_GENERATOR_H
+
+#include <QDBusInterface>
+#include <QQuickImageProvider>
+
+class AlbumArtGenerator: public QQuickImageProvider
+{
+private:
+    QDBusInterface iface;
+
+public:
+    AlbumArtGenerator();
+    QImage requestImage(const QString &id, QSize *size, const QSize &requestedSize);
+};
+
+#endif

=== modified file 'modules/Ubuntu/Components/plugin/plugin.cpp'
--- modules/Ubuntu/Components/plugin/plugin.cpp	2014-02-13 17:16:06 +0000
+++ modules/Ubuntu/Components/plugin/plugin.cpp	2014-03-24 10:25:09 +0000
@@ -37,6 +37,7 @@
 #include "qquickmimedata.h"
 #include "bottombarvisibilitycommunicator.h"
 #include "thumbnailgenerator.h"
+#include "albumartgenerator.h"
 #include "ucubuntuanimation.h"
 #include "ucfontutils.h"
 #include "ucarguments.h"
@@ -237,6 +238,11 @@
     } catch(std::runtime_error &e) {
         qDebug() << "Could not create thumbnailer: " << e.what();
     }
+    try {
+        engine->addImageProvider(QLatin1String("albumart"), new AlbumArtGenerator);
+    } catch(std::runtime_error &e) {
+        qDebug() << "Could not create albumart: " << e.what();
+    }
 
     // Necessary for Screen.orientation (from import QtQuick.Window 2.0) to work
     QGuiApplication::primaryScreen()->setOrientationUpdateMask(

=== modified file 'modules/Ubuntu/Components/plugin/plugin.pro'
--- modules/Ubuntu/Components/plugin/plugin.pro	2014-02-13 09:38:23 +0000
+++ modules/Ubuntu/Components/plugin/plugin.pro	2014-03-24 10:25:09 +0000
@@ -52,6 +52,7 @@
     ucalarmmodel.h \
     unitythemeiconprovider.h \
     thumbnailgenerator.h \
+    albumartgenerator.h \
     alarmrequest_p.h \
     alarmrequest_p_p.h \
     adapters/alarmsadapter_p.h \
@@ -86,6 +87,7 @@
     ucalarmmodel.cpp \
     unitythemeiconprovider.cpp \
     thumbnailgenerator.cpp \
+    albumartgenerator.cpp \
     alarmrequest_p.cpp \
     ucstatesaver.cpp \
     statesaverbackend_p.cpp \

=== modified file 'modules/Ubuntu/Components/plugin/thumbnailgenerator.cpp'
--- modules/Ubuntu/Components/plugin/thumbnailgenerator.cpp	2014-02-25 09:14:23 +0000
+++ modules/Ubuntu/Components/plugin/thumbnailgenerator.cpp	2014-03-24 10:25:09 +0000
@@ -22,8 +22,8 @@
 #include <QMimeDatabase>
 #include <QUrl>
 
-const char *DEFAULT_VIDEO_ART = "/usr/share/unity/icons/video_missing.png";
-const char *DEFAULT_ALBUM_ART = "/usr/share/unity/icons/album_missing.png";
+static const char *DEFAULT_VIDEO_ART = "/usr/share/unity/icons/video_missing.png";
+static const char *DEFAULT_ALBUM_ART = "/usr/share/unity/icons/album_missing.png";
 
 ThumbnailGenerator::ThumbnailGenerator() : QQuickImageProvider(QQuickImageProvider::Image,
         QQmlImageProviderBase::ForceAsynchronousImageLoading) {
@@ -45,10 +45,13 @@
     std::string tgt_path;
     try {
         ThumbnailSize desiredSize;
+        const int xlarge_cutoff = 512;
         const int large_cutoff = 256;
         const int small_cutoff = 128;
-        if(requestedSize.width() > large_cutoff || requestedSize.height() > large_cutoff) {
+        if(requestedSize.width() > xlarge_cutoff || requestedSize.height() > xlarge_cutoff) {
             desiredSize = TN_SIZE_ORIGINAL;
+        } if(requestedSize.width() > large_cutoff || requestedSize.height() > large_cutoff) {
+                desiredSize = TN_SIZE_XLARGE;
         } else if(requestedSize.width() > small_cutoff || requestedSize.height() > small_cutoff) {
             desiredSize = TN_SIZE_LARGE;
         } else {