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

diff --git a/btif/avrcp/avrcp_service.cc b/btif/avrcp/avrcp_service.cc
index 834a973..d9887ea 100644 (file)
--- 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<std::mutex> 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<std::mutex> lock(service_interface_lock_);
 

diff --git a/btif/avrcp/avrcp_service.h b/btif/avrcp/avrcp_service.h
index 72929c7..b60700f 100644 (file)
--- 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 5612df0..66a5058 100644 (file)
--- 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 7bff661..efa513c 100644 (file)
--- 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<std::shared_ptr<Device>> ConnectionHandler::GetListOfDevices()
     const {
   std::vector<std::shared_ptr<Device>> list;

diff --git a/profile/avrcp/connection_handler.h b/profile/avrcp/connection_handler.h
index d5f0a27..adaf239 100644 (file)
--- 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<std::shared_ptr<Device>> GetListOfDevices() const;
 
   /**

diff --git a/profile/avrcp/device.cc b/profile/avrcp/device.cc
index 34292d2..a31ed91 100644 (file)
--- 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<GetElementAttributesRequest> 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 18bfc98..fc5d736 100644 (file)
--- 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;