Merge lp:~phablet-team/media-hub/fix-1538703-take2 into lp:media-hub
- fix-1538703-take2
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Alfonso Sanchez-Beato |
Approved revision: | 200 |
Merged at revision: | 180 |
Proposed branch: | lp:~phablet-team/media-hub/fix-1538703-take2 |
Merge into: | lp:media-hub |
Diff against target: |
702 lines (+205/-95) 16 files modified
CMakeLists.txt (+2/-2) debian/changelog (+7/-0) debian/libmedia-hub-doc.install (+1/-1) doc/CMakeLists.txt (+2/-2) include/core/media/service.h (+0/-3) src/core/media/hashed_keyed_player_store.cpp (+1/-0) src/core/media/mpris/service.h (+0/-1) src/core/media/player_configuration.h (+3/-0) src/core/media/player_implementation.cpp (+86/-6) src/core/media/player_skeleton.h (+2/-0) src/core/media/service_implementation.cpp (+5/-10) src/core/media/service_implementation.h (+0/-1) src/core/media/service_skeleton.cpp (+94/-59) src/core/media/service_skeleton.h (+2/-0) src/core/media/service_stub.cpp (+0/-9) src/core/media/service_stub.h (+0/-1) |
To merge this branch: | bzr merge lp:~phablet-team/media-hub/fix-1538703-take2 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alfonso Sanchez-Beato | Approve | ||
PS Jenkins bot | continuous-integration | Approve | |
Konrad Zapałowicz (community) | code | Approve | |
Review via email: mp+292584@code.launchpad.net |
Commit message
A rewrite of how the current player is set which is what the MPRIS control interface actively uses.
Description of the change
A rewrite of how the current player is set which is what the MPRIS control interface actively uses.
Konrad Zapałowicz (kzapalowicz) wrote : | # |
Jim Hodapp (jhodapp) : | # |
- 199. By Jim Hodapp
-
Make sure to check if player_service is not nullptr before using it in is_current_player()
Konrad Zapałowicz (kzapalowicz) wrote : | # |
LGTM
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:198
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:199
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote : | # |
Looks good, just curious why you had to remove the version dependency for libdbus-cpp-dev ?
- 200. By Jim Hodapp
-
Restore the libdbus-cpp-dev min version check
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote : | # |
LGTM
Preview Diff
1 | === modified file 'CMakeLists.txt' |
2 | --- CMakeLists.txt 2016-04-04 18:54:45 +0000 |
3 | +++ CMakeLists.txt 2016-05-04 13:11:44 +0000 |
4 | @@ -20,11 +20,11 @@ |
5 | # we define the version to be 5.0.0 |
6 | if (${DISTRO_CODENAME} STREQUAL "vivid") |
7 | set(UBUNTU_MEDIA_HUB_VERSION_MAJOR 4) |
8 | - set(UBUNTU_MEDIA_HUB_VERSION_MINOR 1) |
9 | + set(UBUNTU_MEDIA_HUB_VERSION_MINOR 2) |
10 | set(UBUNTU_MEDIA_HUB_VERSION_PATCH 0) |
11 | else () |
12 | set(UBUNTU_MEDIA_HUB_VERSION_MAJOR 5) |
13 | - set(UBUNTU_MEDIA_HUB_VERSION_MINOR 0) |
14 | + set(UBUNTU_MEDIA_HUB_VERSION_MINOR 1) |
15 | set(UBUNTU_MEDIA_HUB_VERSION_PATCH 0) |
16 | endif() |
17 | endif() |
18 | |
19 | === modified file 'debian/changelog' |
20 | --- debian/changelog 2016-04-07 02:00:40 +0000 |
21 | +++ debian/changelog 2016-05-04 13:11:44 +0000 |
22 | @@ -1,3 +1,10 @@ |
23 | +media-hub (4.2.0+16.04.20160407.1-0ubuntu1) UNRELEASED; urgency=medium |
24 | + |
25 | + * Improve the MPRIS player control interface so that it only |
26 | + controls the last Player instance that called play(). |
27 | + |
28 | + -- Jim Hodapp <jim.hodapp@canonical.com> Tue, 03 May 2016 11:32:14 -0400 |
29 | + |
30 | media-hub (4.1.0+16.04.20160407.1-0ubuntu1) xenial; urgency=medium |
31 | |
32 | * Add a proper logger to media-hub that includes traces, timestamps |
33 | |
34 | === modified file 'debian/libmedia-hub-doc.install' |
35 | --- debian/libmedia-hub-doc.install 2014-03-06 22:51:51 +0000 |
36 | +++ debian/libmedia-hub-doc.install 2016-05-04 13:11:44 +0000 |
37 | @@ -1,1 +1,1 @@ |
38 | -usr/share/doc/music-hub/html |
39 | +usr/share/doc/media-hub/html |
40 | |
41 | === modified file 'doc/CMakeLists.txt' |
42 | --- doc/CMakeLists.txt 2013-08-14 18:05:23 +0000 |
43 | +++ doc/CMakeLists.txt 2016-05-04 13:11:44 +0000 |
44 | @@ -26,5 +26,5 @@ |
45 | COMMENT "Generating API documentation with Doxygen" VERBATIM) |
46 | install( |
47 | DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/html |
48 | - DESTINATION share/doc/music-hub) |
49 | -endif(DOXYGEN_FOUND) |
50 | \ No newline at end of file |
51 | + DESTINATION share/doc/media-hub) |
52 | +endif(DOXYGEN_FOUND) |
53 | |
54 | === modified file 'include/core/media/service.h' |
55 | --- include/core/media/service.h 2015-09-08 20:03:56 +0000 |
56 | +++ include/core/media/service.h 2016-05-04 13:11:44 +0000 |
57 | @@ -60,9 +60,6 @@ |
58 | /** @brief Resumes a fixed-name session directly by player key. */ |
59 | virtual std::shared_ptr<Player> resume_session(Player::PlayerKey) = 0; |
60 | |
61 | - /** @brief Sets the current player that the MPRIS interface will control */ |
62 | - virtual void set_current_player(Player::PlayerKey) = 0; |
63 | - |
64 | /** @brief Pauses sessions other than the supplied one. */ |
65 | virtual void pause_other_sessions(Player::PlayerKey) = 0; |
66 | |
67 | |
68 | === modified file 'src/core/media/hashed_keyed_player_store.cpp' |
69 | --- src/core/media/hashed_keyed_player_store.cpp 2016-03-07 14:51:01 +0000 |
70 | +++ src/core/media/hashed_keyed_player_store.cpp 2016-05-04 13:11:44 +0000 |
71 | @@ -64,6 +64,7 @@ |
72 | void media::HashedKeyedPlayerStore::remove_player_for_key(const media::Player::PlayerKey& key) |
73 | { |
74 | std::lock_guard<std::recursive_mutex> lg{guard}; |
75 | + |
76 | auto it = map.find(key); |
77 | if (it != map.end()) |
78 | { |
79 | |
80 | === modified file 'src/core/media/mpris/service.h' |
81 | --- src/core/media/mpris/service.h 2016-01-04 08:30:35 +0000 |
82 | +++ src/core/media/mpris/service.h 2016-05-04 13:11:44 +0000 |
83 | @@ -127,7 +127,6 @@ |
84 | DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(DestroySession, Service, 1000) |
85 | DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(CreateFixedSession, Service, 1000) |
86 | DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(ResumeSession, Service, 1000) |
87 | - DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(SetCurrentPlayer, Service, 1000) |
88 | DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(PauseOtherSessions, Service, 1000) |
89 | }; |
90 | } |
91 | |
92 | === modified file 'src/core/media/player_configuration.h' |
93 | --- src/core/media/player_configuration.h 2015-04-17 16:44:30 +0000 |
94 | +++ src/core/media/player_configuration.h 2016-05-04 13:11:44 +0000 |
95 | @@ -19,6 +19,7 @@ |
96 | #define CORE_UBUNTU_MEDIA_PLAYER_CLIENT_CONFIGURATION_H_ |
97 | |
98 | #include <core/media/player.h> |
99 | +#include <core/media/service.h> |
100 | |
101 | #include <core/dbus/bus.h> |
102 | #include <core/dbus/object.h> |
103 | @@ -35,6 +36,8 @@ |
104 | std::shared_ptr<core::dbus::Service> service; |
105 | // The actual session object representing a player instance. |
106 | std::shared_ptr<core::dbus::Object> session; |
107 | + // The Service instance that manages Player instances |
108 | + core::ubuntu::media::Service* player_service; |
109 | }; |
110 | |
111 | #endif // CORE_UBUNTU_MEDIA_PLAYER_CLIENT_CONFIGURATION_H_ |
112 | |
113 | === modified file 'src/core/media/player_implementation.cpp' |
114 | --- src/core/media/player_implementation.cpp 2016-04-06 15:28:29 +0000 |
115 | +++ src/core/media/player_implementation.cpp 2016-05-04 13:11:44 +0000 |
116 | @@ -17,7 +17,10 @@ |
117 | * Jim Hodapp <jim.hodapp@canonical.com> |
118 | */ |
119 | |
120 | +#include <core/media/service.h> |
121 | + |
122 | #include "player_implementation.h" |
123 | +#include "service_skeleton.h" |
124 | #include "util/timeout.h" |
125 | |
126 | #include <unistd.h> |
127 | @@ -301,14 +304,14 @@ |
128 | } |
129 | } |
130 | |
131 | - void update_mpris_properties(void) |
132 | + void update_mpris_properties() |
133 | { |
134 | - bool has_previous = track_list->has_previous() |
135 | + const bool has_previous = track_list->has_previous() |
136 | or parent->Parent::loop_status() != Player::LoopStatus::none; |
137 | - bool has_next = track_list->has_next() |
138 | + const bool has_next = track_list->has_next() |
139 | or parent->Parent::loop_status() != Player::LoopStatus::none; |
140 | - auto n_tracks = track_list->tracks()->size(); |
141 | - bool has_tracks = (n_tracks > 0) ? true : false; |
142 | + const auto n_tracks = track_list->tracks()->size(); |
143 | + const bool has_tracks = (n_tracks > 0) ? true : false; |
144 | |
145 | MH_INFO("Updating MPRIS TrackList properties:"); |
146 | MH_INFO("\tTracks: %d", n_tracks); |
147 | @@ -322,6 +325,58 @@ |
148 | parent->can_go_next().set(has_next); |
149 | } |
150 | |
151 | + bool pause_other_players(media::Player::PlayerKey key) |
152 | + { |
153 | + if (not config.parent.player_service) |
154 | + return false; |
155 | + |
156 | + media::ServiceSkeleton* skel { |
157 | + reinterpret_cast<media::ServiceSkeleton*>(config.parent.player_service) |
158 | + }; |
159 | + skel->pause_other_sessions(key); |
160 | + return true; |
161 | + } |
162 | + |
163 | + bool update_current_player(media::Player::PlayerKey key) |
164 | + { |
165 | + if (not config.parent.player_service) |
166 | + return false; |
167 | + |
168 | + media::ServiceSkeleton* skel { |
169 | + reinterpret_cast<media::ServiceSkeleton*>(config.parent.player_service) |
170 | + }; |
171 | + skel->set_current_player(key); |
172 | + return true; |
173 | + } |
174 | + |
175 | + bool is_current_player() const |
176 | + { |
177 | + if (not config.parent.player_service) |
178 | + return false; |
179 | + |
180 | + media::ServiceSkeleton* skel { |
181 | + reinterpret_cast<media::ServiceSkeleton*>(config.parent.player_service) |
182 | + }; |
183 | + return skel->is_current_player(parent->key()); |
184 | + } |
185 | + |
186 | + bool is_multimedia_role() const |
187 | + { |
188 | + return parent->audio_stream_role() == media::Player::AudioStreamRole::multimedia; |
189 | + } |
190 | + |
191 | + bool reset_current_player() |
192 | + { |
193 | + if (not config.parent.player_service) |
194 | + return false; |
195 | + |
196 | + media::ServiceSkeleton* skel { |
197 | + reinterpret_cast<media::ServiceSkeleton*>(config.parent.player_service) |
198 | + }; |
199 | + skel->reset_current_player(); |
200 | + return true; |
201 | + } |
202 | + |
203 | // Our link back to our parent. |
204 | media::PlayerImplementation<Parent>* parent; |
205 | // We just store the parameters passed on construction. |
206 | @@ -478,6 +533,16 @@ |
207 | // are cleared |
208 | d->clear_wakelocks(); |
209 | d->track_list->reset(); |
210 | + |
211 | + // This is not a fatal error but merely a warning that should |
212 | + // be logged |
213 | + if (d->is_multimedia_role() and d->is_current_player()) |
214 | + { |
215 | + MH_DEBUG("==== Resetting current player"); |
216 | + if (not d->reset_current_player()) |
217 | + MH_WARNING("Failed to reset current player"); |
218 | + } |
219 | + |
220 | // And tell the outside world that the client has gone away |
221 | d->on_client_disconnected(); |
222 | }); |
223 | @@ -687,7 +752,8 @@ |
224 | d->track_list->reset(); |
225 | |
226 | // If empty uri, give the same meaning as QMediaPlayer::setMedia("") |
227 | - if (uri.empty()) { |
228 | + if (uri.empty()) |
229 | + { |
230 | MH_DEBUG("Resetting current media"); |
231 | return true; |
232 | } |
233 | @@ -697,6 +763,7 @@ |
234 | // Don't set new track as the current track to play since we're calling open_resource_for_uri above |
235 | static const bool make_current = false; |
236 | d->track_list->add_track_with_uri_at(uri, media::TrackList::after_empty_track(), make_current); |
237 | + |
238 | return ret; |
239 | } |
240 | |
241 | @@ -722,6 +789,19 @@ |
242 | void media::PlayerImplementation<Parent>::play() |
243 | { |
244 | MH_TRACE(""); |
245 | + if (d->is_multimedia_role()) |
246 | + { |
247 | + MH_DEBUG("==== Pausing all other multimedia player sessions"); |
248 | + if (not d->pause_other_players(d->config.key)) |
249 | + MH_WARNING("Failed to pause other player sessions"); |
250 | + |
251 | + MH_DEBUG("==== Updating the current player"); |
252 | + // This player will begin playing so make sure it's the current player. If |
253 | + // this operation fails it is not a fatal condition but should be logged |
254 | + if (not d->update_current_player(d->config.key)) |
255 | + MH_WARNING("Failed to update current player"); |
256 | + } |
257 | + |
258 | d->engine->play(); |
259 | } |
260 | |
261 | |
262 | === modified file 'src/core/media/player_skeleton.h' |
263 | --- src/core/media/player_skeleton.h 2015-04-17 15:13:56 +0000 |
264 | +++ src/core/media/player_skeleton.h 2016-05-04 13:11:44 +0000 |
265 | @@ -57,6 +57,8 @@ |
266 | std::shared_ptr<core::dbus::Service> service; |
267 | // The session object that we want to expose the skeleton upon. |
268 | std::shared_ptr<core::dbus::Object> session; |
269 | + // The Service instance that manages Player instances |
270 | + core::ubuntu::media::Service* player_service; |
271 | // Our functional dependencies. |
272 | apparmor::ubuntu::RequestContextResolver::Ptr request_context_resolver; |
273 | apparmor::ubuntu::RequestAuthenticator::Ptr request_authenticator; |
274 | |
275 | === modified file 'src/core/media/service_implementation.cpp' |
276 | --- src/core/media/service_implementation.cpp 2016-04-05 19:27:50 +0000 |
277 | +++ src/core/media/service_implementation.cpp 2016-05-04 13:11:44 +0000 |
278 | @@ -186,11 +186,13 @@ |
279 | auto player = std::make_shared<media::PlayerImplementation<media::PlayerSkeleton>> |
280 | (media::PlayerImplementation<media::PlayerSkeleton>::Configuration |
281 | { |
282 | + // Derive a PlayerSkeleton-specific Configuration based on Player::Configuration |
283 | media::PlayerSkeleton::Configuration |
284 | { |
285 | conf.bus, |
286 | conf.service, |
287 | conf.session, |
288 | + conf.player_service, |
289 | d->request_context_resolver, |
290 | d->request_authenticator |
291 | }, |
292 | @@ -258,11 +260,6 @@ |
293 | return std::shared_ptr<media::Player>(); |
294 | } |
295 | |
296 | -void media::ServiceImplementation::set_current_player(Player::PlayerKey) |
297 | -{ |
298 | - // no impl |
299 | -} |
300 | - |
301 | void media::ServiceImplementation::pause_other_sessions(media::Player::PlayerKey key) |
302 | { |
303 | MH_TRACE(""); |
304 | @@ -276,11 +273,9 @@ |
305 | const std::shared_ptr<media::Player> current_player = |
306 | d->configuration.player_store->player_for_key(key); |
307 | |
308 | - // We immediately make the player known as new current player. |
309 | - if (current_player->audio_stream_role() == media::Player::multimedia) |
310 | - d->configuration.player_store->set_current_player_for_key(key); |
311 | - |
312 | - d->configuration.player_store->enumerate_players([current_player, key](const media::Player::PlayerKey& other_key, const std::shared_ptr<media::Player>& other_player) |
313 | + d->configuration.player_store->enumerate_players([current_player, key] |
314 | + (const media::Player::PlayerKey& other_key, |
315 | + const std::shared_ptr<media::Player>& other_player) |
316 | { |
317 | // Only pause a Player if all of the following criteria are met: |
318 | // 1) currently playing |
319 | |
320 | === modified file 'src/core/media/service_implementation.h' |
321 | --- src/core/media/service_implementation.h 2015-09-08 20:03:56 +0000 |
322 | +++ src/core/media/service_implementation.h 2016-05-04 13:11:44 +0000 |
323 | @@ -49,7 +49,6 @@ |
324 | void destroy_session(const std::string&, const Player::Configuration&); |
325 | std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&); |
326 | std::shared_ptr<Player> resume_session(Player::PlayerKey key); |
327 | - void set_current_player(Player::PlayerKey key); |
328 | void pause_other_sessions(Player::PlayerKey key); |
329 | |
330 | private: |
331 | |
332 | === modified file 'src/core/media/service_skeleton.cpp' |
333 | --- src/core/media/service_skeleton.cpp 2016-04-06 15:28:29 +0000 |
334 | +++ src/core/media/service_skeleton.cpp 2016-05-04 13:11:44 +0000 |
335 | @@ -63,7 +63,7 @@ |
336 | object(impl->access_service()->add_object_for_path( |
337 | dbus::traits::Service<media::Service>::object_path())), |
338 | configuration(config), |
339 | - exported(impl->access_bus(), config.cover_art_resolver, impl) |
340 | + exported(impl->access_bus(), config.cover_art_resolver, impl, configuration) |
341 | { |
342 | object->install_method_handler<mpris::Service::CreateSession>( |
343 | std::bind( |
344 | @@ -95,11 +95,6 @@ |
345 | &Private::handle_resume_session, |
346 | this, |
347 | std::placeholders::_1)); |
348 | - object->install_method_handler<mpris::Service::SetCurrentPlayer>( |
349 | - std::bind( |
350 | - &Private::handle_set_current_player, |
351 | - this, |
352 | - std::placeholders::_1)); |
353 | object->install_method_handler<mpris::Service::PauseOtherSessions>( |
354 | std::bind( |
355 | &Private::handle_pause_other_sessions, |
356 | @@ -133,7 +128,8 @@ |
357 | key, |
358 | impl->access_bus(), |
359 | impl->access_service(), |
360 | - impl->access_service()->add_object_for_path(op) |
361 | + impl->access_service()->add_object_for_path(op), |
362 | + impl |
363 | }; |
364 | |
365 | MH_DEBUG("Session created by request of: %s, key: %d, uuid: %d, path: %s", |
366 | @@ -241,12 +237,6 @@ |
367 | // Signal player reconnection |
368 | auto player = configuration.player_store->player_for_key(key); |
369 | player->reconnect(); |
370 | - // We only care to allow the MPRIS controls to apply to multimedia player (i.e. audio, video) |
371 | - if (player->audio_stream_role() == media::Player::AudioStreamRole::multimedia) |
372 | - { |
373 | - MH_TRACE("Setting current_player"); |
374 | - exported.set_current_player(player); |
375 | - } |
376 | |
377 | auto reply = dbus::Message::make_method_return(msg); |
378 | reply->writer() << op; |
379 | @@ -369,7 +359,8 @@ |
380 | key, |
381 | impl->access_bus(), |
382 | impl->access_service(), |
383 | - impl->access_service()->add_object_for_path(op) |
384 | + impl->access_service()->add_object_for_path(op), |
385 | + impl |
386 | }; |
387 | |
388 | auto session = impl->create_session(config); |
389 | @@ -543,7 +534,7 @@ |
390 | } |
391 | |
392 | explicit Exported(const dbus::Bus::Ptr& bus, const media::CoverArtResolver& cover_art_resolver, |
393 | - media::ServiceSkeleton* impl) |
394 | + media::ServiceSkeleton* impl, const ServiceSkeleton::Configuration& config) |
395 | : bus{bus}, |
396 | /* Export MediaHub service interface on dbus */ |
397 | service{dbus::Service::add_service(bus, "org.mpris.MediaPlayer2.MediaHub")}, |
398 | @@ -552,7 +543,8 @@ |
399 | player{mpris::Player::Skeleton::Configuration{bus, object, player_defaults()}}, |
400 | playlists{mpris::Playlists::Skeleton::Configuration{bus, object, mpris::Playlists::Skeleton::Configuration::Defaults{}}}, |
401 | cover_art_resolver{cover_art_resolver}, |
402 | - impl{impl} |
403 | + impl{impl}, |
404 | + service_skel_config(config) |
405 | { |
406 | object->install_method_handler<core::dbus::interfaces::Properties::GetAll>([this](const core::dbus::Message::Ptr& msg) |
407 | { |
408 | @@ -574,7 +566,7 @@ |
409 | // Setup method handlers for mpris::Player methods. |
410 | auto next = [this](const core::dbus::Message::Ptr& msg) |
411 | { |
412 | - const auto sp = current_player.lock(); |
413 | + const auto sp = service_skel_config.player_store->current_player().get(); |
414 | |
415 | if (is_multimedia_role()) |
416 | sp->next(); |
417 | @@ -585,7 +577,7 @@ |
418 | |
419 | auto previous = [this](const core::dbus::Message::Ptr& msg) |
420 | { |
421 | - const auto sp = current_player.lock(); |
422 | + const auto sp = service_skel_config.player_store->current_player().get(); |
423 | |
424 | if (is_multimedia_role()) |
425 | sp->previous(); |
426 | @@ -596,7 +588,7 @@ |
427 | |
428 | auto pause = [this](const core::dbus::Message::Ptr& msg) |
429 | { |
430 | - const auto sp = current_player.lock(); |
431 | + const auto sp = service_skel_config.player_store->current_player().get(); |
432 | |
433 | if (is_multimedia_role() and sp->can_pause()) |
434 | sp->pause(); |
435 | @@ -607,7 +599,7 @@ |
436 | |
437 | auto stop = [this](const core::dbus::Message::Ptr& msg) |
438 | { |
439 | - const auto sp = current_player.lock(); |
440 | + const auto sp = service_skel_config.player_store->current_player().get(); |
441 | |
442 | if (is_multimedia_role()) |
443 | sp->stop(); |
444 | @@ -618,7 +610,7 @@ |
445 | |
446 | auto play = [this, impl](const core::dbus::Message::Ptr& msg) |
447 | { |
448 | - const auto sp = current_player.lock(); |
449 | + const auto sp = service_skel_config.player_store->current_player().get(); |
450 | |
451 | if (is_multimedia_role() and sp->can_play()) |
452 | { |
453 | @@ -636,7 +628,7 @@ |
454 | |
455 | auto play_pause = [this, impl](const core::dbus::Message::Ptr& msg) |
456 | { |
457 | - const auto sp = current_player.lock(); |
458 | + const auto sp = service_skel_config.player_store->current_player().get(); |
459 | |
460 | if (is_multimedia_role()) |
461 | { |
462 | @@ -662,67 +654,79 @@ |
463 | |
464 | inline bool is_multimedia_role() |
465 | { |
466 | - const auto sp = current_player.lock(); |
467 | + MH_TRACE(""); |
468 | |
469 | + const auto sp = service_skel_config.player_store->current_player().get(); |
470 | return (sp ? sp->audio_stream_role() == media::Player::AudioStreamRole::multimedia : false); |
471 | } |
472 | |
473 | - void set_current_player(const std::shared_ptr<media::Player>& cp) |
474 | + void set_current_player(media::Player::PlayerKey key) |
475 | { |
476 | MH_TRACE(""); |
477 | - // We will not keep the object alive. |
478 | - current_player = cp; |
479 | + |
480 | + // Update the current player in the Player store |
481 | + service_skel_config.player_store->set_current_player_for_key(key); |
482 | + const auto player_sp = service_skel_config.player_store->current_player().get(); |
483 | |
484 | // And announce that we can be controlled again. |
485 | player.properties.can_control->set(true); |
486 | |
487 | // We wire up player state changes |
488 | - connections.seeked_to = cp->seeked_to().connect([this](std::uint64_t position) |
489 | + connections.seeked_to = player_sp->seeked_to().connect([this](std::uint64_t position) |
490 | { |
491 | player.signals.seeked_to->emit(position); |
492 | }); |
493 | |
494 | - connections.duration_changed = cp->duration().changed().connect([this](std::uint64_t duration) |
495 | + connections.duration_changed = player_sp->duration().changed().connect([this](std::uint64_t duration) |
496 | { |
497 | player.properties.duration->set(duration); |
498 | }); |
499 | |
500 | - connections.position_changed = cp->position().changed().connect([this](std::uint64_t position) |
501 | + connections.position_changed = player_sp->position().changed().connect([this](std::uint64_t position) |
502 | { |
503 | player.properties.position->set(position); |
504 | }); |
505 | |
506 | - connections.playback_status_changed = cp->playback_status().changed().connect( |
507 | - [this](core::ubuntu::media::Player::PlaybackStatus status) |
508 | + connections.playback_status_changed = player_sp->playback_status().changed().connect( |
509 | + [this, key, player_sp](core::ubuntu::media::Player::PlaybackStatus status) |
510 | { |
511 | - player.properties.playback_status->set(mpris::Player::PlaybackStatus::from(status)); |
512 | + const auto cp = service_skel_config.player_store->current_player().get(); |
513 | + // If key points to the current player's key, then update status |
514 | + if (cp and key == cp->key()) |
515 | + player.properties.playback_status->set(mpris::Player::PlaybackStatus::from(status)); |
516 | }); |
517 | |
518 | - connections.loop_status_changed = cp->loop_status().changed().connect( |
519 | + connections.loop_status_changed = player_sp->loop_status().changed().connect( |
520 | [this](core::ubuntu::media::Player::LoopStatus status) |
521 | { |
522 | player.properties.loop_status->set(mpris::Player::LoopStatus::from(status)); |
523 | }); |
524 | |
525 | - connections.can_play_changed = cp->can_play().changed().connect( |
526 | - [this](bool can_play) |
527 | - { |
528 | - player.properties.can_play->set(can_play); |
529 | - }); |
530 | - |
531 | - connections.can_pause_changed = cp->can_pause().changed().connect( |
532 | - [this](bool can_pause) |
533 | - { |
534 | - player.properties.can_pause->set(can_pause); |
535 | - }); |
536 | - |
537 | - connections.can_go_previous_changed = cp->can_go_previous().changed().connect( |
538 | + connections.can_play_changed = player_sp->can_play().changed().connect( |
539 | + [this, key, player_sp](bool can_play) |
540 | + { |
541 | + const auto cp = service_skel_config.player_store->current_player().get(); |
542 | + // If key points to the current player's key, then update can_play |
543 | + if (cp and key == cp->key()) |
544 | + player.properties.can_play->set(can_play); |
545 | + }); |
546 | + |
547 | + connections.can_pause_changed = player_sp->can_pause().changed().connect( |
548 | + [this, key, player_sp](bool can_pause) |
549 | + { |
550 | + const auto cp = service_skel_config.player_store->current_player().get(); |
551 | + // If key points to the current player's key, then update can_pause |
552 | + if (cp and key == cp->key()) |
553 | + player.properties.can_pause->set(can_pause); |
554 | + }); |
555 | + |
556 | + connections.can_go_previous_changed = player_sp->can_go_previous().changed().connect( |
557 | [this](bool can_go_previous) |
558 | { |
559 | player.properties.can_go_previous->set(can_go_previous); |
560 | }); |
561 | |
562 | - connections.can_go_next_changed = cp->can_go_next().changed().connect( |
563 | + connections.can_go_next_changed = player_sp->can_go_next().changed().connect( |
564 | [this](bool can_go_next) |
565 | { |
566 | player.properties.can_go_next->set(can_go_next); |
567 | @@ -734,16 +738,16 @@ |
568 | // different DBus object paths, /core/ubuntu/media/Service/sessions/<n> |
569 | // and /org/mpris/MediaPlayer2 (this is the one enforced by the MPRIS spec). |
570 | // Discuss why this is needed with tvoss. |
571 | - player.properties.duration->set(cp->duration().get()); |
572 | - player.properties.position->set(cp->position().get()); |
573 | + player.properties.duration->set(player_sp->duration().get()); |
574 | + player.properties.position->set(player_sp->position().get()); |
575 | player.properties.playback_status->set(mpris::Player::PlaybackStatus::from( |
576 | - cp->playback_status().get())); |
577 | + player_sp->playback_status().get())); |
578 | player.properties.loop_status->set(mpris::Player::LoopStatus::from( |
579 | - cp->loop_status().get())); |
580 | - player.properties.can_play->set(cp->can_play().get()); |
581 | - player.properties.can_pause->set(cp->can_pause().get()); |
582 | - player.properties.can_go_previous->set(cp->can_go_previous().get()); |
583 | - player.properties.can_go_next->set(cp->can_go_next().get()); |
584 | + player_sp->loop_status().get())); |
585 | + player.properties.can_play->set(player_sp->can_play().get()); |
586 | + player.properties.can_pause->set(player_sp->can_pause().get()); |
587 | + player.properties.can_go_previous->set(player_sp->can_go_previous().get()); |
588 | + player.properties.can_go_next->set(player_sp->can_go_next().get()); |
589 | |
590 | #if 0 |
591 | // TODO cover_art_resolver() is not implemented yet |
592 | @@ -788,7 +792,27 @@ |
593 | MH_TRACE(""); |
594 | // And announce that we can no longer be controlled. |
595 | player.properties.can_control->set(false); |
596 | - current_player.reset(); |
597 | + player.properties.can_play->set(false); |
598 | + player.properties.can_pause->set(false); |
599 | + player.properties.can_go_previous->set(false); |
600 | + player.properties.can_go_next->set(false); |
601 | + |
602 | + // Reset to null event connections |
603 | + connections.seeked_to = the_empty_signal.connect([](){}); |
604 | + connections.duration_changed = the_empty_signal.connect([](){}); |
605 | + connections.position_changed = the_empty_signal.connect([](){}); |
606 | + connections.playback_status_changed = the_empty_signal.connect([](){}); |
607 | + connections.loop_status_changed = the_empty_signal.connect([](){}); |
608 | + connections.can_play_changed = the_empty_signal.connect([](){}); |
609 | + connections.can_pause_changed = the_empty_signal.connect([](){}); |
610 | + connections.can_go_previous_changed = the_empty_signal.connect([](){}); |
611 | + connections.can_go_next_changed = the_empty_signal.connect([](){}); |
612 | + } |
613 | + |
614 | + bool is_current_player(media::Player::PlayerKey key) |
615 | + |
616 | + { |
617 | + return key == service_skel_config.player_store->current_player().get()->key(); |
618 | } |
619 | |
620 | dbus::Bus::Ptr bus; |
621 | @@ -801,10 +825,9 @@ |
622 | |
623 | // The CoverArtResolver used by the exported player. |
624 | media::CoverArtResolver cover_art_resolver; |
625 | - // The actual player instance. |
626 | - std::weak_ptr<media::Player> current_player; |
627 | |
628 | media::ServiceSkeleton* impl; |
629 | + ServiceSkeleton::Configuration service_skel_config; |
630 | |
631 | // We track event connections. |
632 | struct |
633 | @@ -899,11 +922,23 @@ |
634 | d->configuration.player_store->player_for_key(key); |
635 | // We only care to allow the MPRIS controls to apply to multimedia player (i.e. audio, video) |
636 | if (player->audio_stream_role() == media::Player::AudioStreamRole::multimedia) |
637 | - d->exported.set_current_player(player); |
638 | + d->exported.set_current_player(key); |
639 | +} |
640 | + |
641 | +bool media::ServiceSkeleton::is_current_player(media::Player::PlayerKey key) const |
642 | +{ |
643 | + return d->exported.is_current_player(key); |
644 | +} |
645 | + |
646 | +void media::ServiceSkeleton::reset_current_player() |
647 | +{ |
648 | + MH_TRACE(""); |
649 | + d->exported.reset_current_player(); |
650 | } |
651 | |
652 | void media::ServiceSkeleton::pause_other_sessions(media::Player::PlayerKey key) |
653 | { |
654 | + MH_TRACE(""); |
655 | d->configuration.impl->pause_other_sessions(key); |
656 | } |
657 | |
658 | |
659 | === modified file 'src/core/media/service_skeleton.h' |
660 | --- src/core/media/service_skeleton.h 2015-09-08 20:03:56 +0000 |
661 | +++ src/core/media/service_skeleton.h 2016-05-04 13:11:44 +0000 |
662 | @@ -60,6 +60,8 @@ |
663 | std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&); |
664 | std::shared_ptr<Player> resume_session(Player::PlayerKey); |
665 | void set_current_player(Player::PlayerKey key); |
666 | + bool is_current_player(Player::PlayerKey key) const; |
667 | + void reset_current_player(); |
668 | void pause_other_sessions(Player::PlayerKey key); |
669 | |
670 | void run(); |
671 | |
672 | === modified file 'src/core/media/service_stub.cpp' |
673 | --- src/core/media/service_stub.cpp 2015-09-08 20:03:56 +0000 |
674 | +++ src/core/media/service_stub.cpp 2016-05-04 13:11:44 +0000 |
675 | @@ -139,15 +139,6 @@ |
676 | }); |
677 | } |
678 | |
679 | -void media::ServiceStub::set_current_player(Player::PlayerKey key) |
680 | -{ |
681 | - auto op = d->object->invoke_method_synchronously<mpris::Service::SetCurrentPlayer, |
682 | - void>(key); |
683 | - |
684 | - if (op.is_error()) |
685 | - throw std::runtime_error("Problem setting current player: " + op.error()); |
686 | -} |
687 | - |
688 | void media::ServiceStub::pause_other_sessions(media::Player::PlayerKey key) |
689 | { |
690 | auto op = d->object->invoke_method_synchronously<mpris::Service::PauseOtherSessions, |
691 | |
692 | === modified file 'src/core/media/service_stub.h' |
693 | --- src/core/media/service_stub.h 2015-09-08 20:03:56 +0000 |
694 | +++ src/core/media/service_stub.h 2016-05-04 13:11:44 +0000 |
695 | @@ -45,7 +45,6 @@ |
696 | void destroy_session(const std::string& uuid, const Player::Configuration&); |
697 | std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&); |
698 | std::shared_ptr<Player> resume_session(Player::PlayerKey key); |
699 | - void set_current_player(Player::PlayerKey key); |
700 | void pause_other_sessions(Player::PlayerKey key); |
701 | |
702 | private: |
Minor issue