Merge lp:~phablet-team/media-hub/support-albumart into lp:media-hub

[Jim Hodapp]

Support albumart for music and video tracks.

Support albumart for music and video tracks.

[lp:~phablet-team/media-hub/support-albumart]

192. By Jim Hodapp on 2016-06-15

Add some more robustness and make sure artist and album names are percent encoded when being passed as an image://albumart/ URI

193. By Jim Hodapp on 2016-06-15

Clean up unnecessary debug statements

194. By Jim Hodapp on 2016-06-16

Got rid of one last unnecessary debug line

195. By Jim Hodapp on 2016-06-16

Merged with upstream

Unmerged revisions

=== modified file 'include/core/media/track.h'
--- include/core/media/track.h	2015-07-02 12:30:09 +0000
+++ include/core/media/track.h	2016-06-14 19:43:39 +0000
@@ -39,10 +39,15 @@
 public:
     typedef std::string UriType;
     typedef std::string Id;
+    typedef std::map<std::string, std::string> MetaDataType;
 
     class MetaData
     {
     public:
+        static constexpr const char* TrackArtlUrlKey = "mpris:artUrl";
+        static constexpr const char* TrackLengthKey = "mpris:length";
+        static constexpr const char* TrackIdKey = "mpris:trackid";
+
         bool operator==(const MetaData& rhs) const
         {
             return map == rhs.map;
@@ -90,13 +95,36 @@
             return map.at(key);
         }
 
+        bool is_set(const std::string& key) const
+        {
+            try {
+                return map.at(key).empty();
+            } catch (const std::out_of_range& e) {
+                return false;
+            }
+        }
+
         const std::map<std::string, std::string>& operator*() const
         {
             return map;
         }
 
+        const std::string& album() const;
+        const std::string& artist() const;
+        const std::string& title() const;
+        const std::string& track_id() const;
+        const std::string& art_url() const;
+        const std::string& last_used() const;
+
+        void set_album(const std::string& album);
+        void set_artist(const std::string& artist);
+        void set_title(const std::string& title);
+        void set_track_id(const std::string& id);
+        void set_art_url(const std::string& url);
+        void set_last_used(const std::string& datetime);
+
     private:
-        std::map<std::string, std::string> map;
+        MetaDataType map;
     };
 
     Track(const Id& id);
@@ -108,51 +136,7 @@
 
     virtual const Id& id() const;
     virtual const UriType& uri() const;
-    /*
-    class MetaData
-    {
-    public:
-        MetaData() = default;
-        MetaData(const MetaData&) = default;
-        ~MetaData() = default;
-
-        MetaData& operator=(const MetaData&) = default;
-
-        bool operator==(const MetaData&) const
-        {
-            return true;
-        }
-
-        bool operator!=(const MetaData&) const
-        {
-            return false;
-        }
-
-        struct NotImplementedFields
-        {
-            NotImplementedFields() = default;
-
-            virtual const UriType& uri() const = 0;
-            virtual const std::chrono::microseconds length() const = 0;
-            virtual const UriType& art_uri() const = 0;
-            virtual const std::string& album() const = 0;
-            virtual const std::vector<std::string>& album_artist() const = 0;
-            virtual const std::vector<std::string>& artist() const = 0;
-            virtual const std::string& as_text() const = 0;
-            virtual unsigned int audio_bpm() const = 0;
-            virtual float auto_rating() const = 0;
-            virtual const std::vector<std::string>& comment() const = 0;
-            virtual const std::vector<std::string>& composer() const = 0;
-            virtual unsigned int disc_number() const = 0;
-            virtual const std::vector<std::string>& genre() const = 0;
-            virtual const std::vector<std::string>& lyricist() const = 0;
-            virtual const std::string title() const = 0;
-            virtual unsigned int track_number() const = 0;
-            virtual unsigned int use_count() const = 0;
-            virtual float user_rating() const = 0;
-        };
-    };
-*/
+
 private:
     struct Private;
     std::unique_ptr<Private> d;

=== modified file 'src/core/media/CMakeLists.txt'
--- src/core/media/CMakeLists.txt	2016-04-05 15:40:11 +0000
+++ src/core/media/CMakeLists.txt	2016-06-14 19:43:39 +0000
@@ -49,6 +49,7 @@
   service.cpp
   track.cpp
   track_list.cpp
