Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-2684-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Silviu Florian Barbulescu <silviu.barbulescu@nxp.com>
To: luiz.dentz@gmail.com
Cc: andrei.istodorescu@nxp.com,
	iulia.tanasescu@nxp.com,
	linux-bluetooth@vger.kernel.org,
	mihai-octavian.urzica@nxp.com,
	silviu.barbulescu@nxp.com,
	vlad.pruteanu@nxp.com
Subject: Re: [PATCH BlueZ 2/2] player: Allow endpoint config without local endpoint argument
Date: Thu, 21 Mar 2024 13:33:59 +0200
Message-Id: <20240321113359.12475-1-silviu.barbulescu@nxp.com>
In-Reply-To: <CABBYNZL-JFqb5wq-k4Ge2zOVPf71E3pVsnu5X-h4LOy1tziNCQ@mail.gmail.com>
References: <CABBYNZL-JFqb5wq-k4Ge2zOVPf71E3pVsnu5X-h4LOy1tziNCQ@mail.gmail.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?Vx8ZdDgzfsU51+pjorlIzf9ut4A1BeLv8n8KVTaZLYj5DXk3NIm18HF9mjCk?=
 =?us-ascii?Q?PGe4sIamiqFH7FDtHoQHHhf2Cm8Mto/F+bRCCwCrvRkF7/1sdnNfCgd4T3V8?=
 =?us-ascii?Q?+U82De7AiCeWR9SI+ThEg7oxaCue9czo0a2A6vHHGK49BEiVK21IVsVDMxdE?=
 =?us-ascii?Q?l4nHETpjgqPy86wH6kkJtZxRSOm+WFNRDG0GHEZSr3NVpAiXyNdkRzRloINY?=
 =?us-ascii?Q?UZoiO4L5/SHLTNVy4v5mCsTxCZg+Qx41ydDD11MNVHZdAnq+g3TJyUTwUAjn?=
 =?us-ascii?Q?0+0JfsMfLm/VeaMAViCbVE2vKZEPATZGjPlIuvBpzyaEQLXNBmi5HbwANeQj?=
 =?us-ascii?Q?ibmr1lh8HLztvjsRGPRUBYVgXyQx248gSre0gppXsRAdVP01XsJXXR6qUFoa?=
 =?us-ascii?Q?M1CCxn0a7nhWYquJ0psmjf9hc9wm39hadszd+jKrV1ffPOHH0TjsXec6AZ1A?=
 =?us-ascii?Q?Ujz2GqTUUUSVKoUtqqNGeRQDnvQHUDJCGcoVm8DOY+s67iVtmmb+d813pSGo?=
 =?us-ascii?Q?DPPSlieiiffotP5dtHmC3wvxEIIKP6HVXdkLpSBbQMv/KBK2ubJv/Jl6p6GY?=
 =?us-ascii?Q?eE5PnyHni4V8qUBhBtwEpBpQ/pRarg54uvRnMPEeoGTeiTxCAjZV0eY6Syof?=
 =?us-ascii?Q?6LT/r6pLrcXbGyk7jrG96hhlDWiCc7nA8qLCZ3GzQa/f4DKaa6b9tDpkSZIB?=
 =?us-ascii?Q?LUao7Xw+1D7/qm04ENqWZKuJDAwUxn5i3fdp/v26Ln3lP2OVXoF6xqfMR8D6?=
 =?us-ascii?Q?Nf0LIElf3/wNE/EItpemJ82ICMyOdA/KGPXQ5PlmLARN0Gq73x5mfBoLXYAm?=
 =?us-ascii?Q?3QpYIXpMOyv2uicHKFq5phorBy0FKBeUfi7Fb8Oto4KMzX0OfuYbkuAWz863?=
 =?us-ascii?Q?7Ia7EKmOse3memMm5MCf9Mf5j7BqHiVTESpxKqk9pMZWo1qi8f4fV/uVOmWW?=
 =?us-ascii?Q?i9mEnn5mRzTo/O87wRW+cTIBIDGIP8SOF6aWstj4Nn7Vn7KyYwRkzADpPDss?=
 =?us-ascii?Q?24UV7Zy6P9LnHiewiIVmGfKXRpVZAcc1gKINUweocGVgbroM2sl1pyzunE8f?=
 =?us-ascii?Q?o+neJNywLg0gf79wlXTZLcCaAjn0GMccqLjoWn2FqmJN8BdhVfTnQKJLFcoj?=
 =?us-ascii?Q?w0h1heYtGrMIHj+XtrqBI6g3JAQLC+iLfoghdW/tjavIifif5ZDVkz043cxV?=
 =?us-ascii?Q?GOw6hgcw2iF2te/v8NH1zqqMZlrpvictKMIaRW6BbzhltYmvGt9Y8HNrsiVz?=
 =?us-ascii?Q?OBRkj/7Xu2alrPSOim/VYIR7J0iV2FtzjmCwIOTHLqiV6vQE5/w00bsGH7O4?=
 =?us-ascii?Q?gYA8oUpiq042MX6d+Bz0MlG7X7MrAkUBY7YG4WgdPQxwXVocDjYLHJQlqano?=
 =?us-ascii?Q?hLMPykVEObELxXA10Y34CzgX9i0Zn1Ls5GvazZ6tj0gKdh4YC88xHORAtEBL?=
 =?us-ascii?Q?E/jdpsKHEAn0pJnviHC7ir6Yfk5U1p0cmT4Yee9L8qFO22S9oYzvFUlWP9D9?=
 =?us-ascii?Q?nV8VnguexoNVWudIhCfOcHLyXgmNlvjlkR/4ATrWABKjUVQcgk0aiFiCx5Dz?=
 =?us-ascii?Q?cdP0PXvRLSd/x4KSBYe61TeBta6LgIl4WLQluXLP0BhZ1SQi28pX6aMTcw5L?=
 =?us-ascii?Q?lg=3D=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4490472c-c38a-4381-3ec4-08dc499ace32
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB10073.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Mar 2024 11:34:01.4664
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Wi/ftypgTRCZ3MfTLzsvYaCQPjRxj5gDOrXwPeGSXnrnGi11+N4GfCjhWQroCie9Dv9vCrgZH+H9oy7CyjDy7J/kknjv2Gg6ZRA1pg+P39M=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB7573

