Merge lp:~thomas-voss/media-hub/make-video-size-a-proper-type into lp:media-hub

[Thomas Voß]

Replace home-grown mask type for the video size with a std::tuple, i.e., media::video::Dimensions.
Introduce a simple TaggedInteger class to distinguish between Width, Height and other dimensions.
Adjust interfaces of media::Player to rely on the new type.
Adjust implementation classes to account for interface changes.
Adjust Codec implementation for sending the tagged integer via the bus.
Adjust gstreamer::Engine and gstreamer::Playbin to hand out the correct types.

Replace home-grown mask type for the video size with a std::tuple, i.e., media::video::Dimensions.
Introduce a simple TaggedInteger class to distinguish between Width, Height and other dimensions.
Adjust interfaces of media::Player to rely on the new type.
Adjust implementation classes to account for interface changes.
Adjust Codec implementation for sending the tagged integer via the bus.
Adjust gstreamer::Engine and gstreamer::Playbin to hand out the correct types.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:100
http://jenkins.qa.ubuntu.com/job/media-hub-ci/177/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/17/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/17/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/17/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/177/rebuild

review: Needs Fixing (continuous-integration)

[lp:~thomas-voss/media-hub/make-video-size-a-proper-type]

101. By Thomas Voß on 2014-11-26

Remerge prereq branch.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:101
http://jenkins.qa.ubuntu.com/job/media-hub-ci/192/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/32/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/32/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/32/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/192/rebuild

review: Needs Fixing (continuous-integration)

[Jim Hodapp (jhodapp)]

A few inline comments below. Please make sure to rebase with trunk.

review: Needs Fixing (code)

[lp:~thomas-voss/media-hub/make-video-size-a-proper-type]

102. By Thomas Voß on 2014-12-09