+  metadata.cpp
 
   player_stub.cpp
   service_stub.cpp
@@ -99,6 +100,7 @@
     hybris_client_death_observer.cpp
     cover_art_resolver.cpp
     engine.cpp
+    metadata.cpp
 
     apparmor/context.cpp
     apparmor/ubuntu.cpp

=== modified file 'src/core/media/codec.h'
--- src/core/media/codec.h	2015-04-14 19:11:15 +0000
+++ src/core/media/codec.h	2016-06-14 19:43:39 +0000
@@ -23,10 +23,18 @@
 #include <core/media/player.h>
 #include <core/media/track.h>
 
+#include <core/dbus/types/stl/map.h>
 #include <core/dbus/types/stl/string.h>
+#include <core/dbus/types/variant.h>
 #include <core/dbus/types/stl/vector.h>
 #include <core/dbus/codec.h>
 
+#include <boost/lexical_cast.hpp>
+
+#include "core/media/xesam.h"
+
+#include "core/media/logger/logger.h"
+
 namespace core
 {
 namespace dbus
@@ -38,7 +46,7 @@
 {
     constexpr static ArgumentType type_value()
     {
-        return ArgumentType::floating_point;
+        return ArgumentType::array;
     }
     constexpr static bool is_basic_type()
     {
@@ -51,7 +59,7 @@
 
     static std::string signature()
     {
-        static const std::string s = TypeMapper<double>::signature();
+        static const std::string s = TypeMapper<std::map<std::string, dbus::types::Variant>>::signature();
         return s;
     }
 };
@@ -60,16 +68,88 @@
 template<>
 struct Codec<core::ubuntu::media::Track::MetaData>
 {
-    static void encode_argument(core::dbus::Message::Writer& out, const core::ubuntu::media::Track::MetaData& in)
+    static void encode_argument(core::dbus::Message::Writer& writer, const core::ubuntu::media::Track::MetaData& md)
     {
-        (void) out;
-        (void) in;
+        typedef std::pair<std::string, dbus::types::Variant> Pair;
+        auto dict = writer.open_array(dbus::types::Signature
+                {dbus::helper::TypeMapper<Pair>::signature()});
+
+        for (const auto& pair : *md)
+        {
+            // The following tags are not part of the MPRIS spec and should not be encoded
+            if (pair.first == tags::Image::name or
+                    pair.first == tags::PreviewImage::name)
+                continue;
+
+            auto de = dict.open_dict_entry();
+            {
+                if (pair.first == core::ubuntu::media::Track::MetaData::TrackLengthKey
+                        and not pair.second.empty())
+                {
+                    Codec<Pair>::encode_argument(de, std::make_pair(pair.first,
+                                dbus::types::Variant::encode(
+                                    boost::lexical_cast<std::int64_t>(pair.second))));
+                }
+                else if (pair.first == xesam::Album::name and not pair.second.empty())
+                {
+                    Codec<Pair>::encode_argument(de, std::make_pair(pair.first,
+                                dbus::types::Variant::encode(pair.second)));
+                }
+                else if (pair.first == xesam::AlbumArtist::name and not pair.second.empty())
+                {
+#if 0
+                    // TODO: This code doesn't work but will be needed for full MPRIS compliance.
+                    // Technically there can be more than one album artist stuffed into an array.
+                    // How to satisfy stuffing this data into dbus-cpp is what needs fixing.
+                    auto array = de.open_array(dbus::types::Signature
+                            {dbus::helper::TypeMapper<Pair>::signature()});
+                    {
+                        // TODO: this should really iterate over all artists, but seems
+                        // we only extract one artist from playbin
+                        Codec<Pair>::encode_argument(array, std::make_pair(pair.first,
+                                    dbus::types::Variant::encode(pair.second)));
+                    }
+                    de.close_array(std::move(array));
+#else
+                    Codec<Pair>::encode_argument(de, std::make_pair(pair.first,
+                                dbus::types::Variant::encode(pair.second)));
+#endif
+                }
+                else
+                {
+                    Codec<Pair>::encode_argument(de, std::make_pair(pair.first,
+                                dbus::types::Variant::encode(pair.second)));
+                }
+            }
+            dict.close_dict_entry(std::move(de));
+        }
+        writer.close_array(std::move(dict));
     }
 
-    static void decode_argument(core::dbus::Message::Reader& out, core::ubuntu::media::Track::MetaData& in)
+    static void decode_argument(core::dbus::Message::Reader& reader, core::ubuntu::media::Track::MetaData& md)
     {
-        (void) out;
-        (void) in;
+        auto array = reader.pop_array();
+
+        while (array.type() != dbus::ArgumentType::invalid)
+        {
+            auto entry = array.pop_dict_entry();
+            {
+                std::string key {entry.pop_string()};
+                auto variant = entry.pop_variant();
+                {
+                    if (key == xesam::Album::name)
+                    {
+                        const std::string album = variant.pop_string();
+                        MH_DEBUG("Getting key \"%s\" and value \"%s\"", key, album);
+                        md.set_album(album);
+                    }
+                    else
+                    {
+                        MH_WARNING("Unknown metadata key \"%s\" while decoding dbus message", key);
+                    }
+                }
+            }
+        }
     }
 };
 

=== modified file 'src/core/media/cover_art_resolver.cpp'
--- src/core/media/cover_art_resolver.cpp	2014-09-09 14:02:52 +0000
+++ src/core/media/cover_art_resolver.cpp	2016-06-14 19:43:39 +0000
@@ -22,6 +22,6 @@
 {
     return [](const std::string&, const std::string&, const std::string&)
     {
-        return "file:///usr/share/unity/icons/album_missing.png";
+        return "file:///usr/lib/arm-linux-gnueabihf/unity-scopes/mediascanner-music/album_missing.svg";
     };
 }

=== modified file 'src/core/media/gstreamer/meta_data_extractor.h'
--- src/core/media/gstreamer/meta_data_extractor.h	2015-09-23 11:17:26 +0000
+++ src/core/media/gstreamer/meta_data_extractor.h	2016-06-14 19:43:39 +0000
@@ -24,6 +24,8 @@
 
 #include "bus.h"
 
+#include "core/media/logger/logger.h"
+
 #include <gst/gst.h>
 
 #include <exception>
@@ -49,7 +51,11 @@
             {GST_TAG_GENRE, std::string{xesam::Genre::name}},
             {GST_TAG_TITLE, std::string{xesam::Title::name}},
             {GST_TAG_TRACK_NUMBER, std::string{xesam::TrackNumber::name}},
-            {GST_TAG_USER_RATING, std::string{xesam::UserRating::name}}
+            {GST_TAG_USER_RATING, std::string{xesam::UserRating::name}},
+            // Below this line are custom entries related but not directly from
+            // the MPRIS spec:
+            {GST_TAG_IMAGE, std::string{tags::Image::name}},
+            {GST_TAG_PREVIEW_IMAGE, std::string{tags::PreviewImage::name}}
         };
 
         return lut;
@@ -72,7 +78,7 @@
             auto md = static_cast<media::Track::MetaData*>(user_data);
             std::stringstream ss;
 
-            switch(gst_tag_get_type(tag))
+            switch (gst_tag_get_type(tag))
             {
             case G_TYPE_BOOLEAN:
             {
@@ -137,9 +143,22 @@
                 break;
             }
 
-            (*md).set(
-                        (gstreamer_to_mpris_tag_lut().count(tag) > 0 ? gstreamer_to_mpris_tag_lut().at(tag) : tag),
-                        ss.str());
+            const bool has_tag_from_lut = (gstreamer_to_mpris_tag_lut().count(tag) > 0);
+            const std::string tag_name{(has_tag_from_lut) ?
+                    gstreamer_to_mpris_tag_lut().at(tag) : tag};
+
+
+            // Specific handling for the following tag types:
+            if (tag_name == tags::PreviewImage::name)
+                ss << "true";
+            if (tag_name == tags::Image::name)
+                ss << "true";
+
+            (*md).set((has_tag_from_lut ?
+                            gstreamer_to_mpris_tag_lut().at(tag) : tag), ss.str());
+
+            MH_DEBUG("Tag name: \"%s\", value: \"%s\"", tag_name, ss.str());
+
         },
         &md);
     }

