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

Proposed by Alfonso Sanchez-Beato
Status: Merged
Approved by: Jim Hodapp
Approved revision: 168
Merged at revision: 163
Proposed branch: lp:~phablet-team/media-hub/bg-playlist-fixes
Merge into: lp:media-hub/stable
Prerequisite: lp:~morphis/media-hub/previous-to-same-after-four-secs
Diff against target: 703 lines (+237/-42)
11 files modified
include/core/media/track_list.h (+8/-0)
src/core/media/mpris/service.h (+12/-0)
src/core/media/mpris/track_list.h (+17/-0)
src/core/media/player_implementation.cpp (+12/-0)
src/core/media/service_skeleton.cpp (+22/-6)
src/core/media/track_list.cpp (+5/-0)
src/core/media/track_list_implementation.cpp (+25/-18)
src/core/media/track_list_skeleton.cpp (+102/-16)
src/core/media/track_list_skeleton.h (+4/-0)
src/core/media/track_list_stub.cpp (+29/-2)
src/core/media/track_list_stub.h (+1/-0)
To merge this branch: bzr merge lp:~phablet-team/media-hub/bg-playlist-fixes
Reviewer Review Type Date Requested Status
Jim Hodapp (community) code Approve
Review via email: mp+276094@code.launchpad.net

Commit message