Hi Luiz, 

> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Wednesday, March 20, 2024 7:33 PM
> To: Silviu Florian Barbulescu <silviu.barbulescu@nxp.com>
> Cc: linux-bluetooth@vger.kernel.org; Mihai-Octavian Urzica <mihai-
> octavian.urzica@nxp.com>; Vlad Pruteanu <vlad.pruteanu@nxp.com>; Andrei
> Istodorescu <andrei.istodorescu@nxp.com>; Iulia Tanasescu
> <iulia.tanasescu@nxp.com>
> Subject: [EXT] Re: [PATCH BlueZ 2/2] player: Allow endpoint config without local
> endpoint argument
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
> 
> 
> Hi Silviu,
> 
> On Wed, Mar 20, 2024 at 2:42PM Silviu Florian Barbulescu
> <silviu.barbulescu@nxp.com> wrote:
> >
> > Update endpoint config command to create local endpoint for broadcast
> > endpoint, this will be used to configure a broadcast source\sink if
> > register endpoint is done from another application like PipeWire
> 
> Sorry but will not gona fly, we don't have any business with endpoints from
> other process in the system, in fact we might actually make it strictly prohibit
> for third-party process to use SetConfiguration with endpoint that do not belong
> their own D-Bus connection, same for MediaTransport, they shall only be
> acquired by the process that created them.
> 
> Also I remember quite clearly stating that we would move away from using
> MediaEndpoint for Broadcast Sink, instead what we shall be doing is
> enumerating BIS as MediaTransport directly so the likes of pipewire can Acquire
> them directly without doing any configuration since they are already configured
> over the air. Broadcast source will probably require a special interface at
> pipewire side, or perhaps a dedicated card which gets configured via some
> configuration file, etc, but we shall not depend on bluetoothctl to configured it,
> that will just make things more complicated than really helping.
> 

We are investigating a solution with the broadcast source configuration
from PipeWire and also removing the MediaEndpoint reliance for the broadcast sink.