=== modified file 'src/core/media/gstreamer/playbin.cpp'
--- src/core/media/gstreamer/playbin.cpp	2016-04-06 15:28:29 +0000
+++ src/core/media/gstreamer/playbin.cpp	2016-06-14 19:43:39 +0000
@@ -30,8 +30,6 @@
 
 #include <utility>
 
-//#define VERBOSE_DEBUG
-
 namespace
 {
 void setup_video_sink_for_buffer_streaming(GstElement* pipeline)
@@ -256,7 +254,7 @@
 
 void gstreamer::Playbin::on_new_message_async(const Bus::Message& message)
 {
-    switch(message.type)
+    switch (message.type)
     {
     case GST_MESSAGE_ERROR:
         signals.on_error(message.detail.error_warning_info);

=== added file 'src/core/media/metadata.cpp'
--- src/core/media/metadata.cpp	1970-01-01 00:00:00 +0000
+++ src/core/media/metadata.cpp	2016-06-14 19:43:39 +0000
@@ -0,0 +1,83 @@
+/*
+ * Copyright © 2016 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 version 3,
+ * as published by the Free Software Foundation.
+ *
+ * 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/>.
+ *
+ * Authored by: Jim Hodapp <[e-mail]>
+ */
+
+#include "xesam.h"
+
+#include <core/media/track.h>
+
+namespace media = core::ubuntu::media;
+
+const std::string& media::Track::MetaData::album() const
+{
+    return map.at(xesam::Album::name);
+}
+
+const std::string& media::Track::MetaData::artist() const
+{
+    return map.at(xesam::Artist::name);
+}
+
+const std::string& media::Track::MetaData::title() const
+{
+    return map.at(xesam::Title::name);
+}
+
+const std::string& media::Track::MetaData::track_id() const
+{
+    return map.at(media::Track::MetaData::TrackIdKey);
+}
+
+const std::string& media::Track::MetaData::art_url() const
+{
+    return map.at(media::Track::MetaData::TrackArtlUrlKey);
+}
+
+const std::string& media::Track::MetaData::last_used() const
+{
+    return map.at(xesam::LastUsed::name);
+}
+
+void media::Track::MetaData::set_album(const std::string& album)
+{
+    map[xesam::Album::name] = album;
+}
+
+void media::Track::MetaData::set_artist(const std::string& artist)
+{
+    map[xesam::Artist::name] = artist;
+}
+
+void media::Track::MetaData::set_title(const std::string& title)
+{
+    map[xesam::Title::name] = title;
+}
+
+void media::Track::MetaData::set_track_id(const std::string& id)
+{
+    map[media::Track::MetaData::TrackIdKey] = id;
+}
+
+void media::Track::MetaData::set_art_url(const std::string& url)
+{
+    map[media::Track::MetaData::TrackArtlUrlKey] = url;
+}
+
+void media::Track::MetaData::set_last_used(const std::string& datetime)
+{
+    map[xesam::LastUsed::name] = datetime;
+}

=== modified file 'src/core/media/mpris/player.h'
--- src/core/media/mpris/player.h	2016-02-19 16:14:42 +0000
+++ src/core/media/mpris/player.h	2016-06-14 19:43:39 +0000
@@ -44,6 +44,8 @@
 
 #include <cstdint>
 
+#include "core/media/logger/logger.h"
+
 namespace dbus = core::dbus;
 
 namespace mpris
@@ -172,8 +174,7 @@
         DBUS_CPP_WRITABLE_PROPERTY_DEF(PlaybackRate, Player, double)
         DBUS_CPP_WRITABLE_PROPERTY_DEF(Rate, Player, double)
         DBUS_CPP_WRITABLE_PROPERTY_DEF(Shuffle, Player, bool)
-        DBUS_CPP_READABLE_PROPERTY_DEF(Metadata, Player, Dictionary)
-        DBUS_CPP_READABLE_PROPERTY_DEF(TypedMetaData, Player, core::ubuntu::media::Track::MetaData)
+        DBUS_CPP_READABLE_PROPERTY_DEF(Metadata, Player, core::ubuntu::media::Track::MetaData)
         DBUS_CPP_WRITABLE_PROPERTY_DEF(Volume, Player, double)
         DBUS_CPP_READABLE_PROPERTY_DEF(Position, Player, std::int64_t)
         DBUS_CPP_READABLE_PROPERTY_DEF(Duration, Player, std::int64_t)
@@ -222,7 +223,7 @@
                 Properties::TypedLoopStatus::ValueType typed_loop_status{core::ubuntu::media::Player::LoopStatus::none};
                 Properties::PlaybackRate::ValueType playback_rate{1.f};
                 Properties::Shuffle::ValueType shuffle{false};
-                Properties::TypedMetaData::ValueType typed_meta_data{};
+                Properties::Metadata::ValueType meta_data{};
                 Properties::Volume::ValueType volume{0.f};
                 Properties::Position::ValueType position{0};
                 Properties::Duration::ValueType duration{0};
@@ -253,7 +254,7 @@
                   configuration.object->template get_property<Properties::Lifetime>(),
                   configuration.object->template get_property<Properties::PlaybackRate>(),
                   configuration.object->template get_property<Properties::Shuffle>(),
-                  configuration.object->template get_property<Properties::TypedMetaData>(),
+                  configuration.object->template get_property<Properties::Metadata>(),
                   configuration.object->template get_property<Properties::Volume>(),
                   configuration.object->template get_property<Properties::Position>(),
                   configuration.object->template get_property<Properties::Duration>(),
@@ -288,6 +289,7 @@
             properties.lifetime->set(core::ubuntu::media::Player::Lifetime::normal);
             properties.playback_rate->set(configuration.defaults.playback_rate);
             properties.shuffle->set(configuration.defaults.shuffle);
+            properties.meta_data_for_current_track->set(configuration.defaults.meta_data);
             properties.position->set(configuration.defaults.position);
             properties.duration->set(configuration.defaults.duration);
             properties.minimum_playback_rate->set(configuration.defaults.minimum_rate);
@@ -351,9 +353,9 @@
             Dictionary dict; dict[Property::name()] = dbus::types::Variant::encode(value);
 
             signals.properties_changed->emit(std::make_tuple(
-                            dbus::traits::Service<Player>::interface_name(),
-                            dict,
-                            the_empty_list_of_invalidated_properties()));
+                        dbus::traits::Service<Player>::interface_name(),
+                        dict,
+                        the_empty_list_of_invalidated_properties()));
         }
 
         Dictionary get_all_properties()