[ Jim Hodapp ]
* Resubmitting with prerequisite branch (LP: #1331041)
[ Justin McPherson ]
* Resubmitting with prerequisite branch (LP: #1331041)

103. By Thomas Voß on 2014-12-09

Address reviewer comments.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:103
http://jenkins.qa.ubuntu.com/job/media-hub-ci/202/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/42/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/42
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/42/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/42

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/202/rebuild

review: Needs Fixing (continuous-integration)

[lp:~thomas-voss/media-hub/make-video-size-a-proper-type]

104. By Thomas Voß on 2015-01-26

[ Jim Hodapp ]
* Error reporting all the way up to the app level from the playbin
  pipeline.
[ Ubuntu daily release ]
* New rebuild forced
[ Jim Hodapp ]
* Don't auto-resume playback of videos after a phone call ends. (LP:
  #1411273)
[ Ubuntu daily release ]
* New rebuild forced
[ Ricardo Salveti de Araujo ]
* service_implementation: adding debug for call started/ended signals.
  Make sure account and connection are available when setting up
  account manager (patch from Gustavo Boiko). call_monitor: don't
  check caps when hooking up on/off signals, until bug 1409125 is
  fixed. Enable parallel building . (LP: #1409125)
[ Jim Hodapp ]
* Pause playback when recording begins. (LP: #1398047)
[ Ricardo Salveti de Araujo ]
* call_monitor.cpp: waiting for bridge to be up, and also protecting
  the on_change call (LP: #1408137)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:104
http://jenkins.qa.ubuntu.com/job/media-hub-ci/230/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/70/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/70/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/70/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/230/rebuild

review: Needs Fixing (continuous-integration)

[lp:~thomas-voss/media-hub/make-video-size-a-proper-type]

105. By Thomas Voß on 2015-01-26

Merge prereq branch.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:105
http://jenkins.qa.ubuntu.com/job/media-hub-ci/239/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/79/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/79
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/79/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/79/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/239/rebuild

review: Needs Fixing (continuous-integration)

[Francis Ginther (fginther)]

The jenkins node for the i386 build failed, I've restarted a new ci run.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:105
http://jenkins.qa.ubuntu.com/job/media-hub-ci/249/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/89
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/89
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/89/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/89

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/249/rebuild

review: Approve (continuous-integration)

[lp:~thomas-voss/media-hub/make-video-size-a-proper-type]

106. By Thomas Voß on 2015-01-29

Merge prereq branch.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:106
http://jenkins.qa.ubuntu.com/job/media-hub-ci/260/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/100
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/100
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/100/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/100

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/260/rebuild

review: Approve (continuous-integration)

[lp:~thomas-voss/media-hub/make-video-size-a-proper-type]

107. By Thomas Voß on 2015-03-12

* debian/control:
  - Removing pre-depends that are not required
  - Bumping standards-version to 3.9.6
[ Ricardo Salveti de Araujo ]
* Migrating tests to use ogg instead of mp3/avi removed:
  tests/h264.avi tests/test.mp3 added: tests/test-audio-1.ogg
  tests/test-video.ogg tests/test.mp3 renamed: tests/test.ogg =>
  tests/test-audio.ogg

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:107
http://jenkins.qa.ubuntu.com/job/media-hub-ci/315/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/155
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/155
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/155/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/155

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/315/rebuild

review: Approve (continuous-integration)

=== modified file 'include/core/media/player.h'
--- include/core/media/player.h	2015-03-12 11:39:32 +0000
+++ include/core/media/player.h	2015-03-12 11:39:32 +0000
@@ -20,6 +20,8 @@
 #define CORE_UBUNTU_MEDIA_PLAYER_H_
 
 #include <core/media/track.h>
+
+#include <core/media/video/dimensions.h>
 #include <core/media/video/sink.h>
 
 #include <core/property.h>
@@ -168,14 +170,9 @@
     virtual const core::Signal<int64_t>& seeked_to() const = 0;
     virtual const core::Signal<void>& end_of_stream() const = 0;
     virtual core::Signal<PlaybackStatus>& playback_status_changed() = 0;
-    /**
-     * Called when the video height/width change. Passes height and width as a bitmask with
-     * height in the upper 32 bits and width in the lower 32 bits (both unsigned values)
-     */
-    virtual const core::Signal<uint64_t>& video_dimension_changed() const = 0;
+    virtual const core::Signal<video::Dimensions>& video_dimension_changed() const = 0;
     /** Signals all errors and warnings (typically from GStreamer and below) */
     virtual const core::Signal<Error>& error() const = 0;
-
   protected:
     Player();
 
 
=== added file 'include/core/media/video/dimensions.h'
--- include/core/media/video/dimensions.h	1970-01-01 00:00:00 +0000
+++ include/core/media/video/dimensions.h	2015-03-12 11:39:32 +0000
@@ -0,0 +1,145 @@
+/*
+ * 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 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: Thomas Voß <[e-mail]>
+ */
+#ifndef CORE_UBUNTU_MEDIA_VIDEO_DIMENSIONS_H_
+#define CORE_UBUNTU_MEDIA_VIDEO_DIMENSIONS_H_
+
+#include <cstdint>
+
+#include <tuple>
+
+namespace core
+{
+namespace ubuntu
+{
+namespace media
+{
+namespace video
+{
+namespace detail
+{
+enum class DimensionTag { width, height };
+
+/**
+ * @brief IntWrapper is a type-safe integer that allows for encoding/enforcing semantics by means of tags.
+ * @tparam Tag Hint for the compiler on the semantics of the underlying integer.
+ * @tparam IntegerType The underlying integer type.
+ *
+ * Handling dimensions like width and height with raw integer values is tedious and error prone
+ * as the compiler has no way of distinguishing a width from a height, or an x coordinate from a
+ * y coordinate. The problem is solvable with the tagged integer type presented here. Consider
+ * the following example:
+ *
+ * @code{.cpp}
+ * typedef IntWrapper<DimensionTag::width, std::uint32_t> Width;
+ * typedef IntWrapper<DimensionTag::height, std::uint32_t> Height;
+ *
+ * void an_unsafe_function_expecting_width_and_height(std::uint32_t width, std::uint32_t height);
+ * void a_safe_function_expecting_width_and_height(Width width, Height height);
+ *
+ * int main()
+ * {
+ *     std::uint unsafe_width{640}, unsafe_height{480};
+ *     Width width{640}; Height height{480};
+ *     // This will compile but is still wrong
+ *     an_unsafe_function_expecting_width_and_height(unsafe_height, unsafe_width);
+ *
+ *    // This will not compile
+ *    a_safe_function_expecting_width_and_height(height, width);
+ *
+ * }
+ * @endcode
+ */
+template<DimensionTag Tag, typename IntegerType>
+class IntWrapper
+{
+public:
+    static_assert(std::is_integral<IntegerType>::value, "IntWrapper<> only supports integral types.");
+    typedef IntegerType ValueType;
+
+    IntWrapper() : value{0} {}
+    template<typename AnyInteger>
+    explicit IntWrapper(AnyInteger value) : value{static_cast<ValueType>(value)} {}
+
+    template<typename T = IntegerType>
+    T as() const
+    {
+        static_assert(std::is_arithmetic<T>::value, "as() only supports arithmetic types.");
+        return static_cast<T>(value);
+    }
+
+private:
+    ValueType value;
+};
+
+template<DimensionTag Tag, typename IntegerType>
+std::ostream& operator<<(std::ostream& out, IntWrapper<Tag, IntegerType> const& value)
+{
+    out << value.template as<>();
+    return out;
+}
+
+template<DimensionTag Tag, typename IntegerType>
+inline bool operator == (IntWrapper<Tag, IntegerType> const& lhs, IntWrapper<Tag, IntegerType> const& rhs)
+{
+    return lhs.template as<>() == rhs.template as<>();
+}
+
+template<DimensionTag Tag, typename IntegerType>
+inline bool operator != (IntWrapper<Tag, IntegerType> const& lhs, IntWrapper<Tag, IntegerType> const& rhs)
+{
+    return lhs.template as<>() != rhs.template as<>();
+}
+
+template<DimensionTag Tag, typename IntegerType>
+inline bool operator <= (IntWrapper<Tag, IntegerType> const& lhs, IntWrapper<Tag, IntegerType> const& rhs)
+{
+    return lhs.template as<>() <= rhs.template as<>();
+}
+
+template<DimensionTag Tag, typename IntegerType>
+inline bool operator >= (IntWrapper<Tag, IntegerType> const& lhs, IntWrapper<Tag, IntegerType> const& rhs)
+{
+    return lhs.template as<>() >= rhs.template as<>();
+}
+
+template<DimensionTag Tag, typename IntegerType>
+inline bool operator < (IntWrapper<Tag, IntegerType> const& lhs, IntWrapper<Tag, IntegerType> const& rhs)
+{
+    return lhs.template as<>() < rhs.template as<>();
+}
+
+template<DimensionTag Tag, typename IntegerType>
+inline bool operator > (IntWrapper<Tag, IntegerType> const& lhs, IntWrapper<Tag, IntegerType> const& rhs)
+{
+    return lhs.template as<>() > rhs.template as<>();
+}
+} // namespace detail
+
+/** @brief The integer Height of a video. */
+typedef detail::IntWrapper<detail::DimensionTag::height, std::uint32_t> Height;
+/** @brief The integer Width of a video. */
+typedef detail::IntWrapper<detail::DimensionTag::width, std::uint32_t> Width;
+
+/** @brief Height and Width of a video. */
+typedef std::tuple<Height, Width> Dimensions;
+}
+}
+}
+}
+
+#endif // CORE_UBUNTU_MEDIA_VIDEO_DIMENSIONS_H_
 
=== modified file 'src/core/media/codec.h'
--- src/core/media/codec.h	2015-01-13 14:18:59 +0000
+++ src/core/media/codec.h	2015-03-12 11:39:32 +0000
@@ -232,6 +232,31 @@
 
 namespace helper
 {
+template<core::ubuntu::media::video::detail::DimensionTag tag, typename IntegerType>
+struct TypeMapper<core::ubuntu::media::video::detail::IntWrapper<tag, IntegerType>>
+{
+    constexpr static ArgumentType type_value()
+    {
+        return core::dbus::ArgumentType::uint32;
+    }
+
+    constexpr static bool is_basic_type()
+    {
+        return true;
+    }
+
+    constexpr static bool requires_signature()
+    {
+        return false;
+    }
+
+    static std::string signature()
+    {
+        static const std::string s = TypeMapper<std::uint32_t>::signature();
+        return s;
+    }
+};
+
 template<>
 struct TypeMapper<core::ubuntu::media::Player::Lifetime>
 {
@@ -239,10 +264,12 @@
     {
         return core::dbus::ArgumentType::int16;
     }
+
     constexpr static bool is_basic_type()
     {
-        return false;
+        return true;
     }
+
     constexpr static bool requires_signature()
     {
         return false;
@@ -256,14 +283,30 @@
 };
 }
 
+template<core::ubuntu::media::video::detail::DimensionTag tag, typename IntegerType>
+struct Codec<core::ubuntu::media::video::detail::IntWrapper<tag, IntegerType>>
+{
+    static void encode_argument(core::dbus::Message::Writer& out, const core::ubuntu::media::video::detail::IntWrapper<tag, IntegerType>& in)
+    {
+        out.push_uint32(in.template as<std::uint32_t>());
+    }
+
+    static void decode_argument(core::dbus::Message::Reader& out, core::ubuntu::media::video::detail::IntWrapper<tag, IntegerType>& in)
+    {
+        in = core::ubuntu::media::video::detail::IntWrapper<tag, IntegerType>{out.pop_uint32()};
+    }
+};
+
 template<>
 struct Codec<core::ubuntu::media::Player::Lifetime>
 {
+
     static void encode_argument(core::dbus::Message::Writer& out, const core::ubuntu::media::Player::Lifetime& in)
     {
         out.push_int16(static_cast<std::int16_t>(in));
     }
 
+
     static void decode_argument(core::dbus::Message::Reader& out, core::ubuntu::media::Player::Lifetime& in)
     {
         in = static_cast<core::ubuntu::media::Player::Lifetime>(out.pop_int16());
 
=== modified file 'src/core/media/engine.h'
--- src/core/media/engine.h	2015-03-12 11:39:32 +0000
+++ src/core/media/engine.h	2015-03-12 11:39:32 +0000
@@ -110,7 +110,7 @@
     virtual const core::Signal<void>& client_disconnected_signal() const = 0;
     virtual const core::Signal<void>& end_of_stream_signal() const = 0;
     virtual const core::Signal<core::ubuntu::media::Player::PlaybackStatus>& playback_status_changed_signal() const = 0;
-    virtual const core::Signal<uint32_t, uint32_t>& video_dimension_changed_signal() const = 0;
+    virtual const core::Signal<video::Dimensions>& video_dimension_changed_signal() const = 0;
     virtual const core::Signal<core::ubuntu::media::Player::Error>& error_signal() const = 0;
 
     virtual void reset() = 0;
 
=== modified file 'src/core/media/gstreamer/engine.cpp'
--- src/core/media/gstreamer/engine.cpp	2015-01-19 21:48:01 +0000
+++ src/core/media/gstreamer/engine.cpp	2015-03-12 11:39:32 +0000
@@ -175,9 +175,9 @@
         end_of_stream();
     }
 
-    void on_video_dimension_changed(uint32_t height, uint32_t width)
+    void on_video_dimension_changed(const media::video::Dimensions& dimensions)
     {
-        video_dimension_changed(height, width);
+        video_dimension_changed(dimensions);
     }
 
     Private()
@@ -262,12 +262,11 @@
                       &Private::on_end_of_stream,
                       this))),
           on_video_dimension_changed_connection(
-              playbin.signals.on_add_frame_dimension.connect(
+              playbin.signals.on_video_dimensions_changed.connect(
                   std::bind(
                       &Private::on_video_dimension_changed,
                       this,
-                      std::placeholders::_1,
-                      std::placeholders::_2)))
+                      std::placeholders::_1)))
     {
     }
 
@@ -307,7 +306,7 @@
     core::Signal<void> client_disconnected;
     core::Signal<void> end_of_stream;
     core::Signal<media::Player::PlaybackStatus> playback_status_changed;
-    core::Signal<uint32_t, uint32_t> video_dimension_changed;
+    core::Signal<core::ubuntu::media::video::Dimensions> video_dimension_changed;
     core::Signal<media::Player::Error> error;
 };
 
@@ -500,7 +499,7 @@
     return d->playback_status_changed;
 }
 
