[Merge] lp:~phablet-team/media-hub/bg-playlist-fixes into lp:media-hub/stable

Alfonso Sanchez-Beato alfonso.sanchez-beato at canonical.com
Thu Nov 12 14:41:38 UTC 2015


Some comments answered. I will update the branch later as I will modify qtubuntu-media too.

Diff comments:

> 
> === modified file 'src/core/media/track_list_skeleton.cpp'
> --- src/core/media/track_list_skeleton.cpp	2015-11-12 12:34:43 +0000
> +++ src/core/media/track_list_skeleton.cpp	2015-11-12 12:34:43 +0000
> @@ -110,6 +115,8 @@
>              // Only add the track to the TrackList if it passes the apparmor permissions check
>              if (std::get<0>(result))
>              {
> +                media::Track::Id next;
> +                //if (make_current)

Good catch, I forgot to remove this line (which was not present in trunk). Btw, I have noticed that many useful warnings are not being emitted by the compiler, we should change that: in this case it did not warn about the unused "next" variable.

>                  impl->add_track_with_uri_at(uri, after, make_current);
>              }
>              else
> @@ -176,8 +183,56 @@
>          media::Track::Id track;
>          msg->reader() >> track;
>  
> +        auto id_it = find(impl->tracks().get().begin(), impl->tracks().get().end(), track);
> +        if (id_it == impl->tracks().get().end()) {
> +            ostringstream err_str;
> +            err_str << "Track " << track << " not found in play list";
> +            cout << __PRETTY_FUNCTION__ << " WARNING " << err_str.str() << endl;
> +            auto reply = dbus::Message::make_error(
> +                            msg,
> +                            mpris::TrackList::Error::TrackNotFound::name,
> +                            err_str.str());
> +            bus->send(reply);
> +            return;
> +        }
> +
> +        media::Track::Id next;
> +        bool deleting_current = false;
> +
> +        if (id_it == impl->current_iterator()) {
> +            cout << "Removing current track" << endl;
> +            deleting_current = true;
> +
> +            if (current_track != empty_iterator) {
> +                ++current_track;
> +
> +                if (   current_track == impl->tracks().get().end()
> +                    && loop_status == media::Player::LoopStatus::playlist)
> +                        current_track = impl->tracks().get().begin();
> +
> +                if (current_track == impl->tracks().get().end())
> +                    current_track = empty_iterator;
> +                else
> +                    next = *current_track;
> +            }
> +        } else if (current_track != empty_iterator) {
> +            next = *current_track;
> +        }
> +
>          impl->remove_track(track);
>  
> +        if (not next.empty()) {
> +            current_track = find(impl->tracks().get().begin(), impl->tracks().get().end(), next);
> +
> +            if (deleting_current) {
> +                const bool toggle_player_state = true;
> +                impl->go_to(next, toggle_player_state);
> +            } else {
> +                // To force an index update in the client
> +                impl->on_track_changed()(next);

This is kind of a work around for something that qtubuntu-media should do. I will try to get this done there.

> +            }
> +        }
> +
>          auto reply = dbus::Message::make_method_return(msg);
>          bus->send(reply);
>      }


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



More information about the Ubuntu-reviews mailing list