@@ -374,6 +376,7 @@
             dict[Properties::Lifetime::name()] = dbus::types::Variant::encode(properties.lifetime->get());
             dict[Properties::PlaybackRate::name()] = dbus::types::Variant::encode(properties.playback_rate->get());
             dict[Properties::Shuffle::name()] = dbus::types::Variant::encode(properties.shuffle->get());
+            dict[Properties::Metadata::name()] = dbus::types::Variant::encode(properties.meta_data_for_current_track->get());
             dict[Properties::Duration::name()] = dbus::types::Variant::encode(properties.duration->get());
             dict[Properties::Position::name()] = dbus::types::Variant::encode(properties.position->get());
             dict[Properties::MinimumRate::name()] = dbus::types::Variant::encode(properties.minimum_playback_rate->get());
@@ -405,7 +408,7 @@
             std::shared_ptr<core::dbus::Property<Properties::Lifetime>> lifetime;
             std::shared_ptr<core::dbus::Property<Properties::PlaybackRate>> playback_rate;
             std::shared_ptr<core::dbus::Property<Properties::Shuffle>> shuffle;
-            std::shared_ptr<core::dbus::Property<Properties::TypedMetaData>> typed_meta_data_for_current_track;
+            std::shared_ptr<core::dbus::Property<Properties::Metadata>> meta_data_for_current_track;
             std::shared_ptr<core::dbus::Property<Properties::Volume>> volume;
             std::shared_ptr<core::dbus::Property<Properties::Position>> position;
             std::shared_ptr<core::dbus::Property<Properties::Duration>> duration;