-const core::Signal<uint32_t, uint32_t>& gstreamer::Engine::video_dimension_changed_signal() const
+const core::Signal<core::ubuntu::media::video::Dimensions>& gstreamer::Engine::video_dimension_changed_signal() const
 {
     return d->video_dimension_changed;
 }
 
=== modified file 'src/core/media/gstreamer/engine.h'
--- src/core/media/gstreamer/engine.h	2015-01-13 14:18:59 +0000
+++ src/core/media/gstreamer/engine.h	2015-03-12 11:39:32 +0000
@@ -66,7 +66,7 @@
     const core::Signal<void>& client_disconnected_signal() const;
     const core::Signal<void>& end_of_stream_signal() const;
     const core::Signal<core::ubuntu::media::Player::PlaybackStatus>& playback_status_changed_signal() const;
-    const core::Signal<uint32_t, uint32_t>& video_dimension_changed_signal() const;
+    const core::Signal<core::ubuntu::media::video::Dimensions>& video_dimension_changed_signal() const;
     const core::Signal<core::ubuntu::media::Player::Error>& error_signal() const;
 
     void reset();
 
=== modified file 'src/core/media/gstreamer/playbin.h'
--- src/core/media/gstreamer/playbin.h	2014-11-11 23:59:07 +0000
+++ src/core/media/gstreamer/playbin.h	2015-03-12 11:39:32 +0000
@@ -85,8 +85,6 @@
           bus{gst_element_get_bus(pipeline)},
           file_type(MEDIA_FILE_TYPE_NONE),
           video_sink(nullptr),
