[BlueZ,2/2] a2dp: Reject incoming connection when channel limit is exceeded

Add counting of active audio streams at MediaEndpoint level.

Reject incoming connection at the audio stream setting stage
when the configured A2DP channel limit is exceeded.

---
 profiles/audio/a2dp.c  |  9 +++++++++
 profiles/audio/media.c | 11 +++++++++++
 2 files changed, 20 insertions(+)

Hi Victor,

On Thu, Nov 7, 2024 at 5:10 PM Victor Pushkarev
<VAnPushkarev@salutedevices.com> wrote:
>
> Add counting of active audio streams at MediaEndpoint level.
>
> Reject incoming connection at the audio stream setting stage
> when the configured A2DP channel limit is exceeded.
>
> ---
>  profiles/audio/a2dp.c  |  9 +++++++++
>  profiles/audio/media.c | 11 +++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> index c97bd6e89..935873d07 100644
> --- a/profiles/audio/a2dp.c
> +++ b/profiles/audio/a2dp.c
> @@ -716,6 +716,15 @@ static gboolean endpoint_setconf_ind(struct avdtp *session,
>                         return TRUE;
>                 }
>
> +               if (ret == -1) {
> +                       /* Reject connection from SEP
> +                       * and clear configuration.
> +                       */
> +                       DBG("Reject connection from %s", device_get_path(setup->chan->device));
> +                       a2dp_sep->endpoint->clear_configuration(a2dp_sep, a2dp_sep->user_data);
> +                       device_request_disconnect(setup->chan->device, NULL);
> +               }
> +
>                 setup_error_init(setup, AVDTP_MEDIA_CODEC,
>                                         AVDTP_UNSUPPORTED_CONFIGURATION);
>                 setup_unref(setup);
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> index 746e538fc..20b860f6a 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -33,6 +33,7 @@
>  #include "src/dbus-common.h"
>  #include "src/profile.h"
>  #include "src/service.h"
> +#include "src/btd.h"
>
>  #include "src/uuid-helper.h"
>  #include "src/log.h"
> @@ -134,6 +135,7 @@ struct media_player {
>  };
>
>  static GSList *adapters = NULL;
> +static unsigned int active_streams;
>
>  static void endpoint_request_free(struct endpoint_request *request)
>  {
> @@ -302,6 +304,9 @@ done:
>
>  static void clear_endpoint(struct media_endpoint *endpoint)
>  {
> +       if (active_streams > 0)
> +               active_streams--;
> +
>         media_endpoint_cancel_all(endpoint);
>
>         while (endpoint->transports != NULL)
> @@ -659,6 +664,12 @@ static int set_config(struct a2dp_sep *sep, uint8_t *configuration,
>         struct media_endpoint *endpoint = user_data;
>         struct a2dp_config_data *data;
>
> +       active_streams++;
> +       if (active_streams > btd_opts.a2dp.channels) {
> +               DBG("A2DP channel limit (%u) exceeded", btd_opts.a2dp.channels);
> +               return -1;
> +       }

This is a global limit? Not really following the reasoning here, if
the platform don't want to connect to more devices just don't connect
them, or in case of incoming connection don't authorize the
connections, make the endpoints return an error to SetConfiguration,
suspend the streams, etc. There are so many ways to block this that I
don't really feel like introducing a new one will do us any good.

>         data = g_new0(struct a2dp_config_data, 1);
>         data->setup = setup;
>         data->cb = cb;
> --
> 2.39.3 (Apple Git-146)
>
>

Hi, Luiz!

> This is a global limit?

Yes, the limit is global, to be able to disable/configure the behavior
via conf file.

> Not really following the reasoning here, if
> the platform don't want to connect to more devices just don't connect
> them

In this case, the patch solves the problem with audio mixing from
multiple sources via A2DP when the sink device is for example
a bluetooth speaker.

> make the endpoints return an error to SetConfiguration

Which I tried to do, but probably not too architecturally correct.
This patch works, but I would like to find the most correct
solution or implement it for everyone's benefit.

> There are so many ways to block this that I
> don't really feel like introducing a new one will do us any good.

In that case, could you please guide me - which option should I use
to limit the number of A2DP connections?
I haven't found a suitable setting or option that solves the problem
of multiple simultaneous A2DP connections..

Thank you in advance.

08.11.2024 02:01, Luiz Augusto von Dentz пишет:
> Hi Victor,
>
> On Thu, Nov 7, 2024 at 5:10 PM Victor Pushkarev
> <VAnPushkarev@salutedevices.com> wrote:
>> Add counting of active audio streams at MediaEndpoint level.
>>
>> Reject incoming connection at the audio stream setting stage
>> when the configured A2DP channel limit is exceeded.
>>
>> ---
>>   profiles/audio/a2dp.c  |  9 +++++++++
>>   profiles/audio/media.c | 11 +++++++++++
>>   2 files changed, 20 insertions(+)
>>
>> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
>> index c97bd6e89..935873d07 100644
>> --- a/profiles/audio/a2dp.c
>> +++ b/profiles/audio/a2dp.c
>> @@ -716,6 +716,15 @@ static gboolean endpoint_setconf_ind(struct avdtp *session,
>>                          return TRUE;
>>                  }
>>
>> +               if (ret == -1) {
>> +                       /* Reject connection from SEP
>> +                       * and clear configuration.
>> +                       */
>> +                       DBG("Reject connection from %s", device_get_path(setup->chan->device));
>> +                       a2dp_sep->endpoint->clear_configuration(a2dp_sep, a2dp_sep->user_data);
>> +                       device_request_disconnect(setup->chan->device, NULL);
>> +               }
>> +
>>                  setup_error_init(setup, AVDTP_MEDIA_CODEC,
>>                                          AVDTP_UNSUPPORTED_CONFIGURATION);
>>                  setup_unref(setup);
>> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
>> index 746e538fc..20b860f6a 100644
>> --- a/profiles/audio/media.c
>> +++ b/profiles/audio/media.c
>> @@ -33,6 +33,7 @@
>>   #include "src/dbus-common.h"
>>   #include "src/profile.h"
>>   #include "src/service.h"
>> +#include "src/btd.h"
>>
>>   #include "src/uuid-helper.h"
>>   #include "src/log.h"
>> @@ -134,6 +135,7 @@ struct media_player {
>>   };
>>
>>   static GSList *adapters = NULL;
>> +static unsigned int active_streams;
>>
>>   static void endpoint_request_free(struct endpoint_request *request)
>>   {
>> @@ -302,6 +304,9 @@ done:
>>
>>   static void clear_endpoint(struct media_endpoint *endpoint)
>>   {
>> +       if (active_streams > 0)
>> +               active_streams--;
>> +
>>          media_endpoint_cancel_all(endpoint);
>>
>>          while (endpoint->transports != NULL)
>> @@ -659,6 +664,12 @@ static int set_config(struct a2dp_sep *sep, uint8_t *configuration,
>>          struct media_endpoint *endpoint = user_data;
>>          struct a2dp_config_data *data;
>>
>> +       active_streams++;
>> +       if (active_streams > btd_opts.a2dp.channels) {
>> +               DBG("A2DP channel limit (%u) exceeded", btd_opts.a2dp.channels);
>> +               return -1;
>> +       }
> This is a global limit? Not really following the reasoning here, if
> the platform don't want to connect to more devices just don't connect
> them, or in case of incoming connection don't authorize the
> connections, make the endpoints return an error to SetConfiguration,
> suspend the streams, etc. There are so many ways to block this that I
> don't really feel like introducing a new one will do us any good.
>
>>          data = g_new0(struct a2dp_config_data, 1);
>>          data->setup = setup;
>>          data->cb = cb;
>> --
>> 2.39.3 (Apple Git-146)
>>
>>
>