=== modified file 'src/core/media/player_implementation.cpp'
--- src/core/media/player_implementation.cpp	2016-05-03 19:08:00 +0000
+++ src/core/media/player_implementation.cpp	2016-06-14 19:43:39 +0000
@@ -24,10 +24,12 @@
 #include "util/timeout.h"
 
 #include <unistd.h>
+#include <ctime>
 
 #include "client_death_observer.h"
 #include "engine.h"
 #include "track_list_implementation.h"
+#include "xesam.h"
 
 #include "gstreamer/engine.h"
 
@@ -138,9 +140,19 @@
             }
             case Engine::State::playing:
             {
-                // We update the track meta data prior to updating the playback status.
+                // We update the track metadata prior to updating the playback status.
                 // Some MPRIS clients expect this order of events.
-                parent->meta_data_for_current_track().set(std::get<1>(engine->track_meta_data().get()));
+                time_t now;
+                time(&now);
+                char buf[sizeof("2011-10-08T07:07:09Z")];
+                strftime(buf, sizeof(buf), "%FT%TZ", gmtime(&now));
+                media::Track::MetaData metadata{std::get<1>(engine->track_meta_data().get())};
+                // Setting this with second resolution makes sure that the track_meta_data property changes
+                // and thus the track_meta_data().changed() signal gets sent out over dbus. Otherwise the
+                // Property caching mechanism would prevent this.
+                metadata.set_last_used(std::string{buf});
+                update_mpris_metadata(std::get<0>(engine->track_meta_data().get()), metadata);
+
                 // And update our playback status.
                 parent->playback_status().set(media::Player::playing);
                 MH_INFO("Requesting power state");
@@ -325,6 +337,78 @@
         parent->can_go_next().set(has_next);
     }
 