-          video_height(0),
-          video_width(0),
           on_new_message_connection(
               bus.on_new_message.connect(
                   std::bind(
@@ -425,19 +423,22 @@
                         pipeline,
                         &current,
                         &pending,
-                        state_change_timeout.count());
+                        state_change_timeout.count());        
+            break;
+        }
 
-        if (new_state == GST_STATE_PLAYING)
+        // The state change has to have been successful to make
+        // sure that we indeed reached the requested new state.
+        if (result && new_state == GST_STATE_PLAYING)
         {
             // Get the video height/width from the video sink
-            get_video_dimensions();
+            if (has_video_sink_with_height_and_width())
+                signals.on_video_dimensions_changed(get_video_dimensions());
 #ifdef DEBUG_GST_PIPELINE
             std::cout << "Dumping pipeline dot file" << std::endl;
             GST_DEBUG_BIN_TO_DOT_FILE((GstBin*)pipeline, GST_DEBUG_GRAPH_SHOW_ALL, "pipeline");
 #endif
         }
-            break;
-        }
 
         return result;
     }
@@ -452,27 +453,26 @@
                     ms.count() * 1000);
     }
 
-    void get_video_dimensions()
-    {
-        if (video_sink != nullptr && g_strcmp0(::getenv("CORE_UBUNTU_MEDIA_SERVICE_VIDEO_SINK_NAME"), "mirsink") == 0)
+    bool has_video_sink_with_height_and_width()
+    {
+        return video_sink != nullptr && g_strcmp0(::getenv("CORE_UBUNTU_MEDIA_SERVICE_VIDEO_SINK_NAME"), "mirsink") == 0;
+    }
+
+    core::ubuntu::media::video::Dimensions get_video_dimensions()
+    {
+        if (not has_video_sink_with_height_and_width())
+            throw std::runtime_error{"Could not get the height/width of each video frame"};
+
+        uint32_t video_height = 0, video_width = 0;
+        g_object_get (video_sink, "height", &video_height, nullptr);
+        g_object_get (video_sink, "width", &video_width, nullptr);
+        std::cout << "video_height: " << video_height << ", video_width: " << video_width << std::endl;
+
+        return core::ubuntu::media::video::Dimensions
         {
-            g_object_get (video_sink, "height", &video_height, nullptr);
-            g_object_get (video_sink, "width", &video_width, nullptr);
-            std::cout << "video_height: " << video_height << ", video_width: " << video_width << std::endl;
-            signals.on_add_frame_dimension(video_height, video_width);
-        }
-        else
-            std::cerr << "Could not get the height/width of each video frame" << std::endl;
-    }
-
-    int get_video_height() const
-    {
-        return video_height;
-    }
-
-    int get_video_width() const
-    {
-        return video_width;
+            core::ubuntu::media::video::Height{video_height},
+            core::ubuntu::media::video::Width{video_width}
+        };
     }
 
     std::string get_file_content_type(const std::string& uri) const
