Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ab93aaef authored by Pavlin Radoslavov's avatar Pavlin Radoslavov
Browse files

Use the correct API to check whether a codec is valid 

Use the (new) A2D_IsPeerSinkCodecValid() API call as appropriate
to check whether the codec information of a Sink peer is valid.
Previously, A2D_IsSourceCodecSupported() was used, and that didn't
match the original code (before the refactoring).
A2D_IsSourceCodecSupported() has extra checks, including min/max bitpool
oundaries, and those shouldn't be used for this initial check - the
bitpool boundaries can be adjusted later.

Similarly, use the new A2D_IsPeerSourceCodecValid() API call instead of
A2D_IsSinkCodecSupported().

Also:
 * Replaced A2D_IsValidCodec() with
   A2D_IsSourceCodecValid(), A2D_IsSinkCodecValid()
   A2D_IsPeerSourceCodecValid(), A2D_IsPeerSinkCodecValid()
   and added the appropriate unit tests.

 * Added extra debug messages to help identify similar issues in the future

Bug: 31749230
Test: manual test with a carkit, and unit tests
Change-Id: Iafaeb82744df9758e686194c91624992a0c55bdf
parent 217e9d16

system/btif/co/bta_av_co.cc

+11 −6
Original line number Diff line number Diff line
@@ -306,7 +306,7 @@ static tA2D_STATUS bta_av_audio_sink_getconfig(tBTA_AV_HNDL hndl, 
    p_peer->num_rx_srcs++;



    /* Check the peer's SOURCE codec */

    if (A2D_IsSinkCodecSupported(p_codec_info)) {

    if (A2D_IsPeerSourceCodecValid(p_codec_info)) {

        /* If there is room for a new one */

        if (p_peer->num_sup_srcs < BTA_AV_CO_NUM_ELEMENTS(p_peer->srcs))

        {
@@ -430,7 +430,7 @@ tA2D_STATUS bta_av_co_audio_getconfig(tBTA_AV_HNDL hndl, 
    p_peer->num_rx_sinks++;



    /* Check the peer's SINK codec */

    if (A2D_IsSourceCodecSupported(p_codec_info)) {

    if (A2D_IsPeerSinkCodecValid(p_codec_info)) {

        /* If there is room for a new one */

        if (p_peer->num_sup_sinks < BTA_AV_CO_NUM_ELEMENTS(p_peer->sinks)) {

            tBTA_AV_CO_SINK *p_sink = &p_peer->sinks[p_peer->num_sup_sinks++];
@@ -460,7 +460,10 @@ tA2D_STATUS bta_av_co_audio_getconfig(tBTA_AV_HNDL hndl, 
        /* Find a sink that matches the codec config */

        const tBTA_AV_CO_SINK *p_sink =

            bta_av_co_find_peer_sink_supports_codec(p_peer);

        if (p_sink != NULL) {

        if (p_sink == NULL) {

            APPL_TRACE_ERROR("%s: cannot find peer SINK for this codec config",

                             __func__);

        } else {

            /* stop fetching caps once we retrieved a supported codec */

            if (p_peer->acp) {

                APPL_TRACE_EVENT("%s: no need to fetch more SEPs", __func__);
@@ -921,7 +924,8 @@ static bool bta_av_co_audio_sink_supports_cp(const tBTA_AV_CO_SINK *p_sink) 
static const tBTA_AV_CO_SINK* bta_av_co_find_peer_sink_supports_codec(

    const tBTA_AV_CO_PEER *p_peer)

{

    APPL_TRACE_DEBUG("%s", __func__);

    APPL_TRACE_DEBUG("%s: peer num_sup_sinks = %d",

                     __func__, p_peer->num_sup_sinks);



    for (size_t index = 0; index < p_peer->num_sup_sinks; index++) {

        if (A2D_CodecConfigMatchesCapabilities(bta_av_co_cb.codec_cfg,
@@ -944,7 +948,8 @@ static const tBTA_AV_CO_SINK* bta_av_co_find_peer_sink_supports_codec( 
static const tBTA_AV_CO_SINK* bta_av_co_find_peer_src_supports_codec(

    const tBTA_AV_CO_PEER *p_peer)

{

    APPL_TRACE_DEBUG("%s", __func__);

    APPL_TRACE_DEBUG("%s: peer num_sup_srcs = %d",

                     __func__, p_peer->num_sup_srcs);



    for (size_t index = 0; index < p_peer->num_sup_srcs; index++) {

        const uint8_t *p_codec_caps = p_peer->srcs[index].codec_caps;
@@ -1159,7 +1164,7 @@ void bta_av_co_audio_encoder_update(tBTIF_MEDIA_UPDATE_AUDIO *msg) 
     * Adjust our preferred bitpool with the remote preference if within

     * our capable range.

     */

    if (A2D_IsValidCodec(bta_av_co_cb.codec_cfg_setconfig) &&

    if (A2D_IsSourceCodecValid(bta_av_co_cb.codec_cfg_setconfig) &&

        A2D_CodecTypeEquals(p_codec_info, bta_av_co_cb.codec_cfg_setconfig)) {

        int setconfig_min_bitpool =

            A2D_GetMinBitpool(bta_av_co_cb.codec_cfg_setconfig);

system/stack/a2dp/a2d_api.c

+59 −5
Original line number Diff line number Diff line
@@ -407,7 +407,12 @@ void A2D_Init(void) 
#endif

}



bool A2D_IsValidCodec(const uint8_t *p_codec_info)

tA2D_CODEC_TYPE A2D_GetCodecType(const uint8_t *p_codec_info)

{

    return (tA2D_CODEC_TYPE)(p_codec_info[AVDT_CODEC_TYPE_INDEX]);

}



bool A2D_IsSourceCodecValid(const uint8_t *p_codec_info)

{

    tA2D_CODEC_TYPE codec_type = A2D_GetCodecType(p_codec_info);
@@ -415,9 +420,9 @@ bool A2D_IsValidCodec(const uint8_t *p_codec_info) 


    switch (codec_type) {

    case A2D_MEDIA_CT_SBC:

        return A2D_IsValidCodecSbc(p_codec_info);

        return A2D_IsSourceCodecValidSbc(p_codec_info);

    case A2D_MEDIA_CT_NON_A2DP:

        return A2D_IsVendorValidCodec(p_codec_info);

        return A2D_IsVendorSourceCodecValid(p_codec_info);

    default:

        break;

    }
@@ -425,9 +430,58 @@ bool A2D_IsValidCodec(const uint8_t *p_codec_info) 
    return false;

}



tA2D_CODEC_TYPE A2D_GetCodecType(const uint8_t *p_codec_info)

bool A2D_IsSinkCodecValid(const uint8_t *p_codec_info)

{

    return (tA2D_CODEC_TYPE)(p_codec_info[AVDT_CODEC_TYPE_INDEX]);

    tA2D_CODEC_TYPE codec_type = A2D_GetCodecType(p_codec_info);



    LOG_DEBUG(LOG_TAG, "%s: codec_type = 0x%x", __func__, codec_type);



    switch (codec_type) {

    case A2D_MEDIA_CT_SBC:

        return A2D_IsSinkCodecValidSbc(p_codec_info);

    case A2D_MEDIA_CT_NON_A2DP:

        return A2D_IsVendorSinkCodecValid(p_codec_info);

    default:

        break;

    }



    return false;

}



bool A2D_IsPeerSourceCodecValid(const uint8_t *p_codec_info)

{

    tA2D_CODEC_TYPE codec_type = A2D_GetCodecType(p_codec_info);



    LOG_DEBUG(LOG_TAG, "%s: codec_type = 0x%x", __func__, codec_type);



    switch (codec_type) {

    case A2D_MEDIA_CT_SBC:

        return A2D_IsPeerSourceCodecValidSbc(p_codec_info);

    case A2D_MEDIA_CT_NON_A2DP:

        return A2D_IsVendorPeerSourceCodecValid(p_codec_info);

    default:

        break;

    }



    return false;

}



bool A2D_IsPeerSinkCodecValid(const uint8_t *p_codec_info)

{

    tA2D_CODEC_TYPE codec_type = A2D_GetCodecType(p_codec_info);



    LOG_DEBUG(LOG_TAG, "%s: codec_type = 0x%x", __func__, codec_type);



    switch (codec_type) {

    case A2D_MEDIA_CT_SBC:

        return A2D_IsPeerSinkCodecValidSbc(p_codec_info);

    case A2D_MEDIA_CT_NON_A2DP:

        return A2D_IsVendorPeerSinkCodecValid(p_codec_info);

    default:

        break;

    }



    return false;

}



bool A2D_IsSourceCodecSupported(const uint8_t *p_codec_info)

system/stack/a2dp/a2d_sbc.c

+46 −2
Original line number Diff line number Diff line
@@ -305,7 +305,34 @@ bool A2D_InitCodecConfigSbcSink(tAVDT_CFG *p_cfg) 
    return true;

}



bool A2D_IsValidCodecSbc(const uint8_t *p_codec_info)

bool A2D_IsSourceCodecValidSbc(const uint8_t *p_codec_info)

{

    tA2D_SBC_CIE cfg_cie;



    /* Use a liberal check when parsing the codec info */

    return (A2D_ParsSbcInfo(&cfg_cie, p_codec_info, false) == A2D_SUCCESS) ||

      (A2D_ParsSbcInfo(&cfg_cie, p_codec_info, true) == A2D_SUCCESS);

}



bool A2D_IsSinkCodecValidSbc(const uint8_t *p_codec_info)

{

    tA2D_SBC_CIE cfg_cie;



    /* Use a liberal check when parsing the codec info */

    return (A2D_ParsSbcInfo(&cfg_cie, p_codec_info, false) == A2D_SUCCESS) ||

      (A2D_ParsSbcInfo(&cfg_cie, p_codec_info, true) == A2D_SUCCESS);

}



bool A2D_IsPeerSourceCodecValidSbc(const uint8_t *p_codec_info)

{

    tA2D_SBC_CIE cfg_cie;



    /* Use a liberal check when parsing the codec info */

    return (A2D_ParsSbcInfo(&cfg_cie, p_codec_info, false) == A2D_SUCCESS) ||

      (A2D_ParsSbcInfo(&cfg_cie, p_codec_info, true) == A2D_SUCCESS);

}



bool A2D_IsPeerSinkCodecValidSbc(const uint8_t *p_codec_info)

{

    tA2D_SBC_CIE cfg_cie;
@@ -677,11 +704,28 @@ bool A2D_CodecConfigMatchesCapabilitiesSbc(const uint8_t *p_codec_config, 
     * Must match all SBC codec fields except min/max bitpool boundaries which

     * can be adjusted.

     */

    return (sbc_cie_config.samp_freq & sbc_cie_caps.samp_freq) &&

    bool result =

      (sbc_cie_config.samp_freq & sbc_cie_caps.samp_freq) &&

      (sbc_cie_config.ch_mode & sbc_cie_caps.ch_mode) &&

      (sbc_cie_config.block_len & sbc_cie_caps.block_len) &&

      (sbc_cie_config.num_subbands & sbc_cie_caps.num_subbands) &&

      (sbc_cie_config.alloc_method & sbc_cie_caps.alloc_method);



    LOG_DEBUG(LOG_TAG, "%s: result=%s", __func__, result ? "true" : "false");

    LOG_DEBUG(LOG_TAG, "%s: config samp_freq=0x%x ch_mode=0x%x block_len=0x%x "

              "num_subbands=0x%x alloc_method=0x%x",

              __func__,

              sbc_cie_config.samp_freq, sbc_cie_config.ch_mode,

              sbc_cie_config.block_len, sbc_cie_config.num_subbands,

              sbc_cie_config.alloc_method);

    LOG_DEBUG(LOG_TAG, "%s: caps   samp_freq=0x%x ch_mode=0x%x block_len=0x%x "

              "num_subbands=0x%x alloc_method=0x%x",

              __func__,

              sbc_cie_caps.samp_freq, sbc_cie_caps.ch_mode,

              sbc_cie_caps.block_len, sbc_cie_caps.num_subbands,

              sbc_cie_caps.alloc_method);



    return result;

}



int A2D_GetTrackFrequencySbc(const uint8_t *p_codec_info)

system/stack/a2dp/a2d_vendor.c

+31 −1
Original line number Diff line number Diff line
@@ -25,7 +25,37 @@ 
#include "osi/include/log.h"

#include "osi/include/osi.h"



bool A2D_IsVendorValidCodec(UNUSED_ATTR const uint8_t *p_codec_info)

bool A2D_IsVendorSourceCodecValid(UNUSED_ATTR const uint8_t *p_codec_info)

{

    // uint32_t vendor_id = A2D_VendorCodecGetVendorId(p_codec_info);

    // uint16_t codec_id = A2D_VendorCodecGetCodecId(p_codec_info);



    // Add checks based on <vendor_id, codec_id>



    return false;

}



bool A2D_IsVendorSinkCodecValid(UNUSED_ATTR const uint8_t *p_codec_info)

{

    // uint32_t vendor_id = A2D_VendorCodecGetVendorId(p_codec_info);

    // uint16_t codec_id = A2D_VendorCodecGetCodecId(p_codec_info);



    // Add checks based on <vendor_id, codec_id>



    return false;

}



bool A2D_IsVendorPeerSourceCodecValid(UNUSED_ATTR const uint8_t *p_codec_info)

{

    // uint32_t vendor_id = A2D_VendorCodecGetVendorId(p_codec_info);

    // uint16_t codec_id = A2D_VendorCodecGetCodecId(p_codec_info);



    // Add checks based on <vendor_id, codec_id>



    return false;

}



bool A2D_IsVendorPeerSinkCodecValid(UNUSED_ATTR const uint8_t *p_codec_info)

{

    // uint32_t vendor_id = A2D_VendorCodecGetVendorId(p_codec_info);

    // uint16_t codec_id = A2D_VendorCodecGetCodecId(p_codec_info);

system/stack/include/a2d_api.h

+24 −5
Original line number Diff line number Diff line
@@ -266,15 +266,34 @@ extern uint8_t A2D_BitsSet(uint8_t num); 
*******************************************************************************/

extern void A2D_Init(void);



// Checks whether the codec capabilities contain a valid A2DP codec.

// Gets the A2DP codec type.

// |p_codec_info| contains information about the codec capabilities.

tA2D_CODEC_TYPE A2D_GetCodecType(const uint8_t *p_codec_info);



// Checks whether the codec capabilities contain a valid A2DP source codec.

// NOTE: only codecs that are implemented are considered valid.

// Returns true if |p_codec_info| contains information about a valid codec,

// otherwise false.

bool A2D_IsValidCodec(const uint8_t *p_codec_info);

bool A2D_IsSourceCodecValid(const uint8_t *p_codec_info);



// Gets the A2DP codec type.

// |p_codec_info| contains information about the codec capabilities.

tA2D_CODEC_TYPE A2D_GetCodecType(const uint8_t *p_codec_info);

// Checks whether the codec capabilities contain a valid A2DP sink codec.

// NOTE: only codecs that are implemented are considered valid.

// Returns true if |p_codec_info| contains information about a valid codec,

// otherwise false.

bool A2D_IsSinkCodecValid(const uint8_t *p_codec_info);



// Checks whether the codec capabilities contain a valid peer A2DP source

// codec.

// NOTE: only codecs that are implemented are considered valid.

// Returns true if |p_codec_info| contains information about a valid codec,

// otherwise false.

bool A2D_IsPeerSourceCodecValid(const uint8_t *p_codec_info);



// Checks whether the codec capabilities contain a valid peer A2DP sink codec.

// NOTE: only codecs that are implemented are considered valid.

// Returns true if |p_codec_info| contains information about a valid codec,

// otherwise false.

bool A2D_IsPeerSinkCodecValid(const uint8_t *p_codec_info);



// Checks whether an A2DP source codec is supported.

// |p_codec_info| contains information about the codec capabilities.