From 8280f1c838e54464f4aaea705c48c05b0f1fd934 Mon Sep 17 00:00:00 2001 From: Sal Savage Date: Fri, 28 Feb 2020 13:33:22 -0800 Subject: [PATCH] Pass BIP client status down and use it to decide to send image handles The new avrcp architecture has the design such that all requests to Java are agnostic of which device is asking for them. However, image handles are not meant to be sent if there is no BIP connection with that device. Rather than invalidate the previous design assumption, this patch sends that status down to the connection handler and device objects to use when building response packets. The image handles, if included, will be automatically removed if there is no client connection. If no image handles are sent, none will be included. Tag: #feature Bug: 153076316 Test: Build, flash, make sure existing metadata isn't impacted and is still sent when requested. Make sure imge handles are sent, when they exist, if and only if we've connected a client on the Java side BIP OBEX server. Change-Id: If252abe120188df2bcdabed22382fba070d17f32 --- btif/avrcp/avrcp_service.cc | 16 ++++++++++++ btif/avrcp/avrcp_service.h | 3 +++ include/hardware/avrcp/avrcp.h | 1 + profile/avrcp/connection_handler.cc | 10 ++++++++ profile/avrcp/connection_handler.h | 8 ++++++ profile/avrcp/device.cc | 50 ++++++++++++++++++++++++++++++++++--- profile/avrcp/device.h | 11 ++++++++ 7 files changed, 96 insertions(+), 3 deletions(-) diff --git a/btif/avrcp/avrcp_service.cc b/btif/avrcp/avrcp_service.cc index 834a9739b..d9887ea16 100644 --- a/btif/avrcp/avrcp_service.cc +++ b/btif/avrcp/avrcp_service.cc @@ -401,6 +401,13 @@ void AvrcpService::DisconnectDevice(const RawAddress& bdaddr) { connection_handler_->DisconnectDevice(bdaddr); } +void AvrcpService::SetBipClientStatus(const RawAddress& bdaddr, + bool connected) { + LOG(INFO) << __PRETTY_FUNCTION__ << ": address=" << bdaddr.ToString() + << ", connected=" << connected; + connection_handler_->SetBipClientStatus(bdaddr, connected); +} + void AvrcpService::SendMediaUpdate(bool track_changed, bool play_state, bool queue) { LOG(INFO) << __PRETTY_FUNCTION__ << " track_changed=" << track_changed @@ -500,6 +507,15 @@ bool AvrcpService::ServiceInterfaceImpl::DisconnectDevice( return true; } +void AvrcpService::ServiceInterfaceImpl::SetBipClientStatus( + const RawAddress& bdaddr, bool connected) { + std::lock_guard lock(service_interface_lock_); + CHECK(instance_ != nullptr); + do_in_main_thread(FROM_HERE, base::Bind(&AvrcpService::SetBipClientStatus, + base::Unretained(instance_), bdaddr, + connected)); +} + bool AvrcpService::ServiceInterfaceImpl::Cleanup() { std::lock_guard lock(service_interface_lock_); diff --git a/btif/avrcp/avrcp_service.h b/btif/avrcp/avrcp_service.h index 72929c7db..b60700f33 100644 --- a/btif/avrcp/avrcp_service.h +++ b/btif/avrcp/avrcp_service.h @@ -62,6 +62,8 @@ class AvrcpService : public MediaCallbacks { void ConnectDevice(const RawAddress& bdaddr); void DisconnectDevice(const RawAddress& bdaddr); + void SetBipClientStatus(const RawAddress& bdaddr, bool connected); + // Functions inherited from MediaCallbacks in order to receive updates void SendMediaUpdate(bool track_changed, bool play_state, bool queue) override; @@ -77,6 +79,7 @@ class AvrcpService : public MediaCallbacks { void UnregisterBipServer() override; bool ConnectDevice(const RawAddress& bdaddr) override; bool DisconnectDevice(const RawAddress& bdaddr) override; + void SetBipClientStatus(const RawAddress& bdaddr, bool connected) override; bool Cleanup() override; private: diff --git a/include/hardware/avrcp/avrcp.h b/include/hardware/avrcp/avrcp.h index 5612df073..66a5058b0 100644 --- a/include/hardware/avrcp/avrcp.h +++ b/include/hardware/avrcp/avrcp.h @@ -180,6 +180,7 @@ class ServiceInterface { virtual void UnregisterBipServer() = 0; virtual bool ConnectDevice(const RawAddress& bdaddr) = 0; virtual bool DisconnectDevice(const RawAddress& bdaddr) = 0; + virtual void SetBipClientStatus(const RawAddress& bdaddr, bool connected) = 0; virtual bool Cleanup() = 0; protected: diff --git a/profile/avrcp/connection_handler.cc b/profile/avrcp/connection_handler.cc index 7bff6617d..efa513cbb 100644 --- a/profile/avrcp/connection_handler.cc +++ b/profile/avrcp/connection_handler.cc @@ -149,6 +149,16 @@ bool ConnectionHandler::DisconnectDevice(const RawAddress& bdaddr) { return false; } +void ConnectionHandler::SetBipClientStatus(const RawAddress& bdaddr, + bool connected) { + for (auto it = device_map_.begin(); it != device_map_.end(); it++) { + if (bdaddr == it->second->GetAddress()) { + it->second->SetBipClientStatus(connected); + return; + } + } +} + std::vector> ConnectionHandler::GetListOfDevices() const { std::vector> list; diff --git a/profile/avrcp/connection_handler.h b/profile/avrcp/connection_handler.h index d5f0a2725..adaf2394b 100644 --- a/profile/avrcp/connection_handler.h +++ b/profile/avrcp/connection_handler.h @@ -110,6 +110,14 @@ class ConnectionHandler { */ virtual bool DisconnectDevice(const RawAddress& bdaddr); + /** + * Indicates the connection status of a device on the BIP OBEX server. + * + * This status is used to determine whether we should include image handles + * when building responses for media item metadata queries. + */ + virtual void SetBipClientStatus(const RawAddress& bdaddr, bool connected); + virtual std::vector> GetListOfDevices() const; /** diff --git a/profile/avrcp/device.cc b/profile/avrcp/device.cc index 34292d226..a31ed917b 100644 --- a/profile/avrcp/device.cc +++ b/profile/avrcp/device.cc @@ -46,7 +46,8 @@ Device::Device( avrcp13_compatibility_(avrcp13_compatibility), send_message_cb_(send_msg_cb), ctrl_mtu_(ctrl_mtu), - browse_mtu_(browse_mtu) {} + browse_mtu_(browse_mtu), + has_bip_client_(false) {} void Device::RegisterInterfaces(MediaInterface* media_interface, A2dpInterface* a2dp_interface, @@ -67,6 +68,24 @@ void Device::SetBrowseMtu(uint16_t browse_mtu) { browse_mtu_ = browse_mtu; } +void Device::SetBipClientStatus(bool connected) { + DEVICE_LOG(INFO) << __PRETTY_FUNCTION__ << ": connected = " << connected; + has_bip_client_ = connected; +} + +bool Device::HasBipClient() const { + return has_bip_client_; +} + +void filter_cover_art(SongInfo& s) { + for (auto it = s.attributes.begin(); it != s.attributes.end(); it++) { + if (it->attribute() == Attribute::DEFAULT_COVER_ART) { + s.attributes.erase(it); + break; + } + } +} + bool Device::IsActive() const { return address_ == a2dp_interface_->active_peer(); } @@ -587,14 +606,17 @@ void Device::GetPlayStatusResponse(uint8_t label, PlayStatus status) { void Device::GetElementAttributesResponse( uint8_t label, std::shared_ptr pkt, SongInfo info) { - DEVICE_VLOG(2) << __func__; - auto get_element_attributes_pkt = pkt; auto attributes_requested = get_element_attributes_pkt->GetAttributesRequested(); auto response = GetElementAttributesResponseBuilder::MakeBuilder(ctrl_mtu_); + // Filter out DEFAULT_COVER_ART handle if this device has no client + if (!HasBipClient()) { + filter_cover_art(info); + } + last_song_info_ = info; if (attributes_requested.size() != 0) { @@ -1008,6 +1030,11 @@ void Device::GetItemAttributesNowPlayingResponse( } } + // Filter out DEFAULT_COVER_ART handle if this device has no client + if (!HasBipClient()) { + filter_cover_art(info); + } + auto attributes_requested = pkt->GetAttributesRequested(); if (attributes_requested.size() != 0) { for (const auto& attribute : attributes_requested) { @@ -1051,6 +1078,11 @@ void Device::GetItemAttributesVFSResponse( } } + // Filter out DEFAULT_COVER_ART handle if this device has no client + if (item_requested.type == ListItem::SONG && !HasBipClient()) { + filter_cover_art(item_requested.song); + } + // TODO (apanicke): Add a helper function or allow adding a map // of attributes to GetItemAttributesResponseBuilder auto attributes_requested = pkt->GetAttributesRequested(); @@ -1170,6 +1202,12 @@ void Device::GetVFSListResponse(uint8_t label, if (!builder->AddFolder(folder_item)) break; } else if (items[i].type == ListItem::SONG) { auto song = items[i].song; + + // Filter out DEFAULT_COVER_ART handle if this device has no client + if (!HasBipClient()) { + filter_cover_art(song); + } + auto title = song.attributes.find(Attribute::TITLE) != song.attributes.end() ? song.attributes.find(Attribute::TITLE)->value() @@ -1208,6 +1246,12 @@ void Device::GetNowPlayingListResponse( for (size_t i = pkt->GetStartItem(); i <= pkt->GetEndItem() && i < song_list.size(); i++) { auto song = song_list[i]; + + // Filter out DEFAULT_COVER_ART handle if this device has no client + if (!HasBipClient()) { + filter_cover_art(song); + } + auto title = song.attributes.find(Attribute::TITLE) != song.attributes.end() ? song.attributes.find(Attribute::TITLE)->value() : "No Song Info"; diff --git a/profile/avrcp/device.h b/profile/avrcp/device.h index 18bfc98be..fc5d7364b 100644 --- a/profile/avrcp/device.h +++ b/profile/avrcp/device.h @@ -81,6 +81,16 @@ class Device { bool Disconnect(); /** + * Set the status of the BIP obex client + */ + void SetBipClientStatus(bool connected); + + /** + * Returns true if the current device has a BIP OBEX client. + */ + bool HasBipClient() const; + + /** * Returns true if the current device is silenced. */ bool IsInSilenceMode() const; @@ -295,6 +305,7 @@ class Device { send_message_cb_; uint16_t ctrl_mtu_; uint16_t browse_mtu_; + bool has_bip_client_; int curr_browsed_player_id_ = -1; -- 2.11.0