@@ -555,8 +555,6 @@
     MediaFileType file_type;
     SurfaceTextureClientHybris stc_hybris;
     GstElement* video_sink;
-    uint32_t video_height;
-    uint32_t video_width;
     core::Connection on_new_message_connection;
     bool is_seeking;
     core::ubuntu::media::Player::HeadersType request_headers;
@@ -571,9 +569,9 @@
         core::Signal<Bus::Message::Detail::StateChanged> on_state_changed;
         core::Signal<uint64_t> on_seeked_to;
         core::Signal<void> on_end_of_stream;
-        core::Signal<media::Player::PlaybackStatus> on_playback_status_changed;
-        core::Signal<media::Player::Orientation> on_orientation_changed;
-        core::Signal<uint32_t, uint32_t> on_add_frame_dimension;
+        core::Signal<core::ubuntu::media::Player::PlaybackStatus> on_playback_status_changed;
+        core::Signal<core::ubuntu::media::Player::Orientation> on_orientation_changed;
+        core::Signal<core::ubuntu::media::video::Dimensions> on_video_dimensions_changed;
         core::Signal<void> client_disconnected;
     } signals;
 };
 
=== modified file 'src/core/media/mpris/player.h'
--- src/core/media/mpris/player.h	2015-03-12 11:39:32 +0000
+++ src/core/media/mpris/player.h	2015-03-12 11:39:32 +0000
@@ -34,12 +34,16 @@
 #include <core/dbus/types/object_path.h>
 #include <core/dbus/types/variant.h>
 
