Add a mechanism to avoid using AVDTP RECONFIGURE for blacklisted devices

Some carkits report SUCCESS for AVDTP RECONFIGURE commands when
changing A2DP codec configuration. However, there is no audio coming
from the carkit.

Bug: 37625892
Test: Manual - A2DP Codec reconfiguration while streaming audio
Change-Id: I01f37a2514e490986a20e96ace78c92463403396
Merged-In: I01f37a2514e490986a20e96ace78c92463403396
(cherry picked from commit e91297a4d1a77ae0c367fbe756090b7357e9893a)

diff --git a/bta/av/bta_av_aact.cc b/bta/av/bta_av_aact.cc
index 3bf6f8e..886ed0c 100644 (file)
--- a/bta/av/bta_av_aact.cc
+++ b/bta/av/bta_av_aact.cc
@@ -34,6 +34,8 @@
 #include "bt_utils.h"
 #include "bta_av_int.h"
 #include "btif/include/btif_av_co.h"
+#include "btif/include/btif_storage.h"
+#include "device/include/interop.h"
 #include "l2c_api.h"
 #include "l2cdefs.h"
 #include "osi/include/osi.h"
@@ -2742,11 +2744,33 @@ void bta_av_rcfg_cfm(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) {
   uint8_t err_code = p_data->str_msg.msg.hdr.err_code;
 
   APPL_TRACE_DEBUG("%s: err_code = %d", __func__, err_code);
-  if (err_code) {
+
+  // Disable AVDTP RECONFIGURE for blacklisted devices
+  bool disable_avdtp_reconfigure = false;
+  {
+    char remote_name[BTM_MAX_REM_BD_NAME_LEN] = "";
+    bt_bdaddr_t bd_addr;
+    for (int i = 0; i < 6; i++) bd_addr.address[i] = p_scb->peer_addr[i];
+    if (btif_storage_get_stored_remote_name(bd_addr, remote_name)) {
+      if (interop_match_name(INTEROP_DISABLE_AVDTP_RECONFIGURE, remote_name) ||
+          interop_match_addr(INTEROP_DISABLE_AVDTP_RECONFIGURE,
+                             (const bt_bdaddr_t*)&p_scb->peer_addr)) {
+        APPL_TRACE_DEBUG(
+            "%s: disable AVDTP RECONFIGURE: interop matched "
+            "name %s address %02x:%02x:%02x:%02x:%02x:%02x",
+            __func__, remote_name, p_scb->peer_addr[0], p_scb->peer_addr[1],
+            p_scb->peer_addr[2], p_scb->peer_addr[3], p_scb->peer_addr[4],
+            p_scb->peer_addr[5]);
+        disable_avdtp_reconfigure = true;
+      }
+    }
+  }
+
+  if ((err_code != 0) || disable_avdtp_reconfigure) {
     APPL_TRACE_ERROR("%s: reconfig rejected, try close", __func__);
     /* Disable reconfiguration feature only with explicit rejection(not with
      * timeout) */
-    if (err_code != AVDT_ERR_TIMEOUT) {
+    if ((err_code != AVDT_ERR_TIMEOUT) || disable_avdtp_reconfigure) {
       p_scb->recfg_sup = false;
     }
     /* started flag is false when reconfigure command is sent */

diff --git a/btif/include/btif_storage.h b/btif/include/btif_storage.h
index dc7f4cc..a787613 100644 (file)
--- a/btif/include/btif_storage.h
+++ b/btif/include/btif_storage.h
@@ -261,6 +261,14 @@ bt_status_t btif_storage_set_hidd(bt_bdaddr_t* remote_bd_addr);
 
 bt_status_t btif_storage_remove_hidd(bt_bdaddr_t* remote_bd_addr);
 
+// Gets the device name for a given Bluetooth address |bd_addr|.
+// The device name (if found) is stored in |name|.
+// Returns true if the device name is found, othervise false.
+// Note: |name| should point to a buffer that can store string of length
+// |BTM_MAX_REM_BD_NAME_LEN|.
+bool btif_storage_get_stored_remote_name(const bt_bdaddr_t& bd_addr,
+                                         char* name);
+
 /******************************************************************************
  * Exported for unit tests
  *****************************************************************************/

diff --git a/btif/src/btif_storage.cc b/btif/src/btif_storage.cc
index ff733c5..bfbff99 100644 (file)
--- a/btif/src/btif_storage.cc
+++ b/btif/src/btif_storage.cc
@@ -1431,3 +1431,16 @@ bt_status_t btif_storage_remove_hidd(bt_bdaddr_t* remote_bd_addr) {
 
   return BT_STATUS_SUCCESS;
 }
+
+// Get the name of a device from btif for interop database matching.
+bool btif_storage_get_stored_remote_name(const bt_bdaddr_t& bd_addr,
+                                         char* name) {
+  bt_property_t property;
+  property.type = BT_PROPERTY_BDNAME;
+  property.len = BTM_MAX_REM_BD_NAME_LEN;
+  property.val = name;
+
+  return (btif_storage_get_remote_device_property(
+              const_cast<bt_bdaddr_t*>(&bd_addr), &property) ==
+          BT_STATUS_SUCCESS);
+}

diff --git a/device/include/interop.h b/device/include/interop.h
index 6364a1b..a3b252b 100644 (file)
--- a/device/include/interop.h
+++ b/device/include/interop.h
@@ -74,6 +74,11 @@ typedef enum {
   // Do not send service changed indications (GATT client).
   // This should be removed after the characteristic is implmeented b/62088395.
   INTEROP_GATTC_NO_SERVICE_CHANGED_IND,
+
+  // Do not use AVDTP RECONFIGURE when reconfiguring A2DP streams.
+  // Some A2DP Sink devices report SUCCESS to the AVDTP RECONFIGURE command,
+  // but fail to play the reconfigured audio stream.
+  INTEROP_DISABLE_AVDTP_RECONFIGURE,
 } interop_feature_t;
 
 // Check if a given |addr| matches a known interoperability workaround as

diff --git a/device/include/interop_database.h b/device/include/interop_database.h
index a9a958d..36a8ae2 100644 (file)
--- a/device/include/interop_database.h
+++ b/device/include/interop_database.h
@@ -108,6 +108,9 @@ static const interop_addr_entry_t interop_addr_database[] = {
 
     // Kinivo BTC-450 - volume is erratic when using Absolute Volume
     {{{0x00, 0x18, 0x91, 0, 0, 0}}, 3, INTEROP_DISABLE_ABSOLUTE_VOLUME},
+
+    // Kenwood KMM-BT518HD - no audio when A2DP codec sample rate is changed
+    {{{0x00, 0x1d, 0x86, 0, 0, 0}}, 3, INTEROP_DISABLE_AVDTP_RECONFIGURE},
 };
 
 typedef struct {
@@ -131,4 +134,7 @@ static const interop_name_entry_t interop_name_database[] = {
 
     // Pixel C Keyboard doesn't respond to service changed indications.
     {"Pixel C Keyboard", 16, INTEROP_GATTC_NO_SERVICE_CHANGED_IND},
+
+    // Kenwood KMM-BT518HD - no audio when A2DP codec sample rate is changed
+    {"KMM-BT51*HD", 11, INTEROP_DISABLE_AVDTP_RECONFIGURE},
 };

diff --git a/device/src/interop.cc b/device/src/interop.cc
index 382fea4..c6b5062 100644 (file)
--- a/device/src/interop.cc
+++ b/device/src/interop.cc
@@ -126,6 +126,7 @@ static const char* interop_feature_string_(const interop_feature_t feature) {
     CASE_RETURN_STR(INTEROP_2MBPS_LINK_ONLY)
     CASE_RETURN_STR(INTEROP_HID_PREF_CONN_SUP_TIMEOUT_3S)
     CASE_RETURN_STR(INTEROP_GATTC_NO_SERVICE_CHANGED_IND)
+    CASE_RETURN_STR(INTEROP_DISABLE_AVDTP_RECONFIGURE)
   }
 
   return "UNKNOWN";

diff --git a/stack/gatt/gatt_main.cc b/stack/gatt/gatt_main.cc
index 2eb9c04..d375b84 100644 (file)
--- a/stack/gatt/gatt_main.cc
+++ b/stack/gatt/gatt_main.cc
@@ -1103,20 +1103,6 @@ void gatt_init_srv_chg(void) {
   }
 }
 
-// Get the name of a device from btif for interop database matching.
-static bool get_stored_remote_name(BD_ADDR bda, char* name) {
-  bt_bdaddr_t bd_addr;
-  for (int i = 0; i < 6; i++) bd_addr.address[i] = bda[i];
-
-  bt_property_t property;
-  property.type = BT_PROPERTY_BDNAME;
-  property.len = BTM_MAX_REM_BD_NAME_LEN;
-  property.val = name;
-
-  return (btif_storage_get_remote_device_property(&bd_addr, &property) ==
-          BT_STATUS_SUCCESS);
-}
-
 /*******************************************************************************
  *
  * Function         gatt_proc_srv_chg
@@ -1150,7 +1136,10 @@ void gatt_proc_srv_chg(void) {
 
       // Some LE GATT clients don't respond to service changed indications.
       char remote_name[BTM_MAX_REM_BD_NAME_LEN] = "";
-      if (send_indication && get_stored_remote_name(bda, remote_name)) {
+      bt_bdaddr_t bd_addr;
+      for (int i = 0; i < 6; i++) bd_addr.address[i] = bda[i];
+      if (send_indication &&
+          btif_storage_get_stored_remote_name(bd_addr, remote_name)) {
         if (interop_match_name(INTEROP_GATTC_NO_SERVICE_CHANGED_IND,
                                remote_name)) {
           GATT_TRACE_DEBUG("discard srv chg - interop matched %s", remote_name);