+    std::string get_uri_for_album_artwork(const media::Track::UriType& uri,
+            const media::Track::MetaData& metadata)
+    {
+        std::string art_uri;
+        static const std::string file_uri_prefix{"file://"};
+        size_t pos = uri.find_first_of(file_uri_prefix);
+        MH_DEBUG("find_first_of: %d", pos != std::string::npos);
+        MH_DEBUG("uri.at(0): %c", uri.at(0));
+        const bool is_local_file = (pos != std::string::npos or uri.at(0) == '/');
+        MH_DEBUG("is_local_file ? %d", is_local_file);
+        MH_DEBUG("is_video_source: %d", parent->is_video_source().get());
+        // If the track has a full image or preview image or is a video and it is a local file,
+        // then use the thumbnailer cache
+        if (  (( metadata.count(tags::PreviewImage::name) > 0
+                 and metadata.get(tags::PreviewImage::name) == "true")
+           or ( metadata.count(tags::Image::name) > 0
+                 and metadata.get(tags::Image::name) == "true")
+           or parent->is_video_source().get())
+           and is_local_file)
+        {
+            MH_DEBUG("Detected the presence of a local preview image or full image");
+            art_uri = "image://thumbnailer/" + uri;
+            MH_DEBUG("art_uri: %s", art_uri);
+        }
+        // Otherwise we'll try and see if we can look up the album art online through
+        // the dash's artwork proxy
+        else if (metadata.is_set(xesam::Album::name) or metadata.is_set(xesam::Artist::name))
+        {
+            if (metadata.is_set(xesam::Album::name) and metadata.is_set(xesam::Artist::name))
+            {
+                art_uri = "image://albumart/artist=" + metadata.artist()
+                    + "&album=" + metadata.album();
+            }
+            else
+            {
+                art_uri = "image://albumart/";
+                if (metadata.is_set(xesam::Artist::name))
+                    art_uri += "artist=" + metadata.artist();
+                else if (metadata.is_set(xesam::Album::name))
+                    art_uri += "album=" + metadata.album();
+            }
+        }
+        // If all else fails, display a placeholder icon
+        else
+        {
+            art_uri = "file:///usr/lib/arm-linux-gnueabihf/unity-scopes/mediascanner-music/album_missing.svg";
+        }
+
+        return art_uri;
+    }
+
+    // Makes sure all relevant metadata fields are set to current data and
+    // will trigger the track_meta_data().changed() signal to go out over dbus
+    void update_mpris_metadata(const media::Track::UriType& uri, const media::Track::MetaData& md)
+    {
+        media::Track::MetaData metadata{md};
+        if (not metadata.is_set(media::Track::MetaData::TrackIdKey))
+        {
+            const std::size_t last_slash = track_list->current().find_last_of("/");
+            const std::string track_id = track_list->current().substr(last_slash+1);
+            if (not track_id.empty())
+                metadata.set_track_id("/org/mpris/MediaPlayer2/Track/" + track_id);
+            else
+                MH_WARNING("Failed to set MPRIS track id since the id value is NULL");
+        }
+
+        if (not metadata.is_set(media::Track::MetaData::TrackArtlUrlKey))
+            metadata.set_art_url(get_uri_for_album_artwork(uri, metadata));
+
+        parent->meta_data_for_current_track().set(metadata);
+    }
+
     bool pause_other_players(media::Player::PlayerKey key)
     {
         if (not config.parent.player_service)
@@ -501,6 +585,12 @@
         d->engine->lifetime().set(lifetime);
     });
 