> > ---
> >  client/player.c | 80
> > +++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 77 insertions(+), 3 deletions(-)
> >
> > diff --git a/client/player.c b/client/player.c index
> > ab33bfc46..b02a40423 100644
> > --- a/client/player.c
> > +++ b/client/player.c
> > @@ -3662,10 +3662,15 @@ static void endpoint_set_config_bcast(struct
> endpoint_config *cfg)
> >                 config_endpoint_iso_group, cfg);  }
> >
> > +static void endpoint_init_defaults(struct endpoint *ep);
> >  static void cmd_config_endpoint(int argc, char *argv[])  {
> > +       DBusMessageIter iter;
> >         struct endpoint_config *cfg;
> > -       const struct codec_preset *preset;
> > +       const struct codec_preset *preset = NULL;
> > +       const struct capabilities *cap;
> > +       uint8_t codec;
> > +       const char *uuid;
> >
> >         cfg = new0(struct endpoint_config, 1);
> >
> > @@ -3680,8 +3685,75 @@ static void cmd_config_endpoint(int argc, char
> *argv[])
> >         /* Search for the local endpoint */
> >         cfg->ep = endpoint_find(argv[2]);
> >         if (!cfg->ep) {
> > -               bt_shell_printf("Local Endpoint %s not found\n", argv[2]);
> > -               goto fail;
> > +               /* If argc != 3 then argv[2] should be the
> > +                * local endpoint. Return error.
> > +                */
> > +               if (argc != 3) {
> > +                       bt_shell_printf("Local Endpoint %s not found\n",
> > +                                       argv[2]);
> > +                       goto fail;
> > +               }
> > +
> > +               /* If local endpoint is not found verify if this is a
> > +                * broadcast remote endpoint because this can be an
> > +                * endpoint config command with remote endpoint and
> > +                * preset. In this case we have to create its own
> > +                * local endpoint.
> > +                */
> > +               if (!g_dbus_proxy_get_property(cfg->proxy, "UUID", &iter))
> > +                       return;
> > +               dbus_message_iter_get_basic(&iter, &uuid);
> > +
> > +               /* The local endpoint must have the UUID of local pac but
> > +                * the remote endpoint has the UUID of the remote pac, so
> > +                * based on this information we determine the UUID for the
> > +                * local endpoint.
> > +                */
> > +               if (!strcmp(uuid, BAA_SERVICE_UUID))
> > +                       uuid = BCAA_SERVICE_UUID;
> > +               else if (!strcmp(uuid, BCAA_SERVICE_UUID))
> > +                       uuid = BAA_SERVICE_UUID;
> > +               else {
> > +                       /* This should be a local endpoint because is not
> > +                        * a broadcast endpoint.
> > +                        */
> > +                       bt_shell_printf("Local Endpoint %s not found\n",
> > +                                       argv[2]);
> > +                       goto fail;
> > +               }
> > +
> > +               if (!g_dbus_proxy_get_property(cfg->proxy, "Codec", &iter))
> > +                       return;
> > +
> > +               dbus_message_iter_get_basic(&iter, &codec);
> > +
> > +               /* Verify if the local endpoint already exists */
> > +               cfg->ep = endpoint_find(uuid);
> > +               if ((!cfg->ep) || (cfg->ep->codec != codec)) {
> > +                       /* Get capabilities to create the local endpoint */
> > +                       cap = find_capabilities(uuid, codec);
> > +                       if (!cap) {
> > +                               bt_shell_printf(
> > +                                       "Capabilities not found for UUID %s, codec id %d",
> > +                                       uuid, codec);
> > +                               goto fail;
> > +                       }
> > +
> > +                       cfg->ep = endpoint_new(cap);
> > +                       endpoint_init_defaults(cfg->ep);
> > +               }
> > +
> > +               /* Verify if argv[2] parameter is a valid preset */
> > +               preset = preset_find_name(cfg->ep->preset, argv[2]);
> > +               if (!preset) {
> > +                       bt_shell_printf("Preset %s not found\n",
> > +                               argv[2]);
> > +                       /* Free created endpoint */
> > +                       local_endpoints = g_list_remove(local_endpoints,
> > +                                               cfg->ep);
> > +                       endpoint_free(cfg->ep);
> > +                       goto fail;
> > +               }
> >         }
> >
> >         if (argc > 3) {
> > @@ -3690,7 +3762,9 @@ static void cmd_config_endpoint(int argc, char
> *argv[])
> >                         bt_shell_printf("Preset %s not found\n", argv[3]);
> >                         goto fail;
> >                 }
> > +       }
> >
> > +       if (preset) {
> >                 cfg->caps = g_new0(struct iovec, 1);
> >                 /* Copy capabilities */
> >                 util_iov_append(cfg->caps, preset->data.iov_base,
> > --
> > 2.39.2
> >
> 
> 
> --
> Luiz Augusto von Dentz