[Merge] lp:~justinmcp/media-hub/proxy-player into lp:media-hub
Jim Hodapp
jim.hodapp at canonical.com
Mon Apr 18 14:16:39 UTC 2016
Diff comments:
>
> === modified file 'src/core/media/service_implementation.cpp'
> --- src/core/media/service_implementation.cpp 2016-03-02 18:32:46 +0000
> +++ src/core/media/service_implementation.cpp 2016-04-06 01:02:31 +0000
> @@ -297,6 +298,45 @@
> });
> }
>
> +std::shared_ptr<media::Player> media::ServiceImplementation::create_proxy_session(
I can see your point somewhat but I still think it'd be preferable if we were to create a new enum type even though this affects both the internal and external (stub) interface. I think this is the right time to do this refactoring as it wouldn't be that major. I think it's preferable because it reduces some of the repetitious functions that we've been getting with the Oxide integration from the past. I've been doing some changes lately that have required me to copy and paste code between a normal function and the Oxide-specific function and most times when I'm required to copy and paste code like that, that tells me that the design is not optimal.
> + const media::Player::Configuration& conf)
> +{
> + auto player = std::make_shared<media::ProxyPlayerImplementation<media::PlayerSkeleton>>(media::ProxyPlayerImplementation<media::PlayerSkeleton>::Configuration
> + {
> + media::PlayerSkeleton::Configuration
> + {
> + conf.bus,
> + conf.service,
> + conf.session,
> + d->request_context_resolver,
> + d->request_authenticator
> + },
> + conf.key,
> + d->client_death_observer,
> + d->power_state_controller
> + });
> +
> + auto key = conf.key;
> + player->on_client_disconnected().connect([this, key]()
> + {
> + // Call remove_player_for_key asynchronously otherwise deadlock can occur
> + // if called within this dispatcher context.
> + // remove_player_for_key can destroy the player instance which in turn
> + // destroys the "on_client_disconnected" signal whose destructor will wait
> + // until all dispatches are done
> + d->configuration.external_services.io_service.post([this, key]()
> + {
> + if (!d->configuration.player_store->has_player_for_key(key))
> + return;
> +
> + if (d->configuration.player_store->player_for_key(key)->lifetime() == Player::Lifetime::normal)
> + d->configuration.player_store->remove_player_for_key(key);
> + });
> + });
> +
> + return player;
> +}
> +
> void media::ServiceImplementation::pause_all_multimedia_sessions(bool resume_play_after_phonecall)
> {
> d->configuration.player_store->enumerate_players([this, resume_play_after_phonecall](const media::Player::PlayerKey& key, const std::shared_ptr<media::Player>& player)
--
https://code.launchpad.net/~justinmcp/media-hub/proxy-player/+merge/268862
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.
More information about the Ubuntu-reviews
mailing list