+    d->engine->track_meta_data().changed().connect([this, config](
+           const std::tuple<media::Track::UriType, media::Track::MetaData>& md)
+    {
+        d->update_mpris_metadata(std::get<0>(md), std::get<1>(md));
+    });
+
     d->engine->about_to_finish_signal().connect([this]()
     {
         if (d->doing_abandon)

=== modified file 'src/core/media/player_skeleton.cpp'
--- src/core/media/player_skeleton.cpp	2016-04-06 15:28:29 +0000
+++ src/core/media/player_skeleton.cpp	2016-06-14 19:43:39 +0000
@@ -478,7 +478,7 @@
 
 const core::Property<media::Track::MetaData>& media::PlayerSkeleton::meta_data_for_current_track() const
 {
-    return *d->skeleton.properties.typed_meta_data_for_current_track;
+    return *d->skeleton.properties.meta_data_for_current_track;
 }
 
 const core::Property<media::Player::Volume>& media::PlayerSkeleton::volume() const
@@ -608,7 +608,7 @@
 
 core::Property<media::Track::MetaData>& media::PlayerSkeleton::meta_data_for_current_track()
 {
-    return *d->skeleton.properties.typed_meta_data_for_current_track;
+    return *d->skeleton.properties.meta_data_for_current_track;
 }
 
 core::Property<media::Player::PlaybackRate>& media::PlayerSkeleton::minimum_playback_rate()

=== modified file 'src/core/media/player_stub.cpp'
--- src/core/media/player_stub.cpp	2016-04-06 15:28:29 +0000
+++ src/core/media/player_stub.cpp	2016-06-14 19:43:39 +0000
@@ -70,7 +70,7 @@
                     object->get_property<mpris::Player::Properties::TypedLoopStatus>(),
                     object->get_property<mpris::Player::Properties::PlaybackRate>(),
                     object->get_property<mpris::Player::Properties::Shuffle>(),
-                    object->get_property<mpris::Player::Properties::TypedMetaData>(),
+                    object->get_property<mpris::Player::Properties::Metadata>(),
                     object->get_property<mpris::Player::Properties::Volume>(),
                     object->get_property<mpris::Player::Properties::Position>(),
                     object->get_property<mpris::Player::Properties::Duration>(),
@@ -118,7 +118,7 @@
         std::shared_ptr<core::dbus::Property<mpris::Player::Properties::TypedLoopStatus>> loop_status;
         std::shared_ptr<core::dbus::Property<mpris::Player::Properties::PlaybackRate>> playback_rate;
         std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Shuffle>> shuffle;
-        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::TypedMetaData>> meta_data_for_current_track;
+        std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Metadata>> meta_data_for_current_track;
         std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Volume>> volume;
         std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Position>> position;
         std::shared_ptr<core::dbus::Property<mpris::Player::Properties::Duration>> duration;

=== modified file 'src/core/media/service_skeleton.cpp'
--- src/core/media/service_skeleton.cpp	2016-05-17 17:51:16 +0000
+++ src/core/media/service_skeleton.cpp	2016-06-14 19:43:39 +0000
@@ -732,6 +732,21 @@
                 player.properties.can_go_next->set(can_go_next);
             });
 