+#include <core/dbus/types/stl/tuple.h>
+
 #include <boost/utility/identity_type.hpp>
 
 #include <string>
 #include <tuple>
 #include <vector>
 
+#include <cstdint>
+
 namespace dbus = core::dbus;
 
 namespace mpris
@@ -134,7 +138,7 @@
         DBUS_CPP_SIGNAL_DEF(Seeked, Player, std::int64_t)
         DBUS_CPP_SIGNAL_DEF(EndOfStream, Player, void)
         DBUS_CPP_SIGNAL_DEF(PlaybackStatusChanged, Player, core::ubuntu::media::Player::PlaybackStatus)
-        DBUS_CPP_SIGNAL_DEF(VideoDimensionChanged, Player, std::uint64_t)
+        DBUS_CPP_SIGNAL_DEF(VideoDimensionChanged, Player, core::ubuntu::media::video::Dimensions)
         DBUS_CPP_SIGNAL_DEF(Error, Player, core::ubuntu::media::Player::Error)
     };
 
 
=== modified file 'src/core/media/player_implementation.cpp'
--- src/core/media/player_implementation.cpp	2015-03-12 11:39:32 +0000
+++ src/core/media/player_implementation.cpp	2015-03-12 11:39:32 +0000
@@ -415,12 +415,9 @@
         playback_status_changed()(status);
     });
 
-    d->engine->video_dimension_changed_signal().connect([this](uint32_t height, uint32_t width)
+    d->engine->video_dimension_changed_signal().connect([this](const media::video::Dimensions& dimensions)
     {
-        uint64_t mask = 0;
-        // Left most 32 bits are for height, right most 32 bits are for width
-        mask = (static_cast<uint64_t>(height) << 32) | static_cast<uint64_t>(width);
-        video_dimension_changed()(mask);
+        video_dimension_changed()(dimensions);
     });
 
     d->engine->error_signal().connect([this](const Player::Error& e)
 
=== modified file 'src/core/media/player_skeleton.cpp'
--- src/core/media/player_skeleton.cpp	2015-03-12 11:39:32 +0000
+++ src/core/media/player_skeleton.cpp	2015-03-12 11:39:32 +0000
@@ -19,6 +19,7 @@
 
 #include "apparmor.h"
 #include "codec.h"
+#include "engine.h"
 #include "player_skeleton.h"
 #include "player_traits.h"
 #include "property_stub.h"
@@ -338,9 +339,9 @@
                 remote_playback_status_changed->emit(status);
             });
 
