[Merge] lp:~thomas-voss/media-hub/refactor-video-sink-interface-to-not-rely-on-hybris-types into lp:media-hub
Jim Hodapp
jim.hodapp at canonical.com
Fri Nov 14 20:08:25 UTC 2014
Review: Needs Fixing code
Looking good so far. I'd like to see unit tests for HybrisGlSink and a lot of the functionality surrounding it. See my comments inline below.
Diff comments:
> === modified file 'CMakeLists.txt'
> --- CMakeLists.txt 2014-11-03 00:56:55 +0000
> +++ CMakeLists.txt 2014-11-13 09:06:33 +0000
> @@ -2,7 +2,7 @@
>
> project(ubuntu-media-hub)
>
> -set(UBUNTU_MEDIA_HUB_VERSION_MAJOR 2)
> +set(UBUNTU_MEDIA_HUB_VERSION_MAJOR 3)
> set(UBUNTU_MEDIA_HUB_VERSION_MINOR 0)
> set(UBUNTU_MEDIA_HUB_VERSION_PATCH 0)
>
> @@ -27,7 +27,15 @@
> pkg_check_modules(PROCESS_CPP process-cpp REQUIRED)
> pkg_check_modules(PROPERTIES_CPP properties-cpp REQUIRED)
> pkg_check_modules(GIO gio-2.0 REQUIRED)
> -pkg_check_modules(HYBRIS_MEDIA libmedia REQUIRED)
> +pkg_check_modules(HYBRIS_MEDIA libmedia)
> +
> +# We do not require the hybris media layer anymore and instead fallback
> +# to a dummy implementation for now. Later on, we will have to tune
> +# core::ubuntu::media::video::make_platform_default_sink(...) to account for
> +# things like libva.
> +if (HYBRIS_MEDIA_FOUND)
> + add_definitions(-DMEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER)
> +endif (HYBRIS_MEDIA_FOUND)
>
> #####################################################################
> # Enable code coverage calculation with gcov/gcovr/lcov
>
> === modified file 'debian/changelog'
> --- debian/changelog 2014-11-11 20:18:50 +0000
> +++ debian/changelog 2014-11-13 09:06:33 +0000
> @@ -1,3 +1,9 @@
> +media-hub (3.0.0) UNRELEASED; urgency=medium
> +
> + * Refactor client-facing interfaces to pull out explicit dependency on hybris-based media layer.
> +
> + -- Thomas Voss <thomas.voss at canonical.com> Tue, 11 Nov 2014 17:15:52 +0100
> +
> media-hub (2.0.0+15.04.20141111-0ubuntu1) vivid; urgency=low
>
> [ Ubuntu daily release ]
>
> === modified file 'debian/control'
> --- debian/control 2014-11-11 20:18:36 +0000
> +++ debian/control 2014-11-13 09:06:33 +0000
> @@ -22,7 +22,7 @@
> libdbus-1-dev,
> libdbus-cpp-dev (>= 4.3.0),
> libgoogle-glog-dev,
> - libhybris-dev (>=0.1.0+git20131207+e452e83-0ubuntu30),
> + libhybris-dev (>=0.1.0+git20131207+e452e83-0ubuntu30) [!arm64 !ppc64el !powerpc],
> libprocess-cpp-dev,
> libproperties-cpp-dev,
> gstreamer1.0-libav,
>
> === modified file 'include/core/media/player.h'
> --- include/core/media/player.h 2014-10-14 20:05:20 +0000
> +++ include/core/media/player.h 2014-11-13 09:06:33 +0000
> @@ -21,6 +21,9 @@
>
> #include <core/media/track.h>
>
> +#include <core/media/video/dimensions.h>
> +#include <core/media/video/sink.h>
> +
> #include <core/property.h>
>
> #include <chrono>
> @@ -43,10 +46,15 @@
> typedef uint32_t PlayerKey;
> typedef void* GLConsumerWrapperHybris;
>
> - /** Used to set a callback function to be called when a frame is ready to be rendered **/
> - typedef void (*FrameAvailableCbHybris)(GLConsumerWrapperHybris wrapper, void *context);
> - typedef void (*FrameAvailableCb)(void *context);
> - typedef void (*PlaybackCompleteCb)(void *context);
> + struct Error
> + {
> + Error() = delete;
> +
> + struct OutOfProcessBufferStreamingNotSupported : public std::runtime_error
> + {
> + OutOfProcessBufferStreamingNotSupported();
> + };
> + };
>
> struct Configuration;
>
> @@ -103,9 +111,9 @@
> virtual std::shared_ptr<TrackList> track_list() = 0;
> virtual PlayerKey key() const = 0;
>
> + virtual video::Sink::Ptr create_gl_texture_video_sink(std::uint32_t texture_id) = 0;
> +
> virtual bool open_uri(const Track::UriType& uri) = 0;
> - virtual void create_video_sink(uint32_t texture_id) = 0;
> - virtual GLConsumerWrapperHybris gl_consumer() const = 0;
> virtual void next() = 0;
> virtual void previous() = 0;
> virtual void play() = 0;
> @@ -113,10 +121,6 @@
> virtual void stop() = 0;
> virtual void seek_to(const std::chrono::microseconds& offset) = 0;
>
> - // TODO: Convert this to be a signal
> - virtual void set_frame_available_callback(FrameAvailableCb cb, void *context) = 0;
> - virtual void set_playback_complete_callback(PlaybackCompleteCb cb, void *context) = 0;
> -
> virtual const core::Property<bool>& can_play() const = 0;
> virtual const core::Property<bool>& can_pause() const = 0;
> virtual const core::Property<bool>& can_seek() const = 0;
> @@ -146,11 +150,7 @@
> 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;
> protected:
> Player();
>
>
> === added directory 'include/core/media/video'
> === 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 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,112 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +#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 };
> +
> +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
> +
> +typedef detail::IntWrapper<detail::DimensionTag::height, std::uint32_t> Height;
> +typedef detail::IntWrapper<detail::DimensionTag::width, std::uint32_t> Width;
> +
> +typedef std::tuple<Height, Width> Dimensions;
> +}
> +}
> +}
> +}
> +
> +#endif // CORE_UBUNTU_MEDIA_VIDEO_DIMENSIONS_H_
>
> === added file 'include/core/media/video/sink.h'
> --- include/core/media/video/sink.h 1970-01-01 00:00:00 +0000
> +++ include/core/media/video/sink.h 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,128 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +#ifndef CORE_UBUNTU_MEDIA_VIDEO_SINK_H_
> +#define CORE_UBUNTU_MEDIA_VIDEO_SINK_H_
> +
> +#include <core/signal.h>
> +
> +#include <memory>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +namespace video
> +{
> +// A transformation matrix, providing a view on raw float values.
> +template<std::size_t rows, std::size_t columns>
> +class MatrixView
> +{
> +public:
> + // Constructs a new matrix instance for the given
> + // raw float array with at least 3*3 members.
> + inline constexpr explicit MatrixView(float* raw)
> + : begin{raw},
> + end{begin + height() * width()}
> + {
> + }
> +
> + // We disallow all copy operations.
> + MatrixView(const MatrixView&) = delete;
> + MatrixView& operator=(const MatrixView&) = delete;
> +
> + inline constexpr std::size_t height() const
> + {
> + return rows;
> + }
> +
> + inline constexpr std::size_t width() const
> + {
> + return columns;
> + }
> +
> +
> + inline float& operator()(std::size_t row, std::size_t col)
> + {
> + auto offset = row * width() + col;
> + if ((begin + offset) >= end) throw std::out_of_range
> + {
> + "[" + std::to_string(row) + "," + std::to_string(col) + "] exceeds array boundaries"
> + };
> + return begin[offset];
> + }
> +
> + inline constexpr const float& operator()(std::size_t row, std::size_t col) const
> + {
> + auto offset = row * width() + col;
> + if ((begin + offset) >= end) throw std::out_of_range
> + {
> + "[" + std::to_string(row) + "," + std::to_string(col) + "] exceeds array boundaries"
> + };
> + return begin[row * width() + col];
> + }
> +
> + // Provides access to the underlying, raw float pointer.
> + inline constexpr float* to_float() const
> + {
> + return begin;
> + }
> +
> +private:
> + // The raw floating point values in a linear array.
> + float* begin;
> + float* end;
> +};
> +
> +// A video sink abstracts a queue of buffers, that receives
> +// a stream of decoded video buffers from an arbitrary source.
> +struct Sink
> +{
> + // @brief To safe us some typing.
> + typedef std::shared_ptr<Sink> Ptr;
> +
> + /** @cond */
> + Sink() = default;
> + Sink(const Sink&) = delete;
> + virtual ~Sink() = default;
> +
> + Sink& operator=(const Sink&) = delete;
> + /** @endcond */
> +
> + // The signal is emitted whenever a new frame is available and a subsequent
> + // call to swap_buffers will not block and return true.
> + virtual const core::Signal<void>& frame_available() const = 0;
> +
> + // Queries the transformation matrix for the current frame, placing the data into 'matrix'
> + // and returns true or returns false and leaves 'matrix' unchanged in case
> + // of issues.
> + virtual bool transformation_matrix(MatrixView<4,4>& matrix) const = 0;
> +
> + // Releases the current buffer, and consumes the next buffer in the queue,
> + // making it available for consumption by consumers of this API in an
> + // implementation-specific way. Clients will usually rely on a GL texture
> + // to receive the latest buffer.
> + virtual bool swap_buffers() const = 0;
> +};
> +}
> +}
> +}
> +}
> +
> +#endif // CORE_UBUNTU_MEDIA_VIDEO_SINK_H_
>
> === modified file 'src/core/media/CMakeLists.txt'
> --- src/core/media/CMakeLists.txt 2014-11-03 00:56:55 +0000
> +++ src/core/media/CMakeLists.txt 2014-11-13 09:06:33 +0000
> @@ -5,7 +5,10 @@
> # out all symbols that are not in core::ubuntu::media*
> set(symbol_map "${CMAKE_SOURCE_DIR}/symbols.map")
>
> -file(GLOB MPRIS_HEADERS mpris/ *.h)
> +file(GLOB_RECURSE MEDIA_HUB_HEADERS ${CMAKE_SOURCE_DIR}/include/*.h)
> +file(GLOB MPRIS_HEADERS mpris/*.h)
> +
> +message(STATUS ${MEDIA_HUB_HEADERS})
>
> add_subdirectory(call-monitor)
>
> @@ -35,6 +38,8 @@
> add_library(
> media-hub-client SHARED
>
> + ${MEDIA_HUB_HEADERS}
> +
> player.cpp
> service.cpp
> track.cpp
> @@ -43,6 +48,9 @@
> player_stub.cpp
> service_stub.cpp
> track_list_stub.cpp
> +
> + video/hybris_gl_sink.cpp
> + video/platform_default_sink.cpp
> )
>
> set_target_properties(
> @@ -74,11 +82,17 @@
> add_library(
> media-hub-service
>
> + ${MEDIA_HUB_HEADERS}
> ${MPRIS_HEADERS}
>
> + client_death_observer.cpp
> + hybris_client_death_observer.cpp
> cover_art_resolver.cpp
> engine.cpp
> + recorder_observer.cpp
> + hybris_recorder_observer.cpp
> gstreamer/engine.cpp
> + gstreamer/playbin.cpp
>
> player_skeleton.cpp
> player_implementation.cpp
>
> === added file 'src/core/media/client_death_observer.cpp'
> --- src/core/media/client_death_observer.cpp 1970-01-01 00:00:00 +0000
> +++ src/core/media/client_death_observer.cpp 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,43 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +
> +#include <core/media/client_death_observer.h>
> +
> +namespace media = core::ubuntu::media;
> +
> +#if defined(MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER)
> +#include <core/media/hybris_client_death_observer.h>
> +
> +// Accesses the default client death observer implementation for the platform.
> +media::ClientDeathObserver::Ptr media::platform_default_client_death_observer()
> +{
> + static media::ClientDeathObserver::Ptr instance{media::HybrisClientDeathObserver::create()};
Why is this implemented with a global static instance? It should live in the Private class of the ServiceSkeleton on the stack (or even the heap). This will make lifecycle and memory management more robust and maintainable.
> + return instance;
> +}
> +#else // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +// Just throws a std::logic_error as we have not yet defined a default way to
> +// identify client death changes. One possible way of implementing the interface
> +// would be to listen to dbus name changes and react accordingly.
> +media::ClientDeathObserver::Ptr media::platform_default_client_death_observer()
> +{
> + throw std::logic_error
> + {
> + "No platform-specific death observer implementation known."
> + };
> +}
> +#endif // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
>
> === added file 'src/core/media/client_death_observer.h'
> --- src/core/media/client_death_observer.h 1970-01-01 00:00:00 +0000
> +++ src/core/media/client_death_observer.h 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,62 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +
> +#ifndef CORE_UBUNTU_MEDIA_CLIENT_DEATH_OBSERVER_H_
> +#define CORE_UBUNTU_MEDIA_CLIENT_DEATH_OBSERVER_H_
> +
> +#include <core/media/player.h>
> +
> +#include <core/signal.h>
> +
> +#include <memory>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +// Models functionality to be notified whenever a client
Please use doxygen style comments.
> +// of the service goes away, and thus allows us to clean
> +// up in that case.
> +struct ClientDeathObserver
> +{
> + // To safe us some typing.
Typo, should be "save," not "safe"
> + typedef std::shared_ptr<ClientDeathObserver> Ptr;
> +
> + ClientDeathObserver() = default;
> + ClientDeathObserver(const ClientDeathObserver&) = delete;
> + virtual ~ClientDeathObserver() = default;
> +
> + ClientDeathObserver& operator=(const ClientDeathObserver&) = delete;
> +
> + // Registers the client with the given key for death notifications.
> + virtual void register_for_death_notifications_with_key(const Player::PlayerKey&) = 0;
> +
> + // Emitted whenver a client dies, reporting the key under which the
> + // respective client was known.
> + virtual const core::Signal<Player::PlayerKey>& on_client_with_key_died() const = 0;
> +};
> +
> +// Accesses the default client death observer implementation for the platform.
> +ClientDeathObserver::Ptr platform_default_client_death_observer();
> +}
> +}
> +}
> +
> +#endif // CORE_UBUNTU_MEDIA_CLIENT_DEATH_OBSERVER_H_
>
> === modified file 'src/core/media/codec.h'
> --- src/core/media/codec.h 2014-10-10 01:24:43 +0000
> +++ src/core/media/codec.h 2014-11-13 09:06:33 +0000
> @@ -230,6 +230,46 @@
> }
> };
>
> +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<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()};
> + }
> +};
> +
> }
> }
>
>
> === modified file 'src/core/media/engine.h'
> --- src/core/media/engine.h 2014-11-03 18:00:48 +0000
> +++ src/core/media/engine.h 2014-11-13 09:06:33 +0000
> @@ -76,6 +76,8 @@
> virtual const core::Property<State>& state() const = 0;
>
> virtual bool open_resource_for_uri(const Track::UriType& uri) = 0;
> + // Throws core::ubuntu::media::Player::Error::OutOfProcessBufferStreamingNotSupported if the implementation does not
Please use doxygen style comments.
> + // support this feature.
> virtual void create_video_sink(uint32_t texture_id) = 0;
>
> virtual bool play() = 0;
> @@ -104,7 +106,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 void reset() = 0;
> };
>
> === modified file 'src/core/media/gstreamer/engine.cpp'
> --- src/core/media/gstreamer/engine.cpp 2014-11-03 18:00:48 +0000
> +++ src/core/media/gstreamer/engine.cpp 2014-11-13 09:06:33 +0000
> @@ -100,9 +100,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()
> @@ -163,12 +163,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)))
> {
> }
>
> @@ -203,7 +202,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;
> };
>
> gstreamer::Engine::Engine() : d(new Private{})
> @@ -379,7 +378,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 2014-11-03 18:00:48 +0000
> +++ src/core/media/gstreamer/engine.h 2014-11-13 09:06:33 +0000
> @@ -62,7 +62,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;
>
> void reset();
>
>
> === added file 'src/core/media/gstreamer/playbin.cpp'
> --- src/core/media/gstreamer/playbin.cpp 1970-01-01 00:00:00 +0000
> +++ src/core/media/gstreamer/playbin.cpp 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,501 @@
> +/*
> + * Copyright © 2013 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ß <thomas.voss at canonical.com>
> + */
> +
> +#include <core/media/gstreamer/playbin.h>
> +
> +#include <core/media/gstreamer/engine.h>
> +
> +#if defined(MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER)
> +#include <hybris/media/surface_texture_client_hybris.h>
> +#include <hybris/media/media_codec_layer.h>
> +
> +namespace
> +{
> +void setup_video_sink_for_buffer_streaming(GstElement* video_sink)
> +{
> + // Get the service-side BufferQueue (IGraphicBufferProducer) and associate it with
> + // the SurfaceTextureClientHybris instance
> + IGBPWrapperHybris igbp = decoding_service_get_igraphicbufferproducer();
> + SurfaceTextureClientHybris stc = surface_texture_client_create_by_igbp(igbp);
> + // Because mirsink is being loaded, we are definitely doing * hardware rendering.
> + surface_texture_client_set_hardware_rendering (stc, TRUE);
> + g_object_set (G_OBJECT (video_sink), "surface", static_cast<gpointer>(stc), static_cast<char*>(NULL));
> +}
> +}
> +#else // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +namespace
> +{
> +void setup_video_sink_for_buffer_streaming(GstElement*)
> +{
> + throw core::ubuntu::media::Player::Error::OutOfProcessBufferStreamingNotSupported{};
> +}
> +}
> +#endif // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +
> +namespace
> +{
> +bool is_mir_video_sink()
> +{
> + return g_strcmp0(::getenv("CORE_UBUNTU_MEDIA_SERVICE_VIDEO_SINK_NAME"), "mirsink") == 0;
> +}
> +}
> +// Uncomment to generate a dot file at the time that the pipeline
> +// goes to the PLAYING state. Make sure to export GST_DEBUG_DUMP_DOT_DIR
> +// before starting media-hub-server. To convert the dot file to something
> +// other image format, use: dot pipeline.dot -Tpng -o pipeline.png
> +//#define DEBUG_GST_PIPELINE
> +
> +namespace media = core::ubuntu::media;
> +namespace video = core::ubuntu::media::video;
> +
> +const std::string& gstreamer::Playbin::pipeline_name()
> +{
> + static const std::string s{"playbin"};
> + return s;
> +}
> +
> +void gstreamer::Playbin::about_to_finish(GstElement*, gpointer user_data)
> +{
> + auto thiz = static_cast<Playbin*>(user_data);
> + thiz->signals.about_to_finish();
> +}
> +
> +gstreamer::Playbin::Playbin()
> + : pipeline(gst_element_factory_make("playbin", pipeline_name().c_str())),
> + 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(
> + &Playbin::on_new_message,
> + this,
> + std::placeholders::_1))),
> + is_seeking(false)
> +{
> + if (!pipeline)
> + throw std::runtime_error("Could not create pipeline for playbin.");
> +
> + // Add audio and/or video sink elements depending on environment variables
> + // being set or not set
> + setup_pipeline_for_audio_video();
> +
> + g_signal_connect(
> + pipeline,
> + "about-to-finish",
> + G_CALLBACK(about_to_finish),
> + this
> + );
> +}
> +
> +gstreamer::Playbin::~Playbin()
> +{
> + if (pipeline)
> + gst_object_unref(pipeline);
> +}
> +
> +void gstreamer::Playbin::reset()
> +{
> + std::cout << "Client died, resetting pipeline" << std::endl;
> + // When the client dies, tear down the current pipeline and get it
> + // in a state that is ready for the next client that connects to the
> + // service
> + reset_pipeline();
> + // Signal to the Player class that the client side has disconnected
> + signals.client_disconnected();
> +}
> +
> +void gstreamer::Playbin::reset_pipeline()
> +{
> + std::cout << __PRETTY_FUNCTION__ << std::endl;
> + auto ret = gst_element_set_state(pipeline, GST_STATE_NULL);
> + switch(ret)
> + {
> + case GST_STATE_CHANGE_FAILURE:
> + std::cout << "Failed to reset the pipeline state. Client reconnect may not function properly." << std::endl;
> + break;
> + case GST_STATE_CHANGE_NO_PREROLL:
> + case GST_STATE_CHANGE_SUCCESS:
> + case GST_STATE_CHANGE_ASYNC:
> + break;
> + default:
> + std::cout << "Failed to reset the pipeline state. Client reconnect may not function properly." << std::endl;
> + }
> + file_type = MEDIA_FILE_TYPE_NONE;
> +}
> +
> +void gstreamer::Playbin::on_new_message(const Bus::Message& message)
> +{
> + switch(message.type)
> + {
> + case GST_MESSAGE_ERROR:
> + signals.on_error(message.detail.error_warning_info);
> + break;
> + case GST_MESSAGE_WARNING:
> + signals.on_warning(message.detail.error_warning_info);
> + break;
> + case GST_MESSAGE_INFO:
> + signals.on_info(message.detail.error_warning_info);
> + break;
> + case GST_MESSAGE_TAG:
> + {
> + gchar *orientation;
> + if (gst_tag_list_get_string(message.detail.tag.tag_list, "image-orientation", &orientation))
> + {
> + // If the image-orientation tag is in the GstTagList, signal the Engine
> + signals.on_orientation_changed(orientation_lut(orientation));
> + g_free (orientation);
> + }
> +
> + signals.on_tag_available(message.detail.tag);
> + }
> + break;
> + case GST_MESSAGE_STATE_CHANGED:
> + signals.on_state_changed(message.detail.state_changed);
> + break;
> + case GST_MESSAGE_ASYNC_DONE:
> + if (is_seeking)
> + {
> + // FIXME: Pass the actual playback time position to the signal call
> + signals.on_seeked_to(0);
> + is_seeking = false;
> + }
> + break;
> + case GST_MESSAGE_EOS:
> + signals.on_end_of_stream();
> + default:
> + break;
> + }
> +}
> +
> +gstreamer::Bus& gstreamer::Playbin::message_bus()
> +{
> + return bus;
> +}
> +
> +void gstreamer::Playbin::setup_pipeline_for_audio_video()
> +{
> + gint flags;
> + g_object_get (pipeline, "flags", &flags, nullptr);
> + flags |= GST_PLAY_FLAG_AUDIO;
> + flags |= GST_PLAY_FLAG_VIDEO;
> + flags &= ~GST_PLAY_FLAG_TEXT;
> + g_object_set (pipeline, "flags", flags, nullptr);
> +
> + if (::getenv("CORE_UBUNTU_MEDIA_SERVICE_AUDIO_SINK_NAME") != nullptr)
> + {
> + auto audio_sink = gst_element_factory_make (
> + ::getenv("CORE_UBUNTU_MEDIA_SERVICE_AUDIO_SINK_NAME"),
> + "audio-sink");
> +
> + std::cout << "audio_sink: " << ::getenv("CORE_UBUNTU_MEDIA_SERVICE_AUDIO_SINK_NAME") << std::endl;
> +
> + g_object_set (
> + pipeline,
> + "audio-sink",
> + audio_sink,
> + NULL);
> + }
> +
> + if (::getenv("CORE_UBUNTU_MEDIA_SERVICE_VIDEO_SINK_NAME") != nullptr)
> + {
> + video_sink = gst_element_factory_make (
> + ::getenv("CORE_UBUNTU_MEDIA_SERVICE_VIDEO_SINK_NAME"),
> + "video-sink");
> +
> + std::cout << "video_sink: " << ::getenv("CORE_UBUNTU_MEDIA_SERVICE_VIDEO_SINK_NAME") << std::endl;
> +
> + g_object_set (
> + pipeline,
> + "video-sink",
> + video_sink,
> + NULL);
> + }
> +}
> +
> +void gstreamer::Playbin::create_video_sink(uint32_t)
> +{
> + if (not video_sink) throw std::logic_error
> + {
> + "No video sink configured for the current pipeline"
> + };
> +
> + setup_video_sink_for_buffer_streaming(video_sink);
> +}
> +
> +void gstreamer::Playbin::set_volume(double new_volume)
> +{
> + g_object_set (pipeline, "volume", new_volume, NULL);
> +}
> +
> +/** Translate the AudioStreamRole enum into a string */
> +std::string gstreamer::Playbin::get_audio_role_str(media::Player::AudioStreamRole audio_role)
> +{
> + switch (audio_role)
> + {
> + case media::Player::AudioStreamRole::alarm:
> + return "alarm";
> + break;
> + case media::Player::AudioStreamRole::alert:
> + return "alert";
> + break;
> + case media::Player::AudioStreamRole::multimedia:
> + return "multimedia";
> + break;
> + case media::Player::AudioStreamRole::phone:
> + return "phone";
> + break;
> + default:
> + return "multimedia";
> + break;
> + }
> +}
> +
> +media::Player::Orientation gstreamer::Playbin::orientation_lut(const gchar *orientation)
> +{
> + if (g_strcmp0(orientation, "rotate-0") == 0)
> + return media::Player::Orientation::rotate0;
> + else if (g_strcmp0(orientation, "rotate-90") == 0)
> + return media::Player::Orientation::rotate90;
> + else if (g_strcmp0(orientation, "rotate-180") == 0)
> + return media::Player::Orientation::rotate180;
> + else if (g_strcmp0(orientation, "rotate-270") == 0)
> + return media::Player::Orientation::rotate270;
> + else
> + return media::Player::Orientation::rotate0;
> +}
> +
> +/** Sets the new audio stream role on the pulsesink in playbin */
> +void gstreamer::Playbin::set_audio_stream_role(media::Player::AudioStreamRole new_audio_role)
> +{
> + GstElement *audio_sink = NULL;
> + g_object_get (pipeline, "audio-sink", &audio_sink, NULL);
> +
> + std::string role_str("props,media.role=" + get_audio_role_str(new_audio_role));
> + std::cout << "Audio stream role: " << role_str << std::endl;
> +
> + GstStructure *props = gst_structure_from_string (role_str.c_str(), NULL);
> + if (audio_sink != nullptr && props != nullptr)
> + g_object_set (audio_sink, "stream-properties", props, NULL);
> + else
> + {
> + std::cerr <<
> + "Warning: couldn't set audio stream role - couldn't get audio_sink from pipeline" <<
> + std::endl;
> + }
> +
> + gst_structure_free (props);
> +}
> +
> +uint64_t gstreamer::Playbin::position() const
> +{
> + int64_t pos = 0;
> + gst_element_query_position (pipeline, GST_FORMAT_TIME, &pos);
> +
> + // FIXME: this should be int64_t, but dbus-cpp doesn't seem to handle it correctly
> + return static_cast<uint64_t>(pos);
> +}
> +
> +uint64_t gstreamer::Playbin::duration() const
> +{
> + int64_t dur = 0;
> + gst_element_query_duration (pipeline, GST_FORMAT_TIME, &dur);
> +
> + // FIXME: this should be int64_t, but dbus-cpp doesn't seem to handle it correctly
> + return static_cast<uint64_t>(dur);
> +}
> +
> +void gstreamer::Playbin::set_uri(const std::string& uri)
> +{
> + g_object_set(pipeline, "uri", uri.c_str(), NULL);
> + if (is_video_file(uri))
> + file_type = MEDIA_FILE_TYPE_VIDEO;
> + else if (is_audio_file(uri))
> + file_type = MEDIA_FILE_TYPE_AUDIO;
> +}
> +
> +std::string gstreamer::Playbin::uri() const
> +{
> + gchar* data = nullptr;
> + g_object_get(pipeline, "current-uri", &data, nullptr);
> +
> + std::string result((data == nullptr ? "" : data));
> + g_free(data);
> +
> + return result;
> +}
> +
> +bool gstreamer::Playbin::set_state_and_wait(GstState new_state)
> +{
> + static const std::chrono::nanoseconds state_change_timeout
> + {
> + // We choose a quite high value here as tests are run under valgrind
> + // and gstreamer pipeline setup/state changes take longer in that scenario.
> + // The value does not negatively impact runtime performance.
> + std::chrono::milliseconds{5000}
> + };
> +
> + auto ret = gst_element_set_state(pipeline, new_state);
> + bool result = false; GstState current, pending;
> + switch(ret)
> + {
> + case GST_STATE_CHANGE_FAILURE:
> + result = false; break;
> + case GST_STATE_CHANGE_NO_PREROLL:
> + case GST_STATE_CHANGE_SUCCESS:
> + result = true; break;
> + case GST_STATE_CHANGE_ASYNC:
> + result = GST_STATE_CHANGE_SUCCESS == gst_element_get_state(
> + pipeline,
> + ¤t,
> + &pending,
> + state_change_timeout.count());
> +
> + if (new_state == GST_STATE_PLAYING)
> + {
> + // Get the video height/width from the video sink
> + try
> + {
> + signals.on_video_dimensions_changed(get_video_dimensions());
> + }
> + catch (const std::exception& e)
> + {
> + std::cerr << "Problem querying video dimensions: " << e.what() << std::endl;
> + }
> + catch (...)
> + {
> + std::cerr << "Problem querying video dimensions." << std::endl;
> + }
> +
> +#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;
> +}
> +
> +bool gstreamer::Playbin::seek(const std::chrono::microseconds& ms)
> +{
> + is_seeking = true;
> + return gst_element_seek_simple(
> + pipeline,
> + GST_FORMAT_TIME,
> + (GstSeekFlags)(GST_SEEK_FLAG_FLUSH | GST_SEEK_FLAG_KEY_UNIT),
> + ms.count() * 1000);
> +}
> +
> +core::ubuntu::media::video::Dimensions gstreamer::Playbin::get_video_dimensions() const
> +{
> + if (not video_sink || not is_mir_video_sink())
> + throw std::runtime_error
> + {
> + "Missing video sink or video sink does not support query of width and height."
> + };
> +
> + // Initialize to default value prior to querying actual values from the sink.
> + video_width = video_height = 0;
> + g_object_get (video_sink, "height", &video_height, nullptr);
> + g_object_get (video_sink, "width", &video_width, nullptr);
> + // TODO(tvoss): We should probably check here if width and height are valid.
> + return core::ubuntu::media::video::Dimensions
> + {
> + core::ubuntu::media::video::Height{video_height},
> + core::ubuntu::media::video::Width{video_width}
> + };
> +}
> +
> +std::string gstreamer::Playbin::get_file_content_type(const std::string& uri) const
> +{
> + if (uri.empty())
> + return std::string();
> +
> + std::string filename(uri);
> + std::cout << "filename: " << filename << std::endl;
> + size_t pos = uri.find("file://");
> + if (pos != std::string::npos)
> + filename = uri.substr(pos + 7, std::string::npos);
> + else
> + // Anything other than a file, for now claim that the type
> + // is both audio and video.
> + // FIXME: implement true net stream sampling and get the type from GstCaps
> + return std::string("audio/video/");
> +
> +
> + GError *error = nullptr;
> + std::unique_ptr<GFile, void(*)(void *)> file(
> + g_file_new_for_path(filename.c_str()), g_object_unref);
> + std::unique_ptr<GFileInfo, void(*)(void *)> info(
> + g_file_query_info(
> + file.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE ","
> + G_FILE_ATTRIBUTE_ETAG_VALUE, G_FILE_QUERY_INFO_NONE,
> + /* cancellable */ NULL, &error),
> + g_object_unref);
> + if (!info)
> + {
> + std::string error_str(error->message);
> + g_error_free(error);
> +
> + std::cout << "Failed to query the URI for the presence of video content: "
> + << error_str << std::endl;
> + return std::string();
> + }
> +
> + std::string content_type(g_file_info_get_attribute_string(
> + info.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE));
> +
> + return content_type;
> +}
> +
> +bool gstreamer::Playbin::is_audio_file(const std::string& uri) const
> +{
> + if (uri.empty())
> + return false;
> +
> + if (get_file_content_type(uri).find("audio/") == 0)
> + {
> + std::cout << "Found audio content" << std::endl;
> + return true;
> + }
> +
> + return false;
> +}
> +
> +bool gstreamer::Playbin::is_video_file(const std::string& uri) const
> +{
> + if (uri.empty())
> + return false;
> +
> + if (get_file_content_type(uri).find("video/") == 0)
> + {
> + std::cout << "Found video content" << std::endl;
> + return true;
> + }
> +
> + return false;
> +}
> +
> +gstreamer::Playbin::MediaFileType gstreamer::Playbin::media_file_type() const
> +{
> + return file_type;
> +}
>
> === modified file 'src/core/media/gstreamer/playbin.h'
> --- src/core/media/gstreamer/playbin.h 2014-11-03 18:00:48 +0000
> +++ src/core/media/gstreamer/playbin.h 2014-11-13 09:06:33 +0000
> @@ -22,9 +22,6 @@
> #include "bus.h"
> #include "../mpris/player.h"
>
> -#include <hybris/media/surface_texture_client_hybris.h>
> -#include <hybris/media/media_codec_layer.h>
> -
> #include <gio/gio.h>
> #include <gst/gst.h>
>
> @@ -37,8 +34,6 @@
> // other image format, use: dot pipeline.dot -Tpng -o pipeline.png
> //#define DEBUG_GST_PIPELINE
>
> -namespace media = core::ubuntu::media;
> -
> namespace gstreamer
> {
> struct Playbin
> @@ -57,450 +52,57 @@
> MEDIA_FILE_TYPE_VIDEO
> };
>
> - static const std::string& pipeline_name()
> - {
> - static const std::string s{"playbin"};
> - return s;
> - }
> -
> - static void about_to_finish(GstElement*,
> - gpointer user_data)
> - {
> - auto thiz = static_cast<Playbin*>(user_data);
> - thiz->signals.about_to_finish();
> - }
> -
> - Playbin()
> - : pipeline(gst_element_factory_make("playbin", pipeline_name().c_str())),
> - 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(
> - &Playbin::on_new_message,
> - this,
> - std::placeholders::_1))),
> - is_seeking(false)
> - {
> - if (!pipeline)
> - throw std::runtime_error("Could not create pipeline for playbin.");
> -
> - // Add audio and/or video sink elements depending on environment variables
> - // being set or not set
> - setup_pipeline_for_audio_video();
> -
> - g_signal_connect(
> - pipeline,
> - "about-to-finish",
> - G_CALLBACK(about_to_finish),
> - this
> - );
> - }
> -
> - ~Playbin()
> - {
> - if (pipeline)
> - gst_object_unref(pipeline);
> - }
> -
> - void reset()
> - {
> - std::cout << "Client died, resetting pipeline" << std::endl;
> - // When the client dies, tear down the current pipeline and get it
> - // in a state that is ready for the next client that connects to the
> - // service
> - reset_pipeline();
> - // Signal to the Player class that the client side has disconnected
> - signals.client_disconnected();
> - }
> -
> - void reset_pipeline()
> - {
> - std::cout << __PRETTY_FUNCTION__ << std::endl;
> - auto ret = gst_element_set_state(pipeline, GST_STATE_NULL);
> - switch(ret)
> - {
> - case GST_STATE_CHANGE_FAILURE:
> - std::cout << "Failed to reset the pipeline state. Client reconnect may not function properly." << std::endl;
> - break;
> - case GST_STATE_CHANGE_NO_PREROLL:
> - case GST_STATE_CHANGE_SUCCESS:
> - case GST_STATE_CHANGE_ASYNC:
> - break;
> - default:
> - std::cout << "Failed to reset the pipeline state. Client reconnect may not function properly." << std::endl;
> - }
> - file_type = MEDIA_FILE_TYPE_NONE;
> - }
> -
> - void on_new_message(const Bus::Message& message)
> - {
> - switch(message.type)
> - {
> - case GST_MESSAGE_ERROR:
> - signals.on_error(message.detail.error_warning_info);
> - break;
> - case GST_MESSAGE_WARNING:
> - signals.on_warning(message.detail.error_warning_info);
> - break;
> - case GST_MESSAGE_INFO:
> - signals.on_info(message.detail.error_warning_info);
> - break;
> - case GST_MESSAGE_TAG:
> - {
> - gchar *orientation;
> - if (gst_tag_list_get_string(message.detail.tag.tag_list, "image-orientation", &orientation))
> - {
> - // If the image-orientation tag is in the GstTagList, signal the Engine
> - signals.on_orientation_changed(orientation_lut(orientation));
> - g_free (orientation);
> - }
> -
> - signals.on_tag_available(message.detail.tag);
> - }
> - break;
> - case GST_MESSAGE_STATE_CHANGED:
> - signals.on_state_changed(message.detail.state_changed);
> - break;
> - case GST_MESSAGE_ASYNC_DONE:
> - if (is_seeking)
> - {
> - // FIXME: Pass the actual playback time position to the signal call
> - signals.on_seeked_to(0);
> - is_seeking = false;
> - }
> - break;
> - case GST_MESSAGE_EOS:
> - signals.on_end_of_stream();
> - default:
> - break;
> - }
> - }
> -
> - gstreamer::Bus& message_bus()
> - {
> - return bus;
> - }
> -
> - void setup_pipeline_for_audio_video()
> - {
> - gint flags;
> - g_object_get (pipeline, "flags", &flags, nullptr);
> - flags |= GST_PLAY_FLAG_AUDIO;
> - flags |= GST_PLAY_FLAG_VIDEO;
> - flags &= ~GST_PLAY_FLAG_TEXT;
> - g_object_set (pipeline, "flags", flags, nullptr);
> -
> - if (::getenv("CORE_UBUNTU_MEDIA_SERVICE_AUDIO_SINK_NAME") != nullptr)
> - {
> - auto audio_sink = gst_element_factory_make (
> - ::getenv("CORE_UBUNTU_MEDIA_SERVICE_AUDIO_SINK_NAME"),
> - "audio-sink");
> -
> - std::cout << "audio_sink: " << ::getenv("CORE_UBUNTU_MEDIA_SERVICE_AUDIO_SINK_NAME") << std::endl;
> -
> - g_object_set (
> - pipeline,
> - "audio-sink",
> - audio_sink,
> - NULL);
> - }
> -
> - if (::getenv("CORE_UBUNTU_MEDIA_SERVICE_VIDEO_SINK_NAME") != nullptr)
> - {
> - video_sink = gst_element_factory_make (
> - ::getenv("CORE_UBUNTU_MEDIA_SERVICE_VIDEO_SINK_NAME"),
> - "video-sink");
> -
> - std::cout << "video_sink: " << ::getenv("CORE_UBUNTU_MEDIA_SERVICE_VIDEO_SINK_NAME") << std::endl;
> -
> - g_object_set (
> - pipeline,
> - "video-sink",
> - video_sink,
> - NULL);
> - }
> - }
> -
> - void create_video_sink(uint32_t texture_id)
> - {
> - std::cout << "Creating video sink for texture_id: " << texture_id << std::endl;
> -
> - if (::getenv("CORE_UBUNTU_MEDIA_SERVICE_VIDEO_SINK_NAME") != nullptr)
> - {
> - g_object_get (pipeline, "video_sink", &video_sink, NULL);
> -
> - // Get the service-side BufferQueue (IGraphicBufferProducer) and associate it with
> - // the SurfaceTextureClientHybris instance
> - IGBPWrapperHybris igbp = decoding_service_get_igraphicbufferproducer();
> - SurfaceTextureClientHybris stc = surface_texture_client_create_by_igbp(igbp);
> - // Because mirsink is being loaded, we are definitely doing * hardware rendering.
> - surface_texture_client_set_hardware_rendering (stc, TRUE);
> - g_object_set (G_OBJECT (video_sink), "surface", static_cast<gpointer>(stc), static_cast<char*>(NULL));
> - }
> - }
> -
> - void set_volume(double new_volume)
> - {
> - g_object_set (pipeline, "volume", new_volume, NULL);
> - }
> -
> - /** Translate the AudioStreamRole enum into a string */
> - static std::string get_audio_role_str(media::Player::AudioStreamRole audio_role)
> - {
> - switch (audio_role)
> - {
> - case media::Player::AudioStreamRole::alarm:
> - return "alarm";
> - break;
> - case media::Player::AudioStreamRole::alert:
> - return "alert";
> - break;
> - case media::Player::AudioStreamRole::multimedia:
> - return "multimedia";
> - break;
> - case media::Player::AudioStreamRole::phone:
> - return "phone";
> - break;
> - default:
> - return "multimedia";
> - break;
> - }
> - }
> -
> - media::Player::Orientation orientation_lut(const gchar *orientation)
> - {
> - if (g_strcmp0(orientation, "rotate-0") == 0)
> - return media::Player::Orientation::rotate0;
> - else if (g_strcmp0(orientation, "rotate-90") == 0)
> - return media::Player::Orientation::rotate90;
> - else if (g_strcmp0(orientation, "rotate-180") == 0)
> - return media::Player::Orientation::rotate180;
> - else if (g_strcmp0(orientation, "rotate-270") == 0)
> - return media::Player::Orientation::rotate270;
> - else
> - return media::Player::Orientation::rotate0;
> - }
> + static std::string get_audio_role_str(core::ubuntu::media::Player::AudioStreamRole audio_role);
> +
> + static const std::string& pipeline_name();
> +
> + static void about_to_finish(GstElement*, gpointer user_data);
> +
> + Playbin();
> + ~Playbin();
> +
> + void reset();
> + void reset_pipeline();
> +
> + void on_new_message(const Bus::Message& message);
> +
> + gstreamer::Bus& message_bus();
> +
> + void setup_pipeline_for_audio_video();
> +
> + void create_video_sink(uint32_t texture_id);
> +
> + void set_volume(double new_volume);
> +
> + core::ubuntu::media::Player::Orientation orientation_lut(const gchar *orientation);
>
> /** Sets the new audio stream role on the pulsesink in playbin */
> - void set_audio_stream_role(media::Player::AudioStreamRole new_audio_role)
> - {
> - GstElement *audio_sink = NULL;
> - g_object_get (pipeline, "audio-sink", &audio_sink, NULL);
> -
> - std::string role_str("props,media.role=" + get_audio_role_str(new_audio_role));
> - std::cout << "Audio stream role: " << role_str << std::endl;
> -
> - GstStructure *props = gst_structure_from_string (role_str.c_str(), NULL);
> - if (audio_sink != nullptr && props != nullptr)
> - g_object_set (audio_sink, "stream-properties", props, NULL);
> - else
> - {
> - std::cerr <<
> - "Warning: couldn't set audio stream role - couldn't get audio_sink from pipeline" <<
> - std::endl;
> - }
> -
> - gst_structure_free (props);
> - }
> -
> - uint64_t position() const
> - {
> - int64_t pos = 0;
> - gst_element_query_position (pipeline, GST_FORMAT_TIME, &pos);
> -
> - // FIXME: this should be int64_t, but dbus-cpp doesn't seem to handle it correctly
> - return static_cast<uint64_t>(pos);
> - }
> -
> - uint64_t duration() const
> - {
> - int64_t dur = 0;
> - gst_element_query_duration (pipeline, GST_FORMAT_TIME, &dur);
> -
> - // FIXME: this should be int64_t, but dbus-cpp doesn't seem to handle it correctly
> - return static_cast<uint64_t>(dur);
> - }
> -
> - void set_uri(const std::string& uri)
> - {
> - g_object_set(pipeline, "uri", uri.c_str(), NULL);
> - if (is_video_file(uri))
> - file_type = MEDIA_FILE_TYPE_VIDEO;
> - else if (is_audio_file(uri))
> - file_type = MEDIA_FILE_TYPE_AUDIO;
> - }
> -
> - std::string uri() const
> - {
> - gchar* data = nullptr;
> - g_object_get(pipeline, "current-uri", &data, nullptr);
> -
> - std::string result((data == nullptr ? "" : data));
> - g_free(data);
> -
> - return result;
> - }
> -
> - bool set_state_and_wait(GstState new_state)
> - {
> - static const std::chrono::nanoseconds state_change_timeout
> - {
> - // We choose a quite high value here as tests are run under valgrind
> - // and gstreamer pipeline setup/state changes take longer in that scenario.
> - // The value does not negatively impact runtime performance.
> - std::chrono::milliseconds{5000}
> - };
> -
> - auto ret = gst_element_set_state(pipeline, new_state);
> - bool result = false; GstState current, pending;
> - switch(ret)
> - {
> - case GST_STATE_CHANGE_FAILURE:
> - result = false; break;
> - case GST_STATE_CHANGE_NO_PREROLL:
> - case GST_STATE_CHANGE_SUCCESS:
> - result = true; break;
> - case GST_STATE_CHANGE_ASYNC:
> - result = GST_STATE_CHANGE_SUCCESS == gst_element_get_state(
> - pipeline,
> - ¤t,
> - &pending,
> - state_change_timeout.count());
> -
> - if (new_state == GST_STATE_PLAYING)
> - {
> - // Get the video height/width from the video sink
> - 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;
> - }
> -
> - bool seek(const std::chrono::microseconds& ms)
> - {
> - is_seeking = true;
> - return gst_element_seek_simple(
> - pipeline,
> - GST_FORMAT_TIME,
> - (GstSeekFlags)(GST_SEEK_FLAG_FLUSH | GST_SEEK_FLAG_KEY_UNIT),
> - ms.count() * 1000);
> - }
> -
> - void get_video_dimensions()
> - {
> - if (video_sink != nullptr && g_strcmp0(::getenv("CORE_UBUNTU_MEDIA_SERVICE_VIDEO_SINK_NAME"), "mirsink") == 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;
> - 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;
> - }
> -
> - std::string get_file_content_type(const std::string& uri) const
> - {
> - if (uri.empty())
> - return std::string();
> -
> - std::string filename(uri);
> - std::cout << "filename: " << filename << std::endl;
> - size_t pos = uri.find("file://");
> - if (pos != std::string::npos)
> - filename = uri.substr(pos + 7, std::string::npos);
> - else
> - // Anything other than a file, for now claim that the type
> - // is both audio and video.
> - // FIXME: implement true net stream sampling and get the type from GstCaps
> - return std::string("audio/video/");
> -
> -
> - GError *error = nullptr;
> - std::unique_ptr<GFile, void(*)(void *)> file(
> - g_file_new_for_path(filename.c_str()), g_object_unref);
> - std::unique_ptr<GFileInfo, void(*)(void *)> info(
> - g_file_query_info(
> - file.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE ","
> - G_FILE_ATTRIBUTE_ETAG_VALUE, G_FILE_QUERY_INFO_NONE,
> - /* cancellable */ NULL, &error),
> - g_object_unref);
> - if (!info)
> - {
> - std::string error_str(error->message);
> - g_error_free(error);
> -
> - std::cout << "Failed to query the URI for the presence of video content: "
> - << error_str << std::endl;
> - return std::string();
> - }
> -
> - std::string content_type(g_file_info_get_attribute_string(
> - info.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE));
> -
> - return content_type;
> - }
> -
> - bool is_audio_file(const std::string& uri) const
> - {
> - if (uri.empty())
> - return false;
> -
> - if (get_file_content_type(uri).find("audio/") == 0)
> - {
> - std::cout << "Found audio content" << std::endl;
> - return true;
> - }
> -
> - return false;
> - }
> -
> - bool is_video_file(const std::string& uri) const
> - {
> - if (uri.empty())
> - return false;
> -
> - if (get_file_content_type(uri).find("video/") == 0)
> - {
> - std::cout << "Found video content" << std::endl;
> - return true;
> - }
> -
> - return false;
> - }
> -
> - MediaFileType media_file_type() const
> - {
> - return file_type;
> - }
> + void set_audio_stream_role(core::ubuntu::media::Player::AudioStreamRole new_audio_role);
> +
> + uint64_t position() const;
> + uint64_t duration() const;
> +
> + void set_uri(const std::string& uri);
> + std::string uri() const;
> +
> + bool set_state_and_wait(GstState new_state);
> + bool seek(const std::chrono::microseconds& ms);
> +
> + core::ubuntu::media::video::Dimensions get_video_dimensions() const;
> +
> + std::string get_file_content_type(const std::string& uri) const;
> +
> + bool is_audio_file(const std::string& uri) const;
> + bool is_video_file(const std::string& uri) const;
> +
> + MediaFileType media_file_type() const;
>
> GstElement* pipeline;
> gstreamer::Bus bus;
> MediaFileType file_type;
> - SurfaceTextureClientHybris stc_hybris;
> GstElement* video_sink;
> - uint32_t video_height;
> - uint32_t video_width;
> + mutable uint32_t video_height;
> + mutable uint32_t video_width;
> core::Connection on_new_message_connection;
> bool is_seeking;
> struct
> @@ -513,9 +115,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;
> };
>
> === added file 'src/core/media/hybris_client_death_observer.cpp'
> --- src/core/media/hybris_client_death_observer.cpp 1970-01-01 00:00:00 +0000
> +++ src/core/media/hybris_client_death_observer.cpp 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,94 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +
> +#include <core/media/hybris_client_death_observer.h>
> +
> +namespace media = core::ubuntu::media;
> +
> +#if defined(MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER)
> +#include <hybris/media/media_codec_layer.h>
> +
> +namespace
> +{
> +struct Holder
Can't you use a std::pair<PlayerKey, std::weak_ptr> instead of creating your own new type here?
> +{
> + media::Player::PlayerKey key;
> + std::weak_ptr<media::HybrisClientDeathObserver> observer;
> +};
> +}
> +
> +struct media::HybrisClientDeathObserver::Private
No reason to use a Private class pattern with this class. Please remove and use the parent HybrisClientDeathObserver class.
> +{
> + // Our static fringe that we inject to hybris
> + static void on_client_died_cb(void* context)
> + {
> + auto holder = static_cast<Holder*>(context);
> +
> + if (not holder)
> + return;
> +
> + // We check if we are still alive or if we already got killed.
> + if (auto sp = holder->observer.lock())
> + {
> + sp->d->client_with_key_died(holder->key);
> + }
> +
> + // And with that, we have reached end of life for our holder object.
> + delete holder;
> + }
> +
> + core::Signal<media::Player::PlayerKey> client_with_key_died;
> +};
> +
> +// Creates an instance of the HybrisClientDeathObserver or throws
> +// if the underlying platform does not support it.
> +media::ClientDeathObserver::Ptr media::HybrisClientDeathObserver::create()
> +{
> + return media::ClientDeathObserver::Ptr{new media::HybrisClientDeathObserver{}};
Please use standard syntax "return media::ClientDeathObserver::Ptr(new media::HybrisClientDeathObserver);
> +}
> +
> +media::HybrisClientDeathObserver::HybrisClientDeathObserver() : d{new Private{}}
Please use standard syntax "d(new Private)"
> +{
> +}
> +
> +media::HybrisClientDeathObserver::~HybrisClientDeathObserver()
> +{
> +}
> +
> +void media::HybrisClientDeathObserver::register_for_death_notifications_with_key(const media::Player::PlayerKey& key)
> +{
> + decoding_service_set_client_death_cb(&Private::on_client_died_cb, key, new Holder{key, shared_from_this()});
Please use standard syntax "new Holder(key, shared_from_this())"
> +}
> +
> +const core::Signal<media::Player::PlayerKey>& media::HybrisClientDeathObserver::on_client_with_key_died() const
> +{
> + return d->client_with_key_died;
> +}
> +#else // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +// Creates an instance of the HybrisClientDeathObserver or throws
> +// if the underlying platform does not support it.
> +media::ClientDeathObserver::Ptr media::HybrisClientDeathObserver::create()
> +{
> + throw std::logic_error
Please use standard syntax:
throw std::logic_error("Hybris-based death observer implementation not supported on this platform.");
> + {
> + "Hybris-based death observer implementation not supported on this platform."
> + };
> +}
> +#endif // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +
> +
>
> === added file 'src/core/media/hybris_client_death_observer.h'
> --- src/core/media/hybris_client_death_observer.h 1970-01-01 00:00:00 +0000
> +++ src/core/media/hybris_client_death_observer.h 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,61 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +
> +#ifndef CORE_UBUNTU_MEDIA_HYBRIS_CLIENT_DEATH_OBSERVER_H_
> +#define CORE_UBUNTU_MEDIA_HYBRIS_CLIENT_DEATH_OBSERVER_H_
> +
> +#include <core/media/client_death_observer.h>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +// Models functionality to be notified whenever a client
> +// of the service goes away, and thus allows us to clean
> +// up in that case.
> +class HybrisClientDeathObserver : public ClientDeathObserver,
> + public std::enable_shared_from_this<HybrisClientDeathObserver>
What does this line give us exactly?
> +{
> +public:
> + // Creates an instance of the HybrisClientDeathObserver or throws
> + // if the underlying platform does not support it.
> + static ClientDeathObserver::Ptr create();
> +
> + // Make std::unique_ptr happy for forward declared Private internals.
> + ~HybrisClientDeathObserver();
> +
> + // Registers the client with the given key for death notifications.
> + void register_for_death_notifications_with_key(const Player::PlayerKey&) override;
> +
> + // Emitted whenver a client dies, reporting the key under which the
> + // respective client was known.
> + const core::Signal<Player::PlayerKey>& on_client_with_key_died() const override;
> +
> +private:
> + HybrisClientDeathObserver();
> +
> + struct Private;
> + std::unique_ptr<Private> d;
> +};
> +}
> +}
> +}
> +
> +#endif // CORE_UBUNTU_MEDIA_HYBRIS_CLIENT_DEATH_OBSERVER_H_
>
> === added file 'src/core/media/hybris_recorder_observer.cpp'
> --- src/core/media/hybris_recorder_observer.cpp 1970-01-01 00:00:00 +0000
> +++ src/core/media/hybris_recorder_observer.cpp 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,99 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +
> +#include <core/media/hybris_recorder_observer.h>
> +
> +namespace media = core::ubuntu::media;
> +
> +#if defined(MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER)
> +#include <hybris/media/media_recorder_layer.h>
> +
> +struct media::HybrisRecorderObserver::Private
No reason to use a Private class here.
> +{
> + struct Holder
Any reason to abstract the weak pointer into a parent struct? Why not just directly hold the weak_ptr since there's only 1 variable here?
> + {
> + std::weak_ptr<media::HybrisRecorderObserver::Private> wp;
> + };
> +
> + // The fringe that we hand over to hybris.
> + static void on_media_recording_state_changed(bool started, void* context)
> + {
> + if (auto holder = static_cast<Holder*>(context))
> + {
> + if (auto sp = holder->wp.lock())
> + {
> + sp->recording_state = started ? media::RecordingState::started : media::RecordingState::stopped;
> + }
> + }
> + }
> +
> + // TODO: We have no way of freeing the observer and thus leak
> + // an instance here.
> + MediaRecorderObserver* observer
> + {
> + android_media_recorder_observer_new()
Can you file a bug against libhybris to add an android_media_recorder_observer_delete()?
> + };
> +
> + core::Property<media::RecordingState> recording_state
When does recording_state get assigned, when Private is created? Does it happen before or after the initializer list?
> + {
> + media::RecordingState::stopped
> + };
> +
> + Holder* holder
> + {
> + nullptr
> + };
> +};
> +
> +media::HybrisRecorderObserver::HybrisRecorderObserver() : d{new Private{}}
Please use standard syntax "d(new Private)"
> +{
> + android_media_recorder_observer_set_cb(
> + d->observer,
> + &Private::on_media_recording_state_changed,
> + d->holder = new Private::Holder{d});
> +}
> +
> +media::HybrisRecorderObserver::~HybrisRecorderObserver()
> +{
> + // We first reset the context of the callback.
> + android_media_recorder_observer_set_cb(
> + d->observer,
> + &Private::on_media_recording_state_changed,
> + nullptr);
> +
> + delete d->holder;
> +}
> +
> +const core::Property<media::RecordingState>& media::HybrisRecorderObserver::recording_state() const
> +{
> + return d->recording_state;
> +}
> +
> +media::RecorderObserver::Ptr media::HybrisRecorderObserver::create()
> +{
> + return media::RecorderObserver::Ptr{new media::HybrisRecorderObserver{}};
Please use standard syntax. I'm not going to list these anymore since I'd like to see them all changed for added code readability.
> +}
> +#else // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +media::RecorderObserver::Ptr media::HybrisRecorderObserver::create()
> +{
> + throw std::logic_error
> + {
> + "Hybris-based recorder observer implementation not supported on this platform."
> + };
> +}
> +#endif // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
>
> === added file 'src/core/media/hybris_recorder_observer.h'
> --- src/core/media/hybris_recorder_observer.h 1970-01-01 00:00:00 +0000
> +++ src/core/media/hybris_recorder_observer.h 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,52 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +
> +#ifndef CORE_UBUNTU_MEDIA_HYBRIS_RECORDER_OBSERVER_H_
> +#define CORE_UBUNTU_MEDIA_HYBRIS_RECORDER_OBSERVER_H_
> +
> +#include <core/media/recorder_observer.h>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +class HybrisRecorderObserver : public RecorderObserver
> +{
> +public:
> + // Creates a new instance of the HybrisRecorderObserver or throws
> + // if hybris is not available.
> + static RecorderObserver::Ptr create();
> +
> + ~HybrisRecorderObserver();
> +
> + // Getable/observable property describing the recording state of the system.
> + const core::Property<RecordingState>& recording_state() const override;
> +
> +private:
> + struct Private;
> +
> + HybrisRecorderObserver();
> + std::shared_ptr<Private> d;
> +};
> +}
> +}
> +}
> +
> +#endif // CORE_UBUNTU_MEDIA_HYBRIS_RECORDER_OBSERVER_H_
>
> === modified file 'src/core/media/mpris/player.h'
> --- src/core/media/mpris/player.h 2014-10-14 20:05:20 +0000
> +++ src/core/media/mpris/player.h 2014-11-13 09:06:33 +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
> @@ -103,6 +107,17 @@
> static constexpr const char* stopped{"Stopped"};
> };
>
> + struct Error
> + {
> + struct OutOfProcessBufferStreamingNotSupported
> + {
> + static constexpr const char* name
> + {
> + "mpris.Player.Error.OutOfProcessBufferStreamingNotSupported"
> + };
> + };
> + };
> +
> typedef std::map<std::string, core::dbus::types::Variant> Dictionary;
>
> DBUS_CPP_METHOD_DEF(Next, Player)
> @@ -122,7 +137,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)
> };
>
> struct Properties
>
> === modified file 'src/core/media/player.cpp'
> --- src/core/media/player.cpp 2014-09-09 10:28:32 +0000
> +++ src/core/media/player.cpp 2014-11-13 09:06:33 +0000
> @@ -22,6 +22,11 @@
>
> namespace media = core::ubuntu::media;
>
> +media::Player::Error::OutOfProcessBufferStreamingNotSupported::OutOfProcessBufferStreamingNotSupported()
> + : std::runtime_error{"Implementation does not support out-of-process buffer streaming"}
> +{
> +}
> +
> const media::Player::Configuration& media::Player::Client::default_configuration()
> {
> static const media::Player::Configuration config
>
> === modified file 'src/core/media/player_implementation.cpp'
> --- src/core/media/player_implementation.cpp 2014-11-03 18:00:48 +0000
> +++ src/core/media/player_implementation.cpp 2014-11-13 09:06:33 +0000
> @@ -21,10 +21,10 @@
>
> #include <unistd.h>
>
> +#include "client_death_observer.h"
> #include "engine.h"
> #include "track_list_implementation.h"
>
> -#include <hybris/media/media_codec_layer.h>
> #include "powerd_service.h"
> #include "unity_screen_service.h"
> #include "gstreamer/engine.h"
> @@ -81,7 +81,16 @@
> auto uscreen_stub_service = dbus::Service::use_service(bus, dbus::traits::Service<core::UScreen>::interface_name());
> uscreen_session = uscreen_stub_service->object_for_path(dbus::types::ObjectPath("/com/canonical/Unity/Screen"));
>
> - decoding_service_set_client_death_cb(&Private::on_client_died_cb, key, static_cast<void*>(this));
> + auto client_death_observer = media::platform_default_client_death_observer();
> +
> + client_death_observer->register_for_death_notifications_with_key(key);
> + client_death_observer->on_client_with_key_died().connect([this](const media::Player::PlayerKey& died)
> + {
> + if (died != this->key)
> + return;
> +
> + on_client_died();
> + });
> }
>
> ~Private()
> @@ -261,15 +270,6 @@
> };
> }
>
> - static void on_client_died_cb(void *context)
> - {
> - if (context)
> - {
> - Private *p = static_cast<Private*>(context);
> - p->on_client_died();
> - }
> - }
> -
> void on_client_died()
> {
> engine->reset();
> @@ -408,12 +408,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);
> });
> }
>
> @@ -458,22 +455,17 @@
> return d->key;
> }
>
> +media::video::Sink::Ptr media::PlayerImplementation::create_gl_texture_video_sink(std::uint32_t texture_id)
> +{
> + d->engine->create_video_sink(texture_id);
> + return media::video::Sink::Ptr{};
> +}
> +
> bool media::PlayerImplementation::open_uri(const Track::UriType& uri)
> {
> return d->engine->open_resource_for_uri(uri);
> }
>
> -void media::PlayerImplementation::create_video_sink(uint32_t texture_id)
> -{
> - d->engine->create_video_sink(texture_id);
> -}
> -
> -media::Player::GLConsumerWrapperHybris media::PlayerImplementation::gl_consumer() const
> -{
> - // This method is client-side only and is simply a no-op for the service side
> - return NULL;
> -}
> -
> void media::PlayerImplementation::next()
> {
> }
> @@ -498,18 +490,6 @@
> d->engine->stop();
> }
>
> -void media::PlayerImplementation::set_frame_available_callback(
> - UNUSED FrameAvailableCb cb, UNUSED void *context)
> -{
> - // This method is client-side only and is simply a no-op for the service side
> -}
> -
> -void media::PlayerImplementation::set_playback_complete_callback(
> - UNUSED PlaybackCompleteCb cb, UNUSED void *context)
> -{
> - // This method is client-side only and is simply a no-op for the service side
> -}
> -
> void media::PlayerImplementation::seek_to(const std::chrono::microseconds& ms)
> {
> d->engine->seek_to(ms);
>
> === modified file 'src/core/media/player_implementation.h'
> --- src/core/media/player_implementation.h 2014-09-25 04:35:46 +0000
> +++ src/core/media/player_implementation.h 2014-11-13 09:06:33 +0000
> @@ -46,16 +46,14 @@
> virtual std::shared_ptr<TrackList> track_list();
> virtual PlayerKey key() const;
>
> + virtual video::Sink::Ptr create_gl_texture_video_sink(std::uint32_t texture_id);
> +
> virtual bool open_uri(const Track::UriType& uri);
> - virtual void create_video_sink(uint32_t texture_id);
> - virtual GLConsumerWrapperHybris gl_consumer() const;
> virtual void next();
> virtual void previous();
> virtual void play();
> virtual void pause();
> virtual void stop();
> - virtual void set_frame_available_callback(FrameAvailableCb cb, void *context);
> - virtual void set_playback_complete_callback(PlaybackCompleteCb cb, void *context);
> virtual void seek_to(const std::chrono::microseconds& offset);
>
> const core::Signal<>& on_client_disconnected() const;
>
> === modified file 'src/core/media/player_skeleton.cpp'
> --- src/core/media/player_skeleton.cpp 2014-10-14 20:15:56 +0000
> +++ src/core/media/player_skeleton.cpp 2014-11-13 09:06:33 +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"
> @@ -135,9 +136,29 @@
> {
> uint32_t texture_id;
> in->reader() >> texture_id;
> - impl->create_video_sink(texture_id);
> -
> - auto reply = dbus::Message::make_method_return(in);
> +
> + core::dbus::Message::Ptr reply;
> +
> + try
> + {
> + impl->create_gl_texture_video_sink(texture_id);
> + reply = dbus::Message::make_method_return(in);
> + }
> + catch (const media::Player::Error::OutOfProcessBufferStreamingNotSupported& e)
How will this work with phone vs desktop? Desktop won't support out of process buffer streaming, so won't the desktop case always throw this exception?
> + {
> + reply = dbus::Message::make_error(
> + in,
> + mpris::Player::Error::OutOfProcessBufferStreamingNotSupported::name,
> + e.what());
> + }
> + catch (...)
> + {
> + reply = dbus::Message::make_error(
> + in,
> + mpris::Player::Error::OutOfProcessBufferStreamingNotSupported::name,
> + std::string{});
> + }
> +
> bus->send(reply);
> }
>
> @@ -298,7 +319,7 @@
> 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& mask)
> {
> remote_video_dimension_changed->emit(mask);
> });
> @@ -307,7 +328,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;
> } signals;
>
> };
> @@ -587,12 +608,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 2014-10-14 20:05:20 +0000
> +++ src/core/media/player_skeleton.h 2014-11-13 09:06:33 +0000
> @@ -73,7 +73,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;
>
> protected:
> // All creation time arguments go here.
> @@ -109,7 +109,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();
>
> private:
> struct Private;
>
> === modified file 'src/core/media/player_stub.cpp'
> --- src/core/media/player_stub.cpp 2014-11-05 20:41:03 +0000
> +++ src/core/media/player_stub.cpp 2014-11-13 09:06:33 +0000
> @@ -29,13 +29,11 @@
>
> #include "mpris/player.h"
>
> +#include "video/platform_default_sink.h"
> +
> #include <core/dbus/property.h>
> #include <core/dbus/types/object_path.h>
>
> -// Hybris
> -#include <hybris/media/media_codec_layer.h>
> -#include <hybris/media/surface_texture_client_hybris.h>
> -
> #include <limits>
>
> #define UNUSED __attribute__((unused))
> @@ -48,13 +46,8 @@
> Private(const std::shared_ptr<Service>& parent,
> const std::shared_ptr<core::dbus::Object>& object
> ) : parent(parent),
> - texture_id(0),
> - igbc_wrapper(nullptr),
> - glc_wrapper(nullptr),
> - decoding_session(nullptr),
> - frame_available_cb(nullptr),
> - frame_available_context(nullptr),
> object(object),
> + key(object->invoke_method_synchronously<mpris::Player::Key, media::Player::PlayerKey>().value()),
> properties
> {
> // Link the properties from the server side to the client side over the bus
> @@ -87,64 +80,19 @@
> object->get_signal<mpris::Player::Signals::VideoDimensionChanged>()
> }
> {
> - auto op = object->invoke_method_synchronously<mpris::Player::Key, media::Player::PlayerKey>();
> - decoding_session = decoding_service_create_session(op.value());
> }
>
> ~Private()
> {
> }
>
> - static void on_frame_available_cb(UNUSED GLConsumerWrapperHybris wrapper, void *context)
> - {
> - if (context != nullptr) {
> - Private *p = static_cast<Private*>(context);
> - p->on_frame_available();
> - }
> - else
> - std::cerr << "context is nullptr, can't call on_frame_available()" << std::endl;
> - }
> -
> - void on_frame_available()
> - {
> - if (frame_available_cb != nullptr) {
> - frame_available_cb(frame_available_context);
> - }
> - else
> - std::cerr << "frame_available_cb is nullptr, can't call frame_available_cb()" << std::endl;
> - }
> -
> - void set_frame_available_cb(FrameAvailableCb cb, void *context)
> - {
> - frame_available_cb = cb;
> - frame_available_context = context;
> -
> - gl_consumer_set_frame_available_cb(glc_wrapper, &Private::on_frame_available_cb, static_cast<void*>(this));
> - }
> -
> - /** We need a GLConsumerHybris instance for doing texture streaming over the
> - * process boundary **/
> - void get_gl_consumer()
> - {
> - igbc_wrapper = decoding_service_get_igraphicbufferconsumer();
> - glc_wrapper = gl_consumer_create_by_id_with_igbc(texture_id, igbc_wrapper);
> -
> - }
> -
> std::shared_ptr<Service> parent;
> std::shared_ptr<TrackList> track_list;
>
> - uint32_t texture_id;
> - IGBCWrapperHybris igbc_wrapper;
> - GLConsumerWrapperHybris glc_wrapper;
> -
> - DSSessionWrapperHybris decoding_session;
> -
> - FrameAvailableCb frame_available_cb;
> - void *frame_available_context;
> -
> dbus::Object::Ptr object;
>
> + media::Player::PlayerKey key;
> +
> struct
> {
> std::shared_ptr<core::dbus::Property<mpris::Player::Properties::CanPlay>> can_play;
> @@ -181,9 +129,7 @@
> const std::shared_ptr<DBusEndOfStreamSignal>& eos,
> const std::shared_ptr<DBusPlaybackStatusChangedSignal>& status,
> const std::shared_ptr<DBusVideoDimensionChangedSignal>& d)
> - : playback_complete_cb(nullptr),
> - playback_complete_context(nullptr),
> - seeked_to(),
> + : seeked_to(),
> end_of_stream(),
> playback_status_changed(),
> video_dimension_changed(),
> @@ -204,8 +150,6 @@
> dbus.end_of_stream->connect([this]()
> {
> std::cout << "EndOfStream signal arrived via the bus." << std::endl;
> - if (playback_complete_cb)
> - playback_complete_cb(playback_complete_context);
> end_of_stream();
> });
>
> @@ -215,25 +159,17 @@
> 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);
> });
> - }
> -
> - void set_playback_complete_cb(PlaybackCompleteCb cb, void *context)
> - {
> - playback_complete_cb = cb;
> - playback_complete_context = context;
> }
>
> - PlaybackCompleteCb playback_complete_cb;
> - void *playback_complete_context;
> 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;
>
> struct DBus
> {
> @@ -269,9 +205,7 @@
>
> media::Player::PlayerKey media::PlayerStub::key() const
> {
> - auto op = d->object->invoke_method_synchronously<mpris::Player::Key, media::Player::PlayerKey>();
> -
> - return op.value();
> + return d->key;
> }
>
> bool media::PlayerStub::open_uri(const media::Track::UriType& uri)
> @@ -281,19 +215,19 @@
> return op.value();
> }
>
> -void media::PlayerStub::create_video_sink(uint32_t texture_id)
> +media::video::Sink::Ptr media::PlayerStub::create_gl_texture_video_sink(std::uint32_t texture_id)
> {
> auto op = d->object->invoke_method_synchronously<mpris::Player::CreateVideoSink, void>(texture_id);
> - d->texture_id = texture_id;
> - d->get_gl_consumer();
>
> if (op.is_error())
> - throw std::runtime_error("Problem creating new video sink instance on remote object");
> -}
> + {
> + if (op.error().name() == mpris::Player::Error::OutOfProcessBufferStreamingNotSupported::name)
> + throw media::Player::Error::OutOfProcessBufferStreamingNotSupported{};
> + else
> + throw std::runtime_error{op.error().print()};
> + }
>
> -GLConsumerWrapperHybris media::PlayerStub::gl_consumer() const
> -{
> - return d->glc_wrapper;
> + return media::video::make_platform_default_sink(texture_id, d->key);
> }
>
> void media::PlayerStub::next()
> @@ -344,16 +278,6 @@
> throw std::runtime_error("Problem stopping playback on remote object");
> }
>
> -void media::PlayerStub::set_frame_available_callback(FrameAvailableCb cb, void *context)
> -{
> - d->set_frame_available_cb(cb, context);
> -}
> -
> -void media::PlayerStub::set_playback_complete_callback(PlaybackCompleteCb cb, void *context)
> -{
> - d->signals.set_playback_complete_cb(cb, context);
> -}
> -
> const core::Property<bool>& media::PlayerStub::can_play() const
> {
> return *d->properties.can_play;
> @@ -489,7 +413,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 2014-10-22 20:08:22 +0000
> +++ src/core/media/player_stub.h 2014-11-13 09:06:33 +0000
> @@ -46,9 +46,9 @@
> virtual std::shared_ptr<TrackList> track_list();
> virtual PlayerKey key() const;
>
> - virtual bool open_uri(const Track::UriType& uri);
> - virtual void create_video_sink(uint32_t texture_id);
> - virtual GLConsumerWrapperHybris gl_consumer() const;
> + virtual video::Sink::Ptr create_gl_texture_video_sink(std::uint32_t texture_id);
> +
> + virtual bool open_uri(const Track::UriType& uri);
> virtual void next();
> virtual void previous();
> virtual void play();
> @@ -56,9 +56,6 @@
> virtual void seek_to(const std::chrono::microseconds& offset);
> virtual void stop();
>
> - virtual void set_frame_available_callback(FrameAvailableCb cb, void *context);
> - virtual void set_playback_complete_callback(PlaybackCompleteCb cb, void *context);
> -
> virtual const core::Property<bool>& can_play() const;
> virtual const core::Property<bool>& can_pause() const;
> virtual const core::Property<bool>& can_seek() const;
> @@ -88,7 +85,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;
>
> private:
> struct Private;
>
> === added file 'src/core/media/recorder_observer.cpp'
> --- src/core/media/recorder_observer.cpp 1970-01-01 00:00:00 +0000
> +++ src/core/media/recorder_observer.cpp 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,28 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +
> +#include <core/media/recorder_observer.h>
> +
> +#include <core/media/hybris_recorder_observer.h>
> +
> +namespace media = core::ubuntu::media;
> +
> +media::RecorderObserver::Ptr media::make_platform_default_recorder_observer()
> +{
> + return media::HybrisRecorderObserver::create();
> +}
>
> === added file 'src/core/media/recorder_observer.h'
> --- src/core/media/recorder_observer.h 1970-01-01 00:00:00 +0000
> +++ src/core/media/recorder_observer.h 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,64 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +
> +#ifndef CORE_UBUNTU_MEDIA_RECORDER_OBSERVER_H_
> +#define CORE_UBUNTU_MEDIA_RECORDER_OBSERVER_H_
> +
> +#include <core/property.h>
> +
> +#include <memory>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +// All known states of the recorder
> +enum class RecordingState
> +{
> + // No active recording
> + stopped,
> + // We have an active recording session
> + started
> +};
> +
> +// A RecorderObserver allows for monitoring the recording state
> +// of the service.
> +struct RecorderObserver
> +{
> + // To safe us some typing.
> + typedef std::shared_ptr<RecorderObserver> Ptr;
> +
> + RecorderObserver() = default;
> + RecorderObserver(const RecorderObserver&) = delete;
> + virtual ~RecorderObserver() = default;
> + RecorderObserver& operator=(const RecorderObserver&) = delete;
> +
> + // Getable/observable property describing the recording state of the system.
> + virtual const core::Property<RecordingState>& recording_state() const = 0;
> +};
> +
> +// Creates an instance of interface RecorderObserver relying on
> +// default services offered by the platform we are currently running on.
> +RecorderObserver::Ptr make_platform_default_recorder_observer();
> +}
> +}
> +}
> +
> +#endif // CORE_UBUNTU_MEDIA_RECORDER_OBSERVER_H_
>
> === modified file 'src/core/media/server/server.cpp'
> --- src/core/media/server/server.cpp 2014-04-04 14:31:43 +0000
> +++ src/core/media/server/server.cpp 2014-11-13 09:06:33 +0000
> @@ -20,8 +20,6 @@
> #include <core/media/player.h>
> #include <core/media/track_list.h>
>
> -#include <hybris/media/media_codec_layer.h>
> -
> #include "core/media/service_implementation.h"
>
> #include <iostream>
> @@ -30,10 +28,32 @@
>
> using namespace std;
>
> +#if defined(MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER)
> +#include <hybris/media/media_codec_layer.h>
> +
> +namespace
> +{
> +// All platform-specific initialization routines go here.
> +void platform_init()
> +{
> + decoding_service_init();
> +}
> +}
> +#else // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +namespace
> +{
> +// All platform-specific initialization routines go here.
> +void platform_init()
> +{
> + // Consciously left empty
> +}
> +}
> +#endif // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +
> int main()
> {
> - // Init hybris-level DecodingService
> - decoding_service_init();
> + platform_init();
> +
> cout << "Starting DecodingService..." << endl;
>
> auto service = std::make_shared<media::ServiceImplementation>();
>
> === modified file 'src/core/media/service_implementation.cpp'
> --- src/core/media/service_implementation.cpp 2014-11-03 00:56:55 +0000
> +++ src/core/media/service_implementation.cpp 2014-11-13 09:06:33 +0000
> @@ -23,6 +23,7 @@
> #include "call-monitor/call_monitor.h"
> #include "player_configuration.h"
> #include "player_implementation.h"
> +#include "recorder_observer.h"
>
> #include <boost/asio.hpp>
>
> @@ -33,7 +34,6 @@
>
> #include "util/timeout.h"
> #include "unity_screen_service.h"
> -#include <hybris/media/media_recorder_layer.h>
>
> namespace media = core::ubuntu::media;
>
> @@ -68,8 +68,11 @@
> auto uscreen_stub_service = dbus::Service::use_service(bus, dbus::traits::Service<core::UScreen>::interface_name());
> uscreen_session = uscreen_stub_service->object_for_path(dbus::types::ObjectPath("/com/canonical/Unity/Screen"));
>
> - observer = android_media_recorder_observer_new();
> - android_media_recorder_observer_set_cb(observer, &Private::media_recording_started_callback, this);
> + recorder_observer = media::make_platform_default_recorder_observer();
> + recorder_observer->recording_state().changed().connect([this](media::RecordingState state)
> + {
> + media_recording_state_changed(state);
> + });
> }
>
> ~Private()
> @@ -80,12 +83,12 @@
> worker.join();
> }
>
> - void media_recording_started(bool started)
> + void media_recording_state_changed(media::RecordingState state)
> {
> if (uscreen_session == nullptr)
> return;
>
> - if (started)
> + if (state == media::RecordingState::started)
> {
> if (disp_cookie > 0)
> return;
> @@ -95,7 +98,7 @@
> throw std::runtime_error(result.error().print());
> disp_cookie = result.value();
> }
> - else
> + else if (state == media::RecordingState::stopped)
> {
> if (disp_cookie != -1)
> {
> @@ -107,15 +110,6 @@
> }
> }
>
> - static void media_recording_started_callback(bool started, void *context)
> - {
> - if (context == nullptr)
> - return;
> -
> - auto p = static_cast<Private*>(context);
> - p->media_recording_started(started);
> - }
> -
> // This holds the key of the multimedia role Player instance that was paused
> // when the battery level reached 10% or 5%
> media::Player::PlayerKey resume_key;
> @@ -128,7 +122,7 @@
> std::shared_ptr<core::dbus::Property<core::IndicatorPower::IsWarning>> is_warning;
> int disp_cookie;
> std::shared_ptr<dbus::Object> uscreen_session;
> - MediaRecorderObserver *observer;
> + media::RecorderObserver::Ptr recorder_observer;
> std::unique_ptr<CallMonitor> call_monitor;
> std::list<media::Player::PlayerKey> paused_sessions;
> };
>
> === added directory 'src/core/media/video'
> === added file 'src/core/media/video/hybris_gl_sink.cpp'
> --- src/core/media/video/hybris_gl_sink.cpp 1970-01-01 00:00:00 +0000
> +++ src/core/media/video/hybris_gl_sink.cpp 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,116 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +
> +#include <core/media/video/hybris_gl_sink.h>
> +
> +#if defined(MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER)
> +// Hybris
> +#include <hybris/media/media_codec_layer.h>
> +#include <hybris/media/surface_texture_client_hybris.h>
> +
> +namespace media = core::ubuntu::media;
> +namespace video = core::ubuntu::media::video;
> +
> +struct video::HybrisGlSink::Private
I see no reason to have a Private class for HybrisGlSink. It just adds to the number of lines of code. Please remove.
> +{
> + static void on_frame_available_callback(GLConsumerWrapperHybris, void* context)
> + {
> + if (not context)
> + return;
> +
> + auto thiz = static_cast<Private*>(context);
> +
> + thiz->frame_available();
> + }
> +
> + Private(std::uint32_t gl_texture, const media::Player::PlayerKey& player_key)
> + : gl_texture{gl_texture},
> + player_key{player_key},
> + decoding_service_session{decoding_service_create_session(player_key)},
> + graphics_buffer_consumer{decoding_service_get_igraphicbufferconsumer()},
> + gl_texture_consumer{gl_consumer_create_by_id_with_igbc(gl_texture, graphics_buffer_consumer)}
> + {
> + if (not decoding_service_session) throw std::runtime_error
Please use the standard syntax for throwing, it reads a lot cleaner and my eye expects it.
i.e.:
if (not decoding_service_session)
throw std::runtime_error("video::HybrisGlSink: Could not connect to the remote decoding service and establish a session.");
> + {
> + "video::HybrisGlSink: Could not connect to the remote decoding service and establish a session."
> + };
> +
> + if (not graphics_buffer_consumer) throw std::runtime_error
Ditto
> + {
> + "video::HybrisGlSink: Could not connect to remote buffer queue."
> + };
> +
> + if (not gl_texture_consumer) throw std::runtime_error
Ditto
> + {
> + "video::HybrisGlSink: Could not associate local texture id with remote buffer streak."
> + };
> +
> + gl_consumer_set_frame_available_cb(gl_texture_consumer, Private::on_frame_available_callback, this);
> + }
> +
> + ~Private()
> + {
> + gl_consumer_set_frame_available_cb(gl_texture_consumer, Private::on_frame_available_callback, nullptr);
> + }
> +
> + std::uint32_t gl_texture;
> + media::Player::PlayerKey player_key;
> + core::Signal<void> frame_available;
> + DSSessionWrapperHybris decoding_service_session;
> + IGBCWrapperHybris graphics_buffer_consumer;
> + GLConsumerWrapperHybris gl_texture_consumer;
> +};
> +
> +// Creates a new instance for the given gl texture or throws
> +// in case of issues.
> +video::HybrisGlSink::HybrisGlSink(std::uint32_t gl_texture, const media::Player::PlayerKey& key) : d{new Private{gl_texture, key}}
> +{
> +}
> +
> +video::HybrisGlSink::~HybrisGlSink()
> +{
> +}
> +
> +// The signal is emitted whenever a new frame is available and a subsequent
> +// call to swap_buffers will not block and return true.
> +const core::Signal<void>& video::HybrisGlSink::frame_available() const
> +{
> + return d->frame_available;
I'm not happy with the frame_available callback being wrapped with a Signal as this introduces even more delay as many new clock cycles are required to get this signal up to qtvideo-node in order to render. Although this may not introduce any noticeable rendering throughput (measured in the microseconds), it still might. I want to absolutely make sure that we get the exact same framerate throughput before this change and after it. You can get a measure of the framerate by enabling some profiling code that I put in qtubuntu-media (it'll print out frame rendering time deltas and decoding deltas).
> +}
> +
> +// Queries the transformation matrix for the current frame, placing the data into 'matrix'
> +// and returns true or returns false and leaves 'matrix' unchanged in case
> +// of issues.
> +bool video::HybrisGlSink::transformation_matrix(video::MatrixView<4,4>& matrix) const
> +{
> + // TODO: The underlying API really should tell us if everything is ok.
Curious when things would not be ok with the matrix? What did you have in mind by your TODO?
> + gl_consumer_get_transformation_matrix(d->gl_texture_consumer, matrix.to_float());
> + return true;
> +}
> +
> +// Releases the current buffer, and consumes the next buffer in the queue,
> +// making it available for consumption by consumers of this API in an
> +// implementation-specific way. Clients will usually rely on a GL texture
> +// to receive the latest buffer.
> +bool video::HybrisGlSink::swap_buffers() const
Please rename this to be update_texture to be consistent with the hybris-level call that it makes.
> +{
> + // TODO: The underlying API really should tell us if everything is ok.
> + gl_consumer_update_texture(d->gl_texture_consumer);
> + return true;
> +}
> +#endif // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
>
> === added file 'src/core/media/video/hybris_gl_sink.h'
> --- src/core/media/video/hybris_gl_sink.h 1970-01-01 00:00:00 +0000
> +++ src/core/media/video/hybris_gl_sink.h 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,67 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +#ifndef CORE_UBUNTU_MEDIA_VIDEO_HYBRIS_GL_SINK_H_
> +#define CORE_UBUNTU_MEDIA_VIDEO_HYBRIS_GL_SINK_H_
> +
> +#include <core/media/video/sink.h>
> +
> +#include <core/media/player.h>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +namespace video
> +{
> +class HybrisGlSink : public video::Sink
> +{
> +public:
> + // Creates a new instance for the given gl texture, connecting to the remote part known
Please use the doxygen style comment tags:
/**
* @brief blah blah
* @param gl_texture blah blah
*/
> + // under the given key or throw in case of issues.
> + HybrisGlSink(std::uint32_t gl_texture, const media::Player::PlayerKey& key);
> +
> + // Need this to avoid std::unique_ptr complaining about forward-declared Private.
> + ~HybrisGlSink();
> +
> + // The signal is emitted whenever a new frame is available and a subsequent
> + // call to swap_buffers will not block and return true.
> + const core::Signal<void>& frame_available() const override;
> +
> + // Queries the transformation matrix for the current frame, placing the data into 'matrix'
> + // and returns true or returns false and leaves 'matrix' unchanged in case
> + // of issues.
> + bool transformation_matrix(MatrixView<4,4>& matrix) const override;
> +
> + // Releases the current buffer, and consumes the next buffer in the queue,
> + // making it available for consumption by consumers of this API in an
> + // implementation-specific way. Clients will usually rely on a GL texture
> + // to receive the latest buffer.
> + bool swap_buffers() const override;
> +
> +private:
> + struct Private;
> + std::unique_ptr<Private> d;
> +};
> +}
> +}
> +}
> +}
> +
> +#endif // CORE_UBUNTU_MEDIA_VIDEO_HYBRIS_GL_SINK_H_
>
> === added file 'src/core/media/video/platform_default_sink.cpp'
> --- src/core/media/video/platform_default_sink.cpp 1970-01-01 00:00:00 +0000
> +++ src/core/media/video/platform_default_sink.cpp 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,67 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +
> +#include <core/media/video/platform_default_sink.h>
> +
> +namespace media = core::ubuntu::media;
> +namespace video = core::ubuntu::media::video;
> +
> +namespace
> +{
> +struct NullSink : public video::Sink
> +{
> + // The signal is emitted whenever a new frame is available and a subsequent
> + // call to swap_buffers will not block and return true.
> + const core::Signal<void>& frame_available() const;
> +
> + // Queries the transformation matrix for the current frame, placing the data into 'matrix'
> + // and returns true or returns false and leaves 'matrix' unchanged in case
> + // of issues.
> + bool transformation_matrix(video::MatrixView<4,4>& matrix) const
We have access to the Qt libs in media-hub now ever since the telephony call monitor was added. Please use the QtMatrix4x4 instead of video::MatrixView as this will make things consistent with qtubuntu-media. Matrix manipulation is complex enough without having to worry about matrix data type conversions.
> + {
> + for (unsigned int i = 0; i < matrix.height(); i++)
> + for (unsigned int j = 0; j < matrix.width(); j++)
> + matrix(i, j) = i == j ? 1.f : 0.f;
> +
> + return true;
> + }
> +
> + // Releases the current buffer, and consumes the next buffer in the queue,
> + // making it available for consumption by consumers of this API in an
> + // implementation-specific way. Clients will usually rely on a GL texture
> + // to receive the latest buffer.
> + bool swap_buffers() const
Please rename this to be update_texture to be consistent.
> + {
> + return true;
> + }
> +};
> +}
> +
> +#if defined(MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER)
> +#include <core/media/video/hybris_gl_sink.h>
> +
> +video::Sink::Ptr video::make_platform_default_sink(std::uint32_t gl_texture, const media::Player::PlayerKey& key)
> +{
> + return std::make_shared<video::HybrisGlSink>(gl_texture, key);
> +}
> +#else // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
> +video::Sink::Ptr video::make_platform_default_sink(std::uint32_t, const media::Player::PlayerKey&)
> +{
> + return std::make_shared<NullSink>();
> +}
> +#endif // MEDIA_HUB_HAVE_HYBRIS_MEDIA_COMPAT_LAYER
>
> === added file 'src/core/media/video/platform_default_sink.h'
> --- src/core/media/video/platform_default_sink.h 1970-01-01 00:00:00 +0000
> +++ src/core/media/video/platform_default_sink.h 2014-11-13 09:06:33 +0000
> @@ -0,0 +1,41 @@
> +/*
> + * 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ß <thomas.voss at canonical.com>
> + */
> +#ifndef CORE_UBUNTU_MEDIA_VIDEO_PLATFORM_DEFAULT_SINK_H_
> +#define CORE_UBUNTU_MEDIA_VIDEO_PLATFORM_DEFAULT_SINK_H_
> +
> +#include <core/media/video/sink.h>
> +
> +#include <core/media/player.h>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +namespace video
> +{
> +// Returns the platform default video sink for the given parameters. Never returns null, but
> +// throws in case of issues.
> +Sink::Ptr make_platform_default_sink(std::uint32_t gl_texture, const Player::PlayerKey& key);
> +}
> +}
> +}
> +}
> +
> +#endif // CORE_UBUNTU_MEDIA_VIDEO_PLATFORM_DEFAULT_SINK_H_
>
> === modified file 'tests/unit-tests/CMakeLists.txt'
> --- tests/unit-tests/CMakeLists.txt 2014-11-03 00:56:55 +0000
> +++ tests/unit-tests/CMakeLists.txt 2014-11-13 09:06:33 +0000
> @@ -7,12 +7,16 @@
> add_executable(
> test-gstreamer-engine
>
> - libmedia-mock.cpp
> + ${CMAKE_SOURCE_DIR}/src/core/media/client_death_observer.cpp
> + ${CMAKE_SOURCE_DIR}/src/core/media/hybris_client_death_observer.cpp
> ${CMAKE_SOURCE_DIR}/src/core/media/cover_art_resolver.cpp
> ${CMAKE_SOURCE_DIR}/src/core/media/engine.cpp
> ${CMAKE_SOURCE_DIR}/src/core/media/gstreamer/engine.cpp
> + ${CMAKE_SOURCE_DIR}/src/core/media/gstreamer/playbin.cpp
> ${CMAKE_SOURCE_DIR}/src/core/media/player_skeleton.cpp
> ${CMAKE_SOURCE_DIR}/src/core/media/player_implementation.cpp
> + ${CMAKE_SOURCE_DIR}/src/core/media/recorder_observer.cpp
> + ${CMAKE_SOURCE_DIR}/src/core/media/hybris_recorder_observer.cpp
> ${CMAKE_SOURCE_DIR}/src/core/media/service_skeleton.cpp
> ${CMAKE_SOURCE_DIR}/src/core/media/service_implementation.cpp
> ${CMAKE_SOURCE_DIR}/src/core/media/track_list_skeleton.cpp
>
> === removed file 'tests/unit-tests/libmedia-mock.cpp'
> --- tests/unit-tests/libmedia-mock.cpp 2014-11-03 18:00:48 +0000
> +++ tests/unit-tests/libmedia-mock.cpp 1970-01-01 00:00:00 +0000
> @@ -1,26 +0,0 @@
> -/*
> - * Copyright © 2013-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: Jim Hodapp <jim.hodapp at canonical.com>
> - */
> -
> -#include <hybris/media/media_codec_layer.h>
> -#include <hybris/media/surface_texture_client_hybris.h>
> -
> -#define UNUSED __attribute__((unused))
> -
> -void decoding_service_set_client_death_cb(UNUSED DecodingClientDeathCbHybris callback, UNUSED uint32_t handle, UNUSED void *context)
> -{
> -}
>
--
https://code.launchpad.net/~thomas-voss/media-hub/refactor-video-sink-interface-to-not-rely-on-hybris-types/+merge/241435
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.
More information about the Ubuntu-reviews
mailing list