+            // NOTE: the metadata first gets updated in the PlayerImplementation constructor which connects
+            // and reacts to the Engine track_meta_data changed signal. Setting the
+            // meta_data_for_current_track here makes sure that the signal uses the MPRIS object path
+            connections.meta_data_changed = player_sp->meta_data_for_current_track().changed().connect(
+                    [this](const media::Track::MetaData& metadata)
+            {
+                player.properties.meta_data_for_current_track->set(metadata);
+                mpris::Player::Dictionary dict; dict[mpris::Player::Properties::Metadata::name()]
+                        = dbus::types::Variant::encode(metadata);
+                player.signals.properties_changed->emit(std::make_tuple(
+                                dbus::traits::Service<mpris::Player>::interface_name(),
+                                dict,
+                                mpris::Player::Skeleton::the_empty_list_of_invalidated_properties()));
+            });
+
             // Sync property values between session and player mpris::Player instances
             // TODO Getters from media::Player actually return values from a
             // mpris::Player::Skeleton instance different from "player". Each of them use
@@ -748,43 +763,6 @@
             player.properties.can_pause->set(player_sp->can_pause().get());
             player.properties.can_go_previous->set(player_sp->can_go_previous().get());
             player.properties.can_go_next->set(player_sp->can_go_next().get());
-
-#if 0
-            // TODO cover_art_resolver() is not implemented yet
-            connections.meta_data_changed = cp->meta_data_for_current_track().changed().connect(
-                [this](const core::ubuntu::media::Track::MetaData& md)
-                {
-                    mpris::Player::Dictionary dict;
-
-                    bool has_title = md.count(xesam::Title::name) > 0;
-                    bool has_album_name = md.count(xesam::Album::name) > 0;
-                    bool has_artist_name = md.count(xesam::Artist::name) > 0;
-
-                    if (has_title)
-                        dict[xesam::Title::name] = dbus::types::Variant::encode(md.get(xesam::Title::name));
-                    if (has_album_name)
-                        dict[xesam::Album::name] = dbus::types::Variant::encode(md.get(xesam::Album::name));
-                    if (has_artist_name)
-                        dict[xesam::Artist::name] = dbus::types::Variant::encode(md.get(xesam::Artist::name));
-
-                    dict[mpris::metadata::ArtUrl::name] = dbus::types::Variant::encode(
-                        cover_art_resolver(
-                            has_title ? md.get(xesam::Title::name) : "",
-                            has_album_name ? md.get(xesam::Album::name) : "",
-                            has_artist_name ? md.get(xesam::Artist::name) : ""));
-
-                    mpris::Player::Dictionary wrap;
-                    wrap[mpris::Player::Properties::Metadata::name()] = dbus::types::Variant::encode(dict);
-
-                    player.signals.properties_changed->emit(
-                        std::make_tuple(
-                            dbus::traits::Service<
-                            mpris::Player::Properties::Metadata::Interface>
-                            ::interface_name(),
-                            wrap,
-                            std::vector<std::string>()));
-                });
-#endif
         }
 
         void reset_current_player()
@@ -807,6 +785,7 @@
             connections.can_pause_changed = the_empty_signal.connect([](){});
             connections.can_go_previous_changed = the_empty_signal.connect([](){});
             connections.can_go_next_changed = the_empty_signal.connect([](){});
+            connections.meta_data_changed = the_empty_signal.connect([](){});
         }
 
         bool is_current_player(media::Player::PlayerKey key)

=== modified file 'src/core/media/xesam.h'
--- src/core/media/xesam.h	2014-09-10 07:05:15 +0000
+++ src/core/media/xesam.h	2016-06-14 19:43:39 +0000
@@ -53,4 +53,12 @@
 DATUM(UserRating, xesam:userRating, double)
 }
 
+namespace tags
+{
+// Does the track contain album art?
+DATUM(Image, tag:image, bool)
+// Does the track contain a small album art preview image?
+DATUM(PreviewImage, tag::previewImage, bool)
+}
+
 #endif // XESAM_H_