[Merge] lp:~jhodapp/media-hub/player-add-prev-next into lp:media-hub

[Nick Dedekind]

Review: Needs Fixing

Added a couple of inline comments.

Diff comments:

> === modified file 'include/core/media/track_list.h'
> --- include/core/media/track_list.h	2015-04-20 15:45:51 +0000
> +++ include/core/media/track_list.h	2015-04-29 21:20:45 +0000
> @@ -70,6 +70,18 @@
>      /** Skip to the specified TrackId. Calls stop() and play() on the player if toggle_player_state is true. */
>      virtual void go_to(const Track::Id& track, bool toggle_player_state) = 0;
>  
> +    /** Returns true if there is a next track in the TrackList after the current one playing */
> +    bool has_next() const;
> +
> +    /** Returns true if there is a previous track in the TrackList before the current one playing */
> +    bool has_previous() const;

should be pure virtual?
or impl in track_list.cpp

> +
> +    /** Skip to the next Track in the TrackList if there is one. */
> +    virtual Track::Id next() = 0;
> +
> +    /** Skip to the previous Track in the TrackList if there is one. */
> +    virtual Track::Id previous() = 0;
> +
>  
>      /** Reorders the tracks such that they are in a random order. */
>      virtual void shuffle_tracks() = 0;
> 
> === modified file 'src/core/media/gstreamer/playbin.cpp'
> --- src/core/media/gstreamer/playbin.cpp	2015-04-15 17:13:58 +0000
> +++ src/core/media/gstreamer/playbin.cpp	2015-04-29 21:20:45 +0000
> @@ -510,7 +510,6 @@
>          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);
> 
> === modified file 'src/core/media/player_implementation.cpp'
> --- src/core/media/player_implementation.cpp	2015-04-27 20:52:06 +0000
> +++ src/core/media/player_implementation.cpp	2015-04-29 21:20:45 +0000
> @@ -316,8 +316,8 @@
>      Parent::can_play().set(true);
>      Parent::can_pause().set(true);
>      Parent::can_seek().set(true);
> -    Parent::can_go_previous().set(true);
> -    Parent::can_go_next().set(true);
> +    Parent::can_go_previous().set(false);
> +    Parent::can_go_next().set(false);
>      Parent::is_video_source().set(false);
>      Parent::is_audio_source().set(false);
>      Parent::shuffle().set(false);
> @@ -360,6 +360,18 @@
>      };
>      Parent::is_audio_source().install(audio_type_getter);
>  
> +    std::function<bool()> can_go_next_getter = [this]()
> +    {
> +        return d->track_list->has_next();
> +    };
> +    Parent::can_go_next().install(can_go_next_getter);
> +
> +    std::function<bool()> can_go_previous_getter = [this]()
> +    {
> +        return d->track_list->has_previous();
> +    };
> +    Parent::can_go_previous().install(can_go_previous_getter);
> +
>      // When the client changes the loop status, make sure to update the TrackList
>      Parent::loop_status().changed().connect([this](media::Player::LoopStatus loop_status)
>      {
> @@ -588,11 +600,13 @@
>  template<typename Parent>
>  void media::PlayerImplementation<Parent>::next()
>  {
> +    d->track_list->next();
>  }
>  
>  template<typename Parent>
>  void media::PlayerImplementation<Parent>::previous()
>  {
> +    d->track_list->previous();
>  }
>  
>  template<typename Parent>
> 
> === modified file 'src/core/media/track_list_implementation.cpp'
> --- src/core/media/track_list_implementation.cpp	2015-04-27 23:01:48 +0000
> +++ src/core/media/track_list_implementation.cpp	2015-04-29 21:20:45 +0000
> @@ -151,6 +151,16 @@
>      on_track_changed()(track);
>  }
>  
> +bool media::TrackListImplementation::has_next() const
> +{
> +    return media::TrackListSkeleton::has_next();
> +}
> +
> +bool media::TrackListImplementation::has_previous() const
> +{
> +    return media::TrackListSkeleton::has_previous();
> +}
> +

Do these need to be here? They are just redirecting to the parent anyway.

>  void media::TrackListImplementation::shuffle_tracks()
>  {
>      std::cout << __PRETTY_FUNCTION__ << std::endl;
> 
> === modified file 'src/core/media/track_list_implementation.h'
> --- src/core/media/track_list_implementation.h	2015-04-20 17:48:48 +0000
> +++ src/core/media/track_list_implementation.h	2015-04-29 21:20:45 +0000
> @@ -46,6 +46,8 @@
>      void remove_track(const Track::Id& id);
>  
>      void go_to(const Track::Id& track, bool toggle_player_state);
> +    bool has_next() const;
> +    bool has_previous() const;
>      void shuffle_tracks();
>      void unshuffle_tracks();
>      void reset();
> 
> === modified file 'src/core/media/track_list_skeleton.cpp'
> --- src/core/media/track_list_skeleton.cpp	2015-04-20 17:56:05 +0000
> +++ src/core/media/track_list_skeleton.cpp	2015-04-29 21:20:45 +0000
> @@ -206,7 +206,17 @@
>      return next_track != tracks().get().end();
>  }
>  
> -const media::Track::Id& media::TrackListSkeleton::next()
> +bool media::TrackListSkeleton::has_previous() const
> +{
> +    // If we are looping over the entire list, then there is always a previous track
> +    if (d->loop_status == media::Player::LoopStatus::playlist)
> +        return true;
> +
> +    std::cout << "has_previous track? " << (d->current_track != tracks().get().begin() ? "yes" : "no") << std::endl;
> +    return d->current_track != tracks().get().begin();
> +}
> +
> +media::Track::Id media::TrackListSkeleton::next()
>  {
>      std::cout << __PRETTY_FUNCTION__ << std::endl;
>      if (tracks().get().empty())
> @@ -235,6 +245,12 @@
>      return *(d->current_track);
>  }
>  
> +media::Track::Id media::TrackListSkeleton::previous()
> +{
> +    // TODO: Add logic to calculate the previous track
> +    return *(d->current_track);
> +}
> +
>  const media::Track::Id& media::TrackListSkeleton::current()
>  {
>      // Prevent the TrackList from sitting at the end which will cause
> 
> === modified file 'src/core/media/track_list_skeleton.h'
> --- src/core/media/track_list_skeleton.h	2015-04-20 17:48:48 +0000
> +++ src/core/media/track_list_skeleton.h	2015-04-29 21:20:45 +0000
> @@ -42,7 +42,9 @@
>      ~TrackListSkeleton();
>  
>      bool has_next() const;
> -    const Track::Id& next();
> +    bool has_previous() const;
> +    Track::Id next();
> +    Track::Id previous();
>      const Track::Id& current();
>  
>      const core::Property<bool>& can_edit_tracks() const;
> 
> === modified file 'src/core/media/track_list_stub.cpp'
> --- src/core/media/track_list_stub.cpp	2015-04-20 17:48:48 +0000
> +++ src/core/media/track_list_stub.cpp	2015-04-29 21:20:45 +0000
> @@ -137,6 +137,18 @@
>          throw std::runtime_error("Problem adding track: " + op.error());
>  }
>  
> +media::Track::Id media::TrackListStub::next()
> +{
> +    // TODO: Add this to the dbus interface on the server and implement a proper dbus method call
> +    return media::Track::Id{"/empty/track/id"};
> +}
> +
> +media::Track::Id media::TrackListStub::previous()
> +{
> +    // TODO: Add this to the dbus interface on the server and implement a proper dbus method call
> +    return media::Track::Id{"/empty/track/id"};
> +}
> +
>  void media::TrackListStub::shuffle_tracks()
>  {
>      std::cerr << "shuffle_tracks() does nothing from the client side" << std::endl;
> 
> === modified file 'src/core/media/track_list_stub.h'
> --- src/core/media/track_list_stub.h	2015-04-20 15:45:51 +0000
> +++ src/core/media/track_list_stub.h	2015-04-29 21:20:45 +0000
> @@ -51,6 +51,9 @@
>  
>      void go_to(const Track::Id& track, bool toggle_player_state);
>  
> +    Track::Id next();
> +    Track::Id previous();
> +
>      void shuffle_tracks();
>      void unshuffle_tracks();
>  
> 


-- 
https://code.launchpad.net/~jhodapp/media-hub/player-add-prev-next/+merge/257824
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.