Make sure our iterator for the current track points to the right place when (un)shuffling (LP #1510219). Fix crash when client tries to set the player for a non-existing key. Do not add empty URIs to the list (LP: #1511029).

Description of the change

Make sure our iterator for the current track points to the right place when (un)shuffling (LP #1510219). Fix crash when client tries to set the player for a non-existing key. Do not add empty URIs to the list (LP: #1511029).

To post a comment you must log in.
165. By Alfonso Sanchez-Beato

Do not add empty URIs to the list (LP: #1511029)

166. By Alfonso Sanchez-Beato

Send TrackChanged signals only when the TrackAdded signal for the
corresponding track has already been sent.

Revision history for this message
Jim Hodapp (jhodapp) wrote :

A few comments inline.

review: Needs Fixing (code)
167. By Alfonso Sanchez-Beato

Address review comments

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

Branch refreshed after addressing comments.

Revision history for this message
Jim Hodapp (jhodapp) wrote :

Looks good, one minor change still needed.

review: Needs Fixing
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

Branch refreshed after addressing comment.

168. By Alfonso Sanchez-Beato

Address review comment

Revision history for this message
Jim Hodapp (jhodapp) wrote :

LGTM

review: Approve (code)
169. By Alfonso Sanchez-Beato

Repeat current track when already played more than five seconds

170. By Alfonso Sanchez-Beato

Fix for the repeat track feature

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :
171. By Alfonso Sanchez-Beato

Add TrackListReset signal, which is emitted when the Reset DBus method is
called.

172. By Alfonso Sanchez-Beato

Update properly playlist iterator when adding or removing tracks
(LP: #1511073).

173. By Alfonso Sanchez-Beato

Use get/set_current track when shuffling

174. By Alfonso Sanchez-Beato

Force index update when changing the track list

Revision history for this message
Jim Hodapp (jhodapp) wrote :

A few comments below about the latest changes.

review: Needs Fixing
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

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

175. By Alfonso Sanchez-Beato

Address review comments

Revision history for this message
Jim Hodapp (jhodapp) wrote :

Several comments inline below.

review: Needs Fixing (code)
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

See comments below. I will address the remaining issues asap.

176. By Alfonso Sanchez-Beato

Address review comments

Revision history for this message
Jim Hodapp (jhodapp) wrote :

A couple of comments below.

Revision history for this message
Jim Hodapp (jhodapp) wrote :

LGTM

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'include/core/media/track_list.h'
--- include/core/media/track_list.h 2015-10-23 14:39:34 +0000
+++ include/core/media/track_list.h 2015-11-13 15:27:09 +0000
@@ -54,6 +54,11 @@
54 {54 {
55 InsufficientPermissionsToAddTrack();55 InsufficientPermissionsToAddTrack();
56 };56 };
57
58 struct TrackNotFound : public std::runtime_error
59 {
60 TrackNotFound();
61 };
57 };62 };
5863
59 static const Track::Id& after_empty_track();64 static const Track::Id& after_empty_track();
@@ -123,6 +128,9 @@
123 /** Indicates that a track has been removed from the track list. */128 /** Indicates that a track has been removed from the track list. */
124 virtual const core::Signal<Track::Id>& on_track_removed() const = 0;129 virtual const core::Signal<Track::Id>& on_track_removed() const = 0;
125130
131 /** Indicates that the track list has been reset and there are no tracks now */
132 virtual const core::Signal<void>& on_track_list_reset() const = 0;
133
126 /** Indicates that the track list advanced from one track to another. */134 /** Indicates that the track list advanced from one track to another. */
127 virtual const core::Signal<Track::Id>& on_track_changed() const = 0;135 virtual const core::Signal<Track::Id>& on_track_changed() const = 0;
128136
129137
=== modified file 'src/core/media/mpris/service.h'
--- src/core/media/mpris/service.h 2015-09-08 20:03:56 +0000
+++ src/core/media/mpris/service.h 2015-11-13 15:27:09 +0000
@@ -107,6 +107,18 @@
107 return s;107 return s;
108 }108 }
109 };109 };
110
111 struct PlayerKeyNotFound
112 {
113 static const std::string& name()
114 {
115 static const std::string s
116 {
117 "core.ubuntu.media.Service.Error.PlayerKeyNotFound"
118 };
119 return s;
120 }
121 };
110 };122 };
111123
112 DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(CreateSession, Service, 1000)124 DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(CreateSession, Service, 1000)
113125
=== modified file 'src/core/media/mpris/track_list.h'
--- src/core/media/mpris/track_list.h 2015-10-23 14:39:34 +0000
+++ src/core/media/mpris/track_list.h 2015-11-13 15:27:09 +0000
@@ -57,6 +57,14 @@
57 "mpris.TrackList.Error.InsufficientPermissionsToAddTrack"57 "mpris.TrackList.Error.InsufficientPermissionsToAddTrack"
58 };58 };
59 };59 };
60
61 struct TrackNotFound
62 {
63 static constexpr const char* name
64 {
65 "mpris.TrackList.Error.TrackNotFound"
66 };
67 };
60 };68 };
6169
62 DBUS_CPP_METHOD_DEF(GetTracksMetadata, TrackList)70 DBUS_CPP_METHOD_DEF(GetTracksMetadata, TrackList)
@@ -108,6 +116,13 @@
108116
109 DBUS_CPP_SIGNAL_DEF117 DBUS_CPP_SIGNAL_DEF
110 (118 (
119 TrackListReset,
120 TrackList,
121 void
122 )
123
124 DBUS_CPP_SIGNAL_DEF
125 (
111 TrackMetadataChanged,126 TrackMetadataChanged,
112 TrackList,127 TrackList,
113 BOOST_IDENTITY_TYPE((std::tuple<std::map<std::string, dbus::types::Variant>, dbus::types::ObjectPath>))128 BOOST_IDENTITY_TYPE((std::tuple<std::map<std::string, dbus::types::Variant>, dbus::types::ObjectPath>))
@@ -156,6 +171,7 @@
156 configuration.object->template get_signal<Signals::TracksAdded>(),171 configuration.object->template get_signal<Signals::TracksAdded>(),
157 configuration.object->template get_signal<Signals::TrackRemoved>(),172 configuration.object->template get_signal<Signals::TrackRemoved>(),
158 configuration.object->template get_signal<Signals::TrackChanged>(),173 configuration.object->template get_signal<Signals::TrackChanged>(),
174 configuration.object->template get_signal<Signals::TrackListReset>(),
159 configuration.object->template get_signal<Signals::TrackMetadataChanged>(),175 configuration.object->template get_signal<Signals::TrackMetadataChanged>(),
160 configuration.object->template get_signal<core::dbus::interfaces::Properties::Signals::PropertiesChanged>()176 configuration.object->template get_signal<core::dbus::interfaces::Properties::Signals::PropertiesChanged>()
161 }177 }
@@ -201,6 +217,7 @@
201 core::dbus::Signal<Signals::TracksAdded, Signals::TracksAdded::ArgumentType>::Ptr tracks_added;217 core::dbus::Signal<Signals::TracksAdded, Signals::TracksAdded::ArgumentType>::Ptr tracks_added;
202 core::dbus::Signal<Signals::TrackRemoved, Signals::TrackRemoved::ArgumentType>::Ptr track_removed;218 core::dbus::Signal<Signals::TrackRemoved, Signals::TrackRemoved::ArgumentType>::Ptr track_removed;
203 core::dbus::Signal<Signals::TrackChanged, Signals::TrackChanged::ArgumentType>::Ptr track_changed;219 core::dbus::Signal<Signals::TrackChanged, Signals::TrackChanged::ArgumentType>::Ptr track_changed;
220 core::dbus::Signal<Signals::TrackListReset, Signals::TrackListReset::ArgumentType>::Ptr track_list_reset;
204 core::dbus::Signal<Signals::TrackMetadataChanged, Signals::TrackMetadataChanged::ArgumentType>::Ptr track_metadata_changed;221 core::dbus::Signal<Signals::TrackMetadataChanged, Signals::TrackMetadataChanged::ArgumentType>::Ptr track_metadata_changed;
205222
206 dbus::Signal <core::dbus::interfaces::Properties::Signals::PropertiesChanged,223 dbus::Signal <core::dbus::interfaces::Properties::Signals::PropertiesChanged,
207224
=== modified file 'src/core/media/player_implementation.cpp'
--- src/core/media/player_implementation.cpp 2015-11-13 15:27:09 +0000
+++ src/core/media/player_implementation.cpp 2015-11-13 15:27:09 +0000
@@ -569,6 +569,11 @@
569 d->update_mpris_properties();569 d->update_mpris_properties();
570 });570 });
571571
572 d->track_list->on_track_list_reset().connect([this](void)
573 {
574 d->update_mpris_properties();
575 });
576
572 d->track_list->on_track_changed().connect([this](const media::Track::Id&)577 d->track_list->on_track_changed().connect([this](const media::Track::Id&)
573 {578 {
574 d->update_mpris_properties();579 d->update_mpris_properties();
@@ -681,6 +686,13 @@
681bool media::PlayerImplementation<Parent>::open_uri(const Track::UriType& uri)686bool media::PlayerImplementation<Parent>::open_uri(const Track::UriType& uri)
682{687{
683 d->track_list->reset();688 d->track_list->reset();
689
690 // If empty uri, give the same meaning as QMediaPlayer::setMedia("")
691 if (uri.empty()) {
692 cout << __PRETTY_FUNCTION__ << ": resetting current media" << endl;
693 return true;
694 }
695
684 const bool ret = d->engine->open_resource_for_uri(uri, false);696 const bool ret = d->engine->open_resource_for_uri(uri, false);
685 // Don't set new track as the current track to play since we're calling open_resource_for_uri above697 // Don't set new track as the current track to play since we're calling open_resource_for_uri above
686 static const bool make_current = false;698 static const bool make_current = false;
687699
=== modified file 'src/core/media/service_skeleton.cpp'
--- src/core/media/service_skeleton.cpp 2015-10-14 20:54:19 +0000
+++ src/core/media/service_skeleton.cpp 2015-11-13 15:27:09 +0000
@@ -46,6 +46,8 @@
46namespace dbus = core::dbus;46namespace dbus = core::dbus;
47namespace media = core::ubuntu::media;47namespace media = core::ubuntu::media;
4848
49using namespace std;
50
49namespace51namespace
50{52{
51core::Signal<void> the_empty_signal;53core::Signal<void> the_empty_signal;
@@ -132,7 +134,9 @@
132 impl->access_service()->add_object_for_path(op)134 impl->access_service()->add_object_for_path(op)
133 };135 };
134136
135 std::cout << "Session created by request of: " << msg->sender() << ", uuid: " << uuid << ", path:" << op << std::endl;137 cout << "Session created by request of: " << msg->sender()
138 << ", key: " << key << ", uuid: " << uuid
139 << ", path:" << op << std::endl;
136140
137 try141 try
138 {142 {
@@ -205,9 +209,11 @@
205 std::string uuid;209 std::string uuid;
206 msg->reader() >> uuid;210 msg->reader() >> uuid;
207211
208 if (uuid_player_map.count(uuid) != 0) {212 if (uuid_player_map.count(uuid) != 0)
213 {
209 auto key = uuid_player_map.at(uuid);214 auto key = uuid_player_map.at(uuid);
210 if (not configuration.player_store->has_player_for_key(key)) {215 if (not configuration.player_store->has_player_for_key(key))
216 {
211 auto reply = dbus::Message::make_error(217 auto reply = dbus::Message::make_error(
212 msg,218 msg,
213 mpris::Service::Errors::ReattachingSession::name(),219 mpris::Service::Errors::ReattachingSession::name(),
@@ -442,9 +448,19 @@
442 {448 {
443 Player::PlayerKey key;449 Player::PlayerKey key;
444 msg->reader() >> key;450 msg->reader() >> key;
445 impl->set_current_player(key);451
446452 core::dbus::Message::Ptr reply;
447 auto reply = dbus::Message::make_method_return(msg);453 if (not configuration.player_store->has_player_for_key(key)) {
454 cerr << __PRETTY_FUNCTION__ << " player key not found - " << key << endl;
455 reply = dbus::Message::make_error(
456 msg,
457 mpris::Service::Errors::PlayerKeyNotFound::name(),
458 "Player key not found");
459 } else {
460 impl->set_current_player(key);
461 reply = dbus::Message::make_method_return(msg);
462 }
463
448 impl->access_bus()->send(reply);464 impl->access_bus()->send(reply);
449 }465 }
450466
451467
=== modified file 'src/core/media/track_list.cpp'
--- src/core/media/track_list.cpp 2015-10-23 14:39:34 +0000
+++ src/core/media/track_list.cpp 2015-11-13 15:27:09 +0000
@@ -25,6 +25,11 @@
25{25{
26}26}
2727
28media::TrackList::Errors::TrackNotFound::TrackNotFound()
29 : std::runtime_error{"Track not found in TrackList"}
30{
31}
32
28const media::Track::Id& media::TrackList::after_empty_track()33const media::Track::Id& media::TrackList::after_empty_track()
29{34{
30 static const media::Track::Id id{"/org/mpris/MediaPlayer2/TrackList/NoTrack"};35 static const media::Track::Id id{"/org/mpris/MediaPlayer2/TrackList/NoTrack"};
3136
=== modified file 'src/core/media/track_list_implementation.cpp'
--- src/core/media/track_list_implementation.cpp 2015-10-23 14:39:34 +0000
+++ src/core/media/track_list_implementation.cpp 2015-11-13 15:27:09 +0000
@@ -118,6 +118,8 @@
118 std::cout << "Adding Track::Id: " << id << std::endl;118 std::cout << "Adding Track::Id: " << id << std::endl;
119 std::cout << "\tURI: " << uri << std::endl;119 std::cout << "\tURI: " << uri << std::endl;
120120
121 const auto current = get_current_track();
122
121 auto result = tracks().update([this, id, position, make_current](TrackList::Container& container)123 auto result = tracks().update([this, id, position, make_current](TrackList::Container& container)
122 {124 {
123 auto it = std::find(container.begin(), container.end(), position);125 auto it = std::find(container.begin(), container.end(), position);
@@ -133,20 +135,24 @@
133135
134 if (make_current)136 if (make_current)
135 {137 {
138 set_current_track(id);
136 // Don't automatically call stop() and play() in player_implementation.cpp on_go_to_track()139 // Don't automatically call stop() and play() in player_implementation.cpp on_go_to_track()
137 // since this breaks video playback when using open_uri() (stop() and play() are unwanted in140 // since this breaks video playback when using open_uri() (stop() and play() are unwanted in
138 // this scenario since the qtubuntu-media will handle this automatically)141 // this scenario since the qtubuntu-media will handle this automatically)
139 const bool toggle_player_state = false;142 const bool toggle_player_state = false;
140 go_to(id, toggle_player_state);143 go_to(id, toggle_player_state);
144 } else {
145 set_current_track(current);
141 }146 }
142147
143 // Signal to the client that the current track has changed for the first track added to the TrackList
144 if (tracks().get().size() == 1)
145 on_track_changed()(id);
146
147 std::cout << "Signaling that we just added track id: " << id << std::endl;148 std::cout << "Signaling that we just added track id: " << id << std::endl;
148 // Signal to the client that a track was added to the TrackList149 // Signal to the client that a track was added to the TrackList
149 on_track_added()(id);150 on_track_added()(id);
151
152 // Signal to the client that the current track has changed for the first
153 // track added to the TrackList
154 if (tracks().get().size() == 1)
155 on_track_changed()(id);
150 }156 }
151}157}
152158
@@ -154,6 +160,9 @@
154{160{
155 std::cout << __PRETTY_FUNCTION__ << std::endl;161 std::cout << __PRETTY_FUNCTION__ << std::endl;
156162
163 const auto current = get_current_track();
164
165 Track::Id current_id;
157 ContainerURI tmp;166 ContainerURI tmp;
158 for (const auto uri : uris)167 for (const auto uri : uris)
159 {168 {
@@ -185,12 +194,17 @@
185194
186 // Signal to the client that the current track has changed for the first track added to the TrackList195 // Signal to the client that the current track has changed for the first track added to the TrackList
187 if (tracks().get().size() == 1)196 if (tracks().get().size() == 1)
188 on_track_changed()(id);197 current_id = id;
189 }198 }
190 }199 }
191200
201 set_current_track(current);
202
192 std::cout << "Signaling that we just added " << tmp.size() << " tracks to the TrackList" << std::endl;203 std::cout << "Signaling that we just added " << tmp.size() << " tracks to the TrackList" << std::endl;
193 on_tracks_added()(tmp);204 on_tracks_added()(tmp);
205
206 if (!current_id.empty())
207 on_track_changed()(current_id);
194}208}
195209
196void media::TrackListImplementation::remove_track(const media::Track::Id& id)210void media::TrackListImplementation::remove_track(const media::Track::Id& id)
@@ -209,7 +223,6 @@
209223
210 on_track_removed()(id);224 on_track_removed()(id);
211 }225 }
212
213}226}
214227
215void media::TrackListImplementation::go_to(const media::Track::Id& track, bool toggle_player_state)228void media::TrackListImplementation::go_to(const media::Track::Id& track, bool toggle_player_state)
@@ -266,26 +279,20 @@
266279
267void media::TrackListImplementation::reset()280void media::TrackListImplementation::reset()
268{281{
282 std::cout << __PRETTY_FUNCTION__ << std::endl;
283
269 // Make sure playback stops284 // Make sure playback stops
270 on_end_of_tracklist()();285 on_end_of_tracklist()();
271 // And make sure there is no "current" track286 // And make sure there is no "current" track
272 media::TrackListSkeleton::reset();287 media::TrackListSkeleton::reset();
273288
274 auto result = tracks().update([this](TrackList::Container& container)289 tracks().update([this](TrackList::Container& container)
275 {290 {
276 TrackList::Container ids = container;291 container.clear();
277292 on_track_list_reset()();
278 for (auto it = container.begin(); it != container.end(); ) {293
279 auto id = *it;
280 it = container.erase(it);
281 on_track_removed()(id);
282 }
283
284 container.resize(0);
285 d->track_counter = 0;294 d->track_counter = 0;
286295
287 return true;296 return true;
288 });297 });
289
290 (void) result;
291}298}
292299
=== modified file 'src/core/media/track_list_skeleton.cpp'
--- src/core/media/track_list_skeleton.cpp 2015-11-13 15:27:09 +0000
+++ src/core/media/track_list_skeleton.cpp 2015-11-13 15:27:09 +0000
@@ -38,10 +38,14 @@
3838
39#include <iostream>39#include <iostream>
40#include <limits>40#include <limits>
41#include <sstream>
42#include <cstdint>
4143
42namespace dbus = core::dbus;44namespace dbus = core::dbus;
43namespace media = core::ubuntu::media;45namespace media = core::ubuntu::media;
4446
47using namespace std;
48
45struct media::TrackListSkeleton::Private49struct media::TrackListSkeleton::Private
46{50{
47 Private(media::TrackListSkeleton* impl, const dbus::Bus::Ptr& bus, const dbus::Object::Ptr& object,51 Private(media::TrackListSkeleton* impl, const dbus::Bus::Ptr& bus, const dbus::Object::Ptr& object,
@@ -63,6 +67,7 @@
63 skeleton.signals.tracks_added,67 skeleton.signals.tracks_added,
64 skeleton.signals.track_removed,68 skeleton.signals.track_removed,
65 skeleton.signals.track_changed,69 skeleton.signals.track_changed,
70 skeleton.signals.track_list_reset,
66 skeleton.signals.tracklist_replaced71 skeleton.signals.tracklist_replaced
67 }72 }
68 {73 {
@@ -176,8 +181,53 @@
176 media::Track::Id track;181 media::Track::Id track;
177 msg->reader() >> track;182 msg->reader() >> track;
178183
184 auto id_it = find(impl->tracks().get().begin(), impl->tracks().get().end(), track);
185 if (id_it == impl->tracks().get().end()) {
186 ostringstream err_str;
187 err_str << "Track " << track << " not found in track list";
188 cout << __PRETTY_FUNCTION__ << " WARNING " << err_str.str() << endl;
189 auto reply = dbus::Message::make_error(
190 msg,
191 mpris::TrackList::Error::TrackNotFound::name,
192 err_str.str());
193 bus->send(reply);
194 return;
195 }
196
197 media::Track::Id next;
198 bool deleting_current = false;
199
200 if (id_it == impl->current_iterator()) {
201 cout << "Removing current track" << endl;
202 deleting_current = true;
203
204 if (current_track != empty_iterator) {
205 ++current_track;
206
207 if (current_track == impl->tracks().get().end()
208 && loop_status == media::Player::LoopStatus::playlist)
209 current_track = impl->tracks().get().begin();
210
211 if (current_track == impl->tracks().get().end())
212 current_track = empty_iterator;
213 else
214 next = *current_track;
215 }
216 } else if (current_track != empty_iterator) {
217 next = *current_track;
218 }
219
179 impl->remove_track(track);220 impl->remove_track(track);
180221
222 if (not next.empty()) {
223 current_track = find(impl->tracks().get().begin(), impl->tracks().get().end(), next);
224
225 if (deleting_current) {
226 const bool toggle_player_state = true;
227 impl->go_to(next, toggle_player_state);
228 }
229 }
230
181 auto reply = dbus::Message::make_method_return(msg);231 auto reply = dbus::Message::make_method_return(msg);
182 bus->send(reply);232 bus->send(reply);
183 }233 }
@@ -221,12 +271,17 @@
221 typedef core::dbus::Signal<mpris::TrackList::Signals::TracksAdded, mpris::TrackList::Signals::TracksAdded::ArgumentType> DBusTracksAddedSignal;271 typedef core::dbus::Signal<mpris::TrackList::Signals::TracksAdded, mpris::TrackList::Signals::TracksAdded::ArgumentType> DBusTracksAddedSignal;
222 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackRemoved, mpris::TrackList::Signals::TrackRemoved::ArgumentType> DBusTrackRemovedSignal;272 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackRemoved, mpris::TrackList::Signals::TrackRemoved::ArgumentType> DBusTrackRemovedSignal;
223 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackChanged, mpris::TrackList::Signals::TrackChanged::ArgumentType> DBusTrackChangedSignal;273 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackChanged, mpris::TrackList::Signals::TrackChanged::ArgumentType> DBusTrackChangedSignal;
274 typedef core::dbus::Signal<
275 mpris::TrackList::Signals::TrackListReset,
276 mpris::TrackList::Signals::TrackListReset::ArgumentType>
277 DBusTrackListResetSignal;
224 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackListReplaced, mpris::TrackList::Signals::TrackListReplaced::ArgumentType> DBusTrackListReplacedSignal;278 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackListReplaced, mpris::TrackList::Signals::TrackListReplaced::ArgumentType> DBusTrackListReplacedSignal;
225279
226 Signals(const std::shared_ptr<DBusTrackAddedSignal>& remote_track_added,280 Signals(const std::shared_ptr<DBusTrackAddedSignal>& remote_track_added,
227 const std::shared_ptr<DBusTracksAddedSignal>& remote_tracks_added,281 const std::shared_ptr<DBusTracksAddedSignal>& remote_tracks_added,
228 const std::shared_ptr<DBusTrackRemovedSignal>& remote_track_removed,282 const std::shared_ptr<DBusTrackRemovedSignal>& remote_track_removed,
229 const std::shared_ptr<DBusTrackChangedSignal>& remote_track_changed,283 const std::shared_ptr<DBusTrackChangedSignal>& remote_track_changed,
284 const std::shared_ptr<DBusTrackListResetSignal>& remote_track_list_reset,
230 const std::shared_ptr<DBusTrackListReplacedSignal>& remote_track_list_replaced)285 const std::shared_ptr<DBusTrackListReplacedSignal>& remote_track_list_replaced)
231 {286 {
232 // Connect all of the MPRIS interface signals to be emitted over dbus287 // Connect all of the MPRIS interface signals to be emitted over dbus
@@ -245,6 +300,11 @@
245 remote_track_removed->emit(id);300 remote_track_removed->emit(id);
246 });301 });
247302
303 on_track_list_reset.connect([remote_track_list_reset]()
304 {
305 remote_track_list_reset->emit();
306 });
307
248 on_track_changed.connect([remote_track_changed](const media::Track::Id &id)308 on_track_changed.connect([remote_track_changed](const media::Track::Id &id)
249 {309 {
250 remote_track_changed->emit(id);310 remote_track_changed->emit(id);
@@ -259,6 +319,7 @@
259 core::Signal<Track::Id> on_track_added;319 core::Signal<Track::Id> on_track_added;
260 core::Signal<TrackList::ContainerURI> on_tracks_added;320 core::Signal<TrackList::ContainerURI> on_tracks_added;
261 core::Signal<Track::Id> on_track_removed;321 core::Signal<Track::Id> on_track_removed;
322 core::Signal<void> on_track_list_reset;
262 core::Signal<Track::Id> on_track_changed;323 core::Signal<Track::Id> on_track_changed;
263 core::Signal<TrackList::ContainerTrackIdTuple> on_track_list_replaced;324 core::Signal<TrackList::ContainerTrackIdTuple> on_track_list_replaced;
264 core::Signal<std::pair<Track::Id, bool>> on_go_to_track;325 core::Signal<std::pair<Track::Id, bool>> on_go_to_track;
@@ -401,6 +462,7 @@
401462
402 if (do_go_to_next_track)463 if (do_go_to_next_track)
403 {464 {
465 cout << "next track id is " << *(current_iterator()) << endl;
404 on_track_changed()(*(current_iterator()));466 on_track_changed()(*(current_iterator()));
405 // Don't automatically call stop() and play() in player_implementation.cpp on_go_to_track()467 // Don't automatically call stop() and play() in player_implementation.cpp on_go_to_track()
406 // since this breaks video playback when using open_uri() (stop() and play() are unwanted in468 // since this breaks video playback when using open_uri() (stop() and play() are unwanted in
@@ -425,15 +487,15 @@
425 }487 }
426488
427 bool do_go_to_previous_track = false;489 bool do_go_to_previous_track = false;
428 bool repeat_current_track = false;490 // Position is measured in nanoseconds
429 const uint64_t max_position = 4 * 1000;491 const uint64_t max_position = 5 * UINT64_C(1000000000);
430492
431 // If we're playing the current already for some time we will493 // If we're playing the current track for > max_position time then
432 // repeat it from the beginning494 // repeat it from the beginning
433 if (d->current_position > max_position)495 if (d->current_position > max_position)
434 {496 {
435 std::cout << "Repeating current track..." << std::endl;497 std::cout << "Repeating current track..." << std::endl;
436 repeat_current_track = true;498 do_go_to_previous_track = true;
437 }499 }
438 // Loop on the current track forever500 // Loop on the current track forever
439 else if (d->loop_status == media::Player::LoopStatus::track)501 else if (d->loop_status == media::Player::LoopStatus::track)
@@ -465,7 +527,7 @@
465 }527 }
466 }528 }
467529
468 if (do_go_to_previous_track && !repeat_current_track)530 if (do_go_to_previous_track)
469 {531 {
470 on_track_changed()(*(current_iterator()));532 on_track_changed()(*(current_iterator()));
471 // Don't automatically call stop() and play() in player_implementation.cpp on_go_to_track()533 // Don't automatically call stop() and play() in player_implementation.cpp on_go_to_track()
@@ -510,6 +572,21 @@
510 d->current_track = d->empty_iterator;572 d->current_track = d->empty_iterator;
511}573}
512574
575media::Track::Id media::TrackListSkeleton::get_current_track(void)
576{
577 if (d->current_track == d->empty_iterator || tracks().get().empty())
578 return media::Track::Id{};
579
580 return *(current_iterator());
581}
582
583void media::TrackListSkeleton::set_current_track(const media::Track::Id& id)
584{
585 const auto id_it = find(tracks().get().begin(), tracks().get().end(), id);
586 if (id_it != tracks().get().end())
587 d->current_track = id_it;
588}
589
513const core::Property<bool>& media::TrackListSkeleton::can_edit_tracks() const590const core::Property<bool>& media::TrackListSkeleton::can_edit_tracks() const
514{591{
515 return *d->skeleton.properties.can_edit_tracks;592 return *d->skeleton.properties.can_edit_tracks;
@@ -527,7 +604,6 @@
527604
528void media::TrackListSkeleton::on_position_changed(uint64_t position)605void media::TrackListSkeleton::on_position_changed(uint64_t position)
529{606{
530 std::cout << "TrackListSkeleton::on_position_changed: position = " << position << std::endl;
531 d->current_position = position;607 d->current_position = position;
532}608}
533609
@@ -543,21 +619,21 @@
543619
544void media::TrackListSkeleton::on_shuffle_changed(bool shuffle)620void media::TrackListSkeleton::on_shuffle_changed(bool shuffle)
545{621{
622 if (tracks().get().empty())
623 return;
624
625 const auto current_id = get_current_track();
626
627 cout << __PRETTY_FUNCTION__ << " " << shuffle
628 << " current track: " << current_id << endl;
629
546 if (shuffle)630 if (shuffle)
547 shuffle_tracks();631 shuffle_tracks();
548 else632 else
549 {
550 // Save the current Track::Id of what's currently playing to restore after unshuffle
551 const media::Track::Id current_id = *(current_iterator());
552
553 unshuffle_tracks();633 unshuffle_tracks();
554634
555 // Since we use assign() in unshuffle_tracks, which invalidates existing iterators, we need635 // Shuffling and unshuffling invalidates iterators, so we re-create current_track
556 // to make sure that current is pointing to the right place636 set_current_track(current_id);
557 auto it = std::find(tracks().get().begin(), tracks().get().end(), current_id);
558 if (it != tracks().get().end())
559 d->current_track = it;
560 }
561}637}
562638
563const core::Property<media::TrackList::Container>& media::TrackListSkeleton::tracks() const639const core::Property<media::TrackList::Container>& media::TrackListSkeleton::tracks() const
@@ -587,6 +663,11 @@
587 return d->signals.on_track_removed;663 return d->signals.on_track_removed;
588}664}
589665
666const core::Signal<void>& media::TrackListSkeleton::on_track_list_reset() const
667{
668 return d->signals.on_track_list_reset;
669}
670
590const core::Signal<media::Track::Id>& media::TrackListSkeleton::on_track_changed() const671const core::Signal<media::Track::Id>& media::TrackListSkeleton::on_track_changed() const
591{672{
592 return d->signals.on_track_changed;673 return d->signals.on_track_changed;
@@ -622,6 +703,11 @@
622 return d->signals.on_track_removed;703 return d->signals.on_track_removed;
623}704}
624705
706core::Signal<void>& media::TrackListSkeleton::on_track_list_reset()
707{
708 return d->signals.on_track_list_reset;
709}
710
625core::Signal<media::Track::Id>& media::TrackListSkeleton::on_track_changed()711core::Signal<media::Track::Id>& media::TrackListSkeleton::on_track_changed()
626{712{
627 return d->signals.on_track_changed;713 return d->signals.on_track_changed;
628714
=== modified file 'src/core/media/track_list_skeleton.h'
--- src/core/media/track_list_skeleton.h 2015-11-13 15:27:09 +0000
+++ src/core/media/track_list_skeleton.h 2015-11-13 15:27:09 +0000
@@ -57,6 +57,7 @@
57 const core::Signal<ContainerURI>& on_tracks_added() const;57 const core::Signal<ContainerURI>& on_tracks_added() const;
58 core::Signal<ContainerURI>& on_tracks_added();58 core::Signal<ContainerURI>& on_tracks_added();
59 const core::Signal<Track::Id>& on_track_removed() const;59 const core::Signal<Track::Id>& on_track_removed() const;
60 const core::Signal<void>& on_track_list_reset() const;
60 const core::Signal<Track::Id>& on_track_changed() const;61 const core::Signal<Track::Id>& on_track_changed() const;
61 core::Signal<Track::Id>& on_track_changed();62 core::Signal<Track::Id>& on_track_changed();
62 const core::Signal<std::pair<Track::Id, bool>>& on_go_to_track() const;63 const core::Signal<std::pair<Track::Id, bool>>& on_go_to_track() const;
@@ -64,6 +65,7 @@
64 const core::Signal<void>& on_end_of_tracklist() const;65 const core::Signal<void>& on_end_of_tracklist() const;
65 core::Signal<void>& on_end_of_tracklist();66 core::Signal<void>& on_end_of_tracklist();
66 core::Signal<Track::Id>& on_track_removed();67 core::Signal<Track::Id>& on_track_removed();
68 core::Signal<void>& on_track_list_reset();
6769
68 core::Property<Container>& tracks();70 core::Property<Container>& tracks();
69 void on_loop_status_changed(const core::ubuntu::media::Player::LoopStatus& loop_status);71 void on_loop_status_changed(const core::ubuntu::media::Player::LoopStatus& loop_status);
@@ -80,6 +82,8 @@
80 inline bool is_last_track(const ConstIterator &it);82 inline bool is_last_track(const ConstIterator &it);
81 inline const TrackList::ConstIterator& current_iterator();83 inline const TrackList::ConstIterator& current_iterator();
82 void reset_current_iterator_if_needed();84 void reset_current_iterator_if_needed();
85 media::Track::Id get_current_track(void);
86 void set_current_track(const media::Track::Id& id);
8387
84 core::Property<bool>& can_edit_tracks();88 core::Property<bool>& can_edit_tracks();
8589
8690
=== modified file 'src/core/media/track_list_stub.cpp'
--- src/core/media/track_list_stub.cpp 2015-10-23 14:39:34 +0000
+++ src/core/media/track_list_stub.cpp 2015-11-13 15:27:09 +0000
@@ -54,6 +54,7 @@
54 object->get_signal<mpris::TrackList::Signals::TrackAdded>(),54 object->get_signal<mpris::TrackList::Signals::TrackAdded>(),
55 object->get_signal<mpris::TrackList::Signals::TracksAdded>(),55 object->get_signal<mpris::TrackList::Signals::TracksAdded>(),
56 object->get_signal<mpris::TrackList::Signals::TrackRemoved>(),56 object->get_signal<mpris::TrackList::Signals::TrackRemoved>(),
57 object->get_signal<mpris::TrackList::Signals::TrackListReset>(),
57 object->get_signal<mpris::TrackList::Signals::TrackListReplaced>(),58 object->get_signal<mpris::TrackList::Signals::TrackListReplaced>(),
58 object->get_signal<mpris::TrackList::Signals::TrackChanged>()59 object->get_signal<mpris::TrackList::Signals::TrackChanged>()
59 }60 }
@@ -73,17 +74,23 @@
73 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackAdded, mpris::TrackList::Signals::TrackAdded::ArgumentType> DBusTrackAddedSignal;74 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackAdded, mpris::TrackList::Signals::TrackAdded::ArgumentType> DBusTrackAddedSignal;
74 typedef core::dbus::Signal<mpris::TrackList::Signals::TracksAdded, mpris::TrackList::Signals::TracksAdded::ArgumentType> DBusTracksAddedSignal;75 typedef core::dbus::Signal<mpris::TrackList::Signals::TracksAdded, mpris::TrackList::Signals::TracksAdded::ArgumentType> DBusTracksAddedSignal;
75 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackRemoved, mpris::TrackList::Signals::TrackRemoved::ArgumentType> DBusTrackRemovedSignal;76 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackRemoved, mpris::TrackList::Signals::TrackRemoved::ArgumentType> DBusTrackRemovedSignal;
77 typedef core::dbus::Signal<
78 mpris::TrackList::Signals::TrackListReset,
79 mpris::TrackList::Signals::TrackListReset::ArgumentType>
80 DBusTrackListResetSignal;
76 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackListReplaced, mpris::TrackList::Signals::TrackListReplaced::ArgumentType> DBusTrackListReplacedSignal;81 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackListReplaced, mpris::TrackList::Signals::TrackListReplaced::ArgumentType> DBusTrackListReplacedSignal;
77 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackChanged, mpris::TrackList::Signals::TrackChanged::ArgumentType> DBusTrackChangedSignal;82 typedef core::dbus::Signal<mpris::TrackList::Signals::TrackChanged, mpris::TrackList::Signals::TrackChanged::ArgumentType> DBusTrackChangedSignal;
7883
79 Signals(const std::shared_ptr<DBusTrackAddedSignal>& track_added,84 Signals(const std::shared_ptr<DBusTrackAddedSignal>& track_added,
80 const std::shared_ptr<DBusTracksAddedSignal>& tracks_added,85 const std::shared_ptr<DBusTracksAddedSignal>& tracks_added,
81 const std::shared_ptr<DBusTrackRemovedSignal>& track_removed,86 const std::shared_ptr<DBusTrackRemovedSignal>& track_removed,
87 const std::shared_ptr<DBusTrackListResetSignal>& track_list_reset,
82 const std::shared_ptr<DBusTrackListReplacedSignal>& track_list_replaced,88 const std::shared_ptr<DBusTrackListReplacedSignal>& track_list_replaced,
83 const std::shared_ptr<DBusTrackChangedSignal>& track_changed)89 const std::shared_ptr<DBusTrackChangedSignal>& track_changed)
84 : on_track_added(),90 : on_track_added(),
85 on_tracks_added(),91 on_tracks_added(),
86 on_track_removed(),92 on_track_removed(),
93 on_track_list_reset(),
87 on_track_list_replaced(),94 on_track_list_replaced(),
88 on_track_changed(),95 on_track_changed(),
89 dbus96 dbus
@@ -91,6 +98,7 @@
91 track_added,98 track_added,
92 tracks_added,99 tracks_added,
93 track_removed,100 track_removed,
101 track_list_reset,
94 track_list_replaced,102 track_list_replaced,
95 track_changed,103 track_changed,
96 }104 }
@@ -113,9 +121,15 @@
113 on_track_removed(id);121 on_track_removed(id);
114 });122 });
115123
124 dbus.on_track_list_reset->connect([this](void)
125 {
126 std::cout << "OnTrackListReset signal arrived via the bus." << std::endl;
127 on_track_list_reset();
128 });
129
116 dbus.on_track_list_replaced->connect([this](const media::TrackList::ContainerTrackIdTuple& list)130 dbus.on_track_list_replaced->connect([this](const media::TrackList::ContainerTrackIdTuple& list)
117 {131 {
118 std::cout << "OnTrackListRemoved signal arrived via the bus." << std::endl;132 std::cout << "OnTrackListReplaced signal arrived via the bus." << std::endl;
119 on_track_list_replaced(list);133 on_track_list_replaced(list);
120 });134 });
121135
@@ -129,6 +143,7 @@
129 core::Signal<Track::Id> on_track_added;143 core::Signal<Track::Id> on_track_added;
130 core::Signal<media::TrackList::ContainerURI> on_tracks_added;144 core::Signal<media::TrackList::ContainerURI> on_tracks_added;
131 core::Signal<Track::Id> on_track_removed;145 core::Signal<Track::Id> on_track_removed;
146 core::Signal<void> on_track_list_reset;
132 core::Signal<media::TrackList::ContainerTrackIdTuple> on_track_list_replaced;147 core::Signal<media::TrackList::ContainerTrackIdTuple> on_track_list_replaced;
133 core::Signal<Track::Id> on_track_changed;148 core::Signal<Track::Id> on_track_changed;
134 core::Signal<std::pair<Track::Id, bool>> on_go_to_track;149 core::Signal<std::pair<Track::Id, bool>> on_go_to_track;
@@ -139,6 +154,7 @@
139 std::shared_ptr<DBusTrackAddedSignal> on_track_added;154 std::shared_ptr<DBusTrackAddedSignal> on_track_added;
140 std::shared_ptr<DBusTracksAddedSignal> on_tracks_added;155 std::shared_ptr<DBusTracksAddedSignal> on_tracks_added;
141 std::shared_ptr<DBusTrackRemovedSignal> on_track_removed;156 std::shared_ptr<DBusTrackRemovedSignal> on_track_removed;
157 std::shared_ptr<DBusTrackListResetSignal> on_track_list_reset;
142 std::shared_ptr<DBusTrackListReplacedSignal> on_track_list_replaced;158 std::shared_ptr<DBusTrackListReplacedSignal> on_track_list_replaced;
143 std::shared_ptr<DBusTrackChangedSignal> on_track_changed;159 std::shared_ptr<DBusTrackChangedSignal> on_track_changed;
144 } dbus;160 } dbus;
@@ -237,7 +253,13 @@
237 track);253 track);
238254
239 if (op.is_error())255 if (op.is_error())
240 throw std::runtime_error("Problem removing track: " + op.error());256 {
257 if (op.error().name() ==
258 mpris::TrackList::Error::TrackNotFound::name)
259 throw media::TrackList::Errors::TrackNotFound{};
260 else
261 throw std::runtime_error{"Problem removing track: " + op.error().print()};
262 }
241}263}
242264
243void media::TrackListStub::go_to(const media::Track::Id& track, bool toggle_player_state)265void media::TrackListStub::go_to(const media::Track::Id& track, bool toggle_player_state)
@@ -300,6 +322,11 @@
300 return d->signals.on_track_removed;322 return d->signals.on_track_removed;
301}323}
302324
325const core::Signal<void>& media::TrackListStub::on_track_list_reset() const
326{
327 return d->signals.on_track_list_reset;
328}
329
303const core::Signal<media::Track::Id>& media::TrackListStub::on_track_changed() const330const core::Signal<media::Track::Id>& media::TrackListStub::on_track_changed() const
304{331{
305 return d->signals.on_track_changed;332 return d->signals.on_track_changed;
306333
=== modified file 'src/core/media/track_list_stub.h'
--- src/core/media/track_list_stub.h 2015-10-21 19:40:20 +0000
+++ src/core/media/track_list_stub.h 2015-11-13 15:27:09 +0000
@@ -65,6 +65,7 @@
65 const core::Signal<Track::Id>& on_track_added() const;65 const core::Signal<Track::Id>& on_track_added() const;
66 const core::Signal<ContainerURI>& on_tracks_added() const;66 const core::Signal<ContainerURI>& on_tracks_added() const;
67 const core::Signal<Track::Id>& on_track_removed() const;67 const core::Signal<Track::Id>& on_track_removed() const;
68 const core::Signal<void>& on_track_list_reset() const;
68 const core::Signal<Track::Id>& on_track_changed() const;69 const core::Signal<Track::Id>& on_track_changed() const;
69 const core::Signal<std::pair<Track::Id, bool>>& on_go_to_track() const;70 const core::Signal<std::pair<Track::Id, bool>>& on_go_to_track() const;
70 const core::Signal<void>& on_end_of_tracklist() const;71 const core::Signal<void>& on_end_of_tracklist() const;

Subscribers

People subscribed via source and target branches

to all changes: