[Merge] lp:~phablet-team/media-hub/mpris into lp:media-hub/stable

[Alfonso Sanchez-Beato]

Review: Needs Fixing

Some minor fixes required. A MP for trunk would be good too.

Diff comments:

> 
> === modified file 'debian/changelog'
> --- debian/changelog	2015-09-04 18:51:23 +0000
> +++ debian/changelog	2015-09-08 21:21:22 +0000
> @@ -1,3 +1,10 @@
> +media-hub (3.2.0+15.04.20150904-0ubuntu1) UNRELEASED; urgency=medium
> +
> +  * Make sure the correct player is set as the current player controled by MPRIS.
> +  * Improved the can_go_next() and can_go_previous() logic to always return true if the loop_status is currently set to loop over the entire playlist.

Too long line. 80 is the maximum we should allow for the changelog. Also, we should consider using the "Description of the change" field in the MP instead of changing directly the changelog in most cases. In the end I think makes the changelog less cluttered.

> +
> + -- phablet <phablet at ubuntu-phablet>  Tue, 08 Sep 2015 17:18:15 -0400
> +
>  media-hub (3.1.0+15.04.20150904-0ubuntu1) vivid; urgency=medium
>  
>    [ Alfonso Sanchez-Beato (email Canonical) ]
> 
> === modified file 'src/core/media/player_implementation.cpp'
> --- src/core/media/player_implementation.cpp	2015-07-27 22:15:33 +0000
> +++ src/core/media/player_implementation.cpp	2015-09-08 21:21:22 +0000
> @@ -376,13 +376,15 @@
>  
>      std::function<bool()> can_go_next_getter = [this]()
>      {
> -        return d->track_list->has_next();
> +        // If LoopStatus == playlist, then there is always a next track
> +        return d->track_list->has_next() or Parent::loop_status() == Player::LoopStatus::playlist;
>      };
>      Parent::can_go_next().install(can_go_next_getter);
>  
>      std::function<bool()> can_go_previous_getter = [this]()
>      {
> -        return d->track_list->has_previous();
> +        // If LoopStatus == playlist, then there is always a next previous
> +        return d->track_list->has_previous()  or Parent::loop_status() == Player::LoopStatus::playlist;

Double space before "or".

>      };
>      Parent::can_go_previous().install(can_go_previous_getter);
>  
> 
> === modified file 'src/core/media/service_skeleton.cpp'
> --- src/core/media/service_skeleton.cpp	2015-08-11 16:10:14 +0000
> +++ src/core/media/service_skeleton.cpp	2015-09-08 21:21:22 +0000
> @@ -516,10 +530,11 @@
>              // Setup method handlers for mpris::Player methods.
>              auto next = [this](const core::dbus::Message::Ptr& msg)
>              {
> -                auto sp = current_player.lock();
> +                const auto sp = current_player.lock();
>  
> -                if (sp)
> -                    sp->next();
> +                if (sp and
> +                    sp->audio_stream_role() == media::Player::AudioStreamRole::multimedia)

This "and" condition is lengthy and repeated in many places. It is probably worth to create an inline function to test it, something like is_multimedia(sp).

> +                        sp->next();
>  
>                  Exported::bus->send(core::dbus::Message::make_method_return(msg));
>              };


-- 
https://code.launchpad.net/~phablet-team/media-hub/mpris/+merge/269755
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub/stable.