-            video_dimension_changed.connect([remote_video_dimension_changed](uint64_t mask)
+            video_dimension_changed.connect([remote_video_dimension_changed](const media::video::Dimensions& dimensions)
             {
-                remote_video_dimension_changed->emit(mask);
+                remote_video_dimension_changed->emit(dimensions);
             });
 
             error.connect([remote_error](const media::Player::Error& e)
@@ -352,7 +353,7 @@
         core::Signal<int64_t> seeked_to;
         core::Signal<void> end_of_stream;
         core::Signal<media::Player::PlaybackStatus> playback_status_changed;
-        core::Signal<uint64_t> video_dimension_changed;
+        core::Signal<media::video::Dimensions> video_dimension_changed;
         core::Signal<media::Player::Error> error;
     } signals;
 
@@ -649,12 +650,12 @@
     return d->signals.playback_status_changed;
 }
 
-const core::Signal<uint64_t>& media::PlayerSkeleton::video_dimension_changed() const
+const core::Signal<media::video::Dimensions>& media::PlayerSkeleton::video_dimension_changed() const
 {
     return d->signals.video_dimension_changed;
 }
 
-core::Signal<uint64_t>& media::PlayerSkeleton::video_dimension_changed()
+core::Signal<media::video::Dimensions>& media::PlayerSkeleton::video_dimension_changed()
 {
     return d->signals.video_dimension_changed;
 }
 
=== modified file 'src/core/media/player_skeleton.h'
--- src/core/media/player_skeleton.h	2015-01-13 14:18:59 +0000
+++ src/core/media/player_skeleton.h	2015-03-12 11:39:32 +0000
@@ -75,7 +75,7 @@
     virtual const core::Signal<int64_t>& seeked_to() const;
     virtual const core::Signal<void>& end_of_stream() const;
     virtual core::Signal<PlaybackStatus>& playback_status_changed();
-    virtual const core::Signal<uint64_t>& video_dimension_changed() const;
+    virtual const core::Signal<video::Dimensions>& video_dimension_changed() const;
     virtual const core::Signal<Error>& error() const;
 
 protected:
@@ -112,7 +112,7 @@
 
     virtual core::Signal<int64_t>& seeked_to();
     virtual core::Signal<void>& end_of_stream();
-    virtual core::Signal<uint64_t>& video_dimension_changed();
+    virtual core::Signal<video::Dimensions>& video_dimension_changed();
     virtual core::Signal<Error>& error();
 
   private:
 
=== modified file 'src/core/media/player_stub.cpp'
--- src/core/media/player_stub.cpp	2015-03-12 11:39:32 +0000
+++ src/core/media/player_stub.cpp	2015-03-12 11:39:32 +0000
@@ -168,10 +168,10 @@
                 playback_status_changed(status);
             });
 
-            dbus.video_dimension_changed->connect([this](uint64_t mask)
+            dbus.video_dimension_changed->connect([this](const media::video::Dimensions dimensions)
             {
                 std::cout << "VideoDimensionChanged signal arrived via the bus." << std::endl;
-                video_dimension_changed(mask);
+                video_dimension_changed(dimensions);
             });
 
             dbus.error->connect([this](const media::Player::Error& e)
@@ -184,7 +184,7 @@
         core::Signal<int64_t> seeked_to;
         core::Signal<void> end_of_stream;
         core::Signal<media::Player::PlaybackStatus> playback_status_changed;
-        core::Signal<uint64_t> video_dimension_changed;
+        core::Signal<media::video::Dimensions> video_dimension_changed;
         core::Signal<media::Player::Error> error;
 
         struct DBus
@@ -448,7 +448,7 @@
     return d->signals.playback_status_changed;
 }
 
-const core::Signal<uint64_t>& media::PlayerStub::video_dimension_changed() const
+const core::Signal<media::video::Dimensions>& media::PlayerStub::video_dimension_changed() const
 {
     return d->signals.video_dimension_changed;
 }
 
=== modified file 'src/core/media/player_stub.h'
--- src/core/media/player_stub.h	2015-03-12 11:39:32 +0000
+++ src/core/media/player_stub.h	2015-03-12 11:39:32 +0000
@@ -88,7 +88,7 @@
     virtual const core::Signal<int64_t>& seeked_to() const;
     virtual const core::Signal<void>& end_of_stream() const;
     virtual core::Signal<PlaybackStatus>& playback_status_changed();
-    virtual const core::Signal<uint64_t>& video_dimension_changed() const;
+    virtual const core::Signal<video::Dimensions>& video_dimension_changed() const;
     virtual const core::Signal<Error>& error() const;
 
   private: