[PATCH BlueZ 0/2] profiles: Add remote endpoint path to SelectProperties

[PATCH BlueZ 0/2] profiles: Add remote endpoint path to SelectProperties

[Frédéric Danis]

The SelectProperties method is only called on the central (initiator) device.
But there's no information related to the remote device for which is call is
done.
These commits allow the audio server to link this call methos to the
appropriate remote endpoint.

Frédéric Danis (2):
  profiles: Add remote endpoint path to SelectProperties
  doc: Add remote endpoint path to SelectProperties

 doc/media-api.txt      | 2 +-
 profiles/audio/bap.c   | 2 +-
 profiles/audio/media.c | 3 +++
 src/shared/bap.c       | 2 ++
 src/shared/bap.h       | 2 ++
 5 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH BlueZ 1/2] profiles: Add remote endpoint path to SelectProperties

[Frédéric Danis]

---
 profiles/audio/bap.c   | 2 +-
 profiles/audio/media.c | 3 +++
 src/shared/bap.c       | 2 ++
 src/shared/bap.h       | 2 ++
 4 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index d388afe56..cf27ec0ae 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -613,7 +613,7 @@ static bool pac_found(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
 
 	/* TODO: Cache LRU? */
 	if (btd_service_is_initiator(service))
-		bt_bap_select(lpac, rpac, select_cb, ep);
+		bt_bap_select(lpac, rpac, ep->path, select_cb, ep);
 
 	return true;
 }
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index ff3fa197b..8d777eedd 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -891,6 +891,7 @@ done:
 
 static int pac_select(struct bt_bap_pac *pac, struct bt_bap_pac_qos *qos,
 			struct iovec *caps, struct iovec *metadata,
+			const char *remote_ep_path,
 			bt_bap_pac_select_t cb, void *cb_data, void *user_data)
 {
 	struct media_endpoint *endpoint = user_data;
@@ -917,6 +918,8 @@ static int pac_select(struct bt_bap_pac *pac, struct bt_bap_pac_qos *qos,
 
 	dbus_message_iter_init_append(msg, &iter);
 
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, &remote_ep_path);
+
 	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, "{sv}", &dict);
 
 	g_dbus_dict_append_basic_array(&dict, DBUS_TYPE_STRING, &key,
diff --git a/src/shared/bap.c b/src/shared/bap.c
index 8edc7b72e..691fec2fa 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -4058,6 +4058,7 @@ static bool match_pac(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
 }
 
 int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
+			const char *remote_ep_path,
 			bt_bap_pac_select_t func, void *user_data)
 {
 	if (!lpac || !rpac || !func)
@@ -4067,6 +4068,7 @@ int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
 		return -EOPNOTSUPP;
 
 	lpac->ops->select(lpac, &rpac->qos, rpac->data, rpac->metadata,
+					remote_ep_path,
 					func, user_data, lpac->user_data);
 
 	return 0;
diff --git a/src/shared/bap.h b/src/shared/bap.h
index ff4bac330..da5fe1431 100644
--- a/src/shared/bap.h
+++ b/src/shared/bap.h
@@ -122,6 +122,7 @@ struct bt_bap_pac *bt_bap_add_pac(struct gatt_db *db, const char *name,
 struct bt_bap_pac_ops {
 	int (*select) (struct bt_bap_pac *pac, struct bt_bap_pac_qos *qos,
 			struct iovec *caps, struct iovec *metadata,
+			const char *remote_ep_path,
 			bt_bap_pac_select_t cb, void *cb_data, void *user_data);
 	int (*config) (struct bt_bap_stream *stream, struct iovec *cfg,
 			struct bt_bap_qos *qos, bt_bap_pac_config_t cb,
@@ -188,6 +189,7 @@ int bt_bap_pac_get_codec(struct bt_bap_pac *pac, uint8_t *id,
 
 /* Stream related functions */
 int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
+			const char *remote_ep_path,
 			bt_bap_pac_select_t func, void *user_data);
 
 struct bt_bap_stream *bt_bap_config(struct bt_bap *bap,
-- 
2.25.1

[PATCH BlueZ 2/2] doc: Add remote endpoint path to SelectProperties

[Frédéric Danis]

---
 doc/media-api.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/media-api.txt b/doc/media-api.txt
index 9cd211355..1866ecfcb 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -598,7 +598,7 @@ Methods		void SetConfiguration(object transport, dict properties)
 			configuration since on success the configuration is
 			send back as parameter of SetConfiguration.
 
-		dict SelectProperties(dict properties)
+		dict SelectProperties(object remote_endpoint, dict properties)
 
 			Select preferable properties from the supported
 			properties. Refer to SetConfiguration for the list of
-- 
2.25.1

RE: profiles: Add remote endpoint path to SelectProperties

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 3091 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=672169

---Test result---

Test Summary:
CheckPatch                    FAIL      2.95 seconds
GitLint                       PASS      2.01 seconds
Prep - Setup ELL              PASS      27.91 seconds
Build - Prep                  PASS      0.84 seconds
Build - Configure             PASS      8.96 seconds
Build - Make                  PASS      1039.25 seconds
Make Check                    PASS      11.03 seconds
Make Check w/Valgrind         PASS      276.22 seconds
Make Distcheck                PASS      238.53 seconds
Build w/ext ELL - Configure   PASS      8.61 seconds
Build w/ext ELL - Make        PASS      82.44 seconds
Incremental Build w/ patches  PASS      201.30 seconds
Scan Build                    WARNING   580.56 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
[BlueZ,1/2] profiles: Add remote endpoint path to SelectProperties
WARNING:LONG_LINE: line length of 86 exceeds 80 columns
#99: FILE: profiles/audio/media.c:921:
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, &remote_ep_path);

/github/workspace/src/12958401.patch total: 0 errors, 1 warnings, 51 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12958401.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: Scan Build - WARNING
Desc: Run Scan Build with patches
Output:
*****************************************************************************
The bugs reported by the scan-build may or may not be caused by your patches.
Please check the list and fix the bugs if they are caused by your patch.
*****************************************************************************
profiles/audio/media.c:1453:6: warning: 8th function call argument is an uninitialized value
        if (media_endpoint_create(adapter, sender, path, uuid, delay_reporting,
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/audio/media.c:2999:3: warning: Use of memory after it is freed
                release_endpoint(adapter->endpoints->data);
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/audio/media.c:3002:3: warning: Use of memory after it is freed
                media_player_destroy(adapter->players->data);
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.




---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ 1/2] profiles: Add remote endpoint path to SelectProperties

[Luiz Augusto von Dentz]

Hi Frédéric,

On Mon, Aug 29, 2022 at 1:36 PM Frédéric Danis
<frederic.danis@collabora.com> wrote:
>
> ---
>  profiles/audio/bap.c   | 2 +-
>  profiles/audio/media.c | 3 +++
>  src/shared/bap.c       | 2 ++
>  src/shared/bap.h       | 2 ++
>  4 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
> index d388afe56..cf27ec0ae 100644
> --- a/profiles/audio/bap.c
> +++ b/profiles/audio/bap.c
> @@ -613,7 +613,7 @@ static bool pac_found(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
>
>         /* TODO: Cache LRU? */
>         if (btd_service_is_initiator(service))
> -               bt_bap_select(lpac, rpac, select_cb, ep);
> +               bt_bap_select(lpac, rpac, ep->path, select_cb, ep);
>
>         return true;
>  }
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> index ff3fa197b..8d777eedd 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -891,6 +891,7 @@ done:
>
>  static int pac_select(struct bt_bap_pac *pac, struct bt_bap_pac_qos *qos,
>                         struct iovec *caps, struct iovec *metadata,
> +                       const char *remote_ep_path,
>                         bt_bap_pac_select_t cb, void *cb_data, void *user_data)
>  {
>         struct media_endpoint *endpoint = user_data;
> @@ -917,6 +918,8 @@ static int pac_select(struct bt_bap_pac *pac, struct bt_bap_pac_qos *qos,
>
>         dbus_message_iter_init_append(msg, &iter);
>
> +       dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, &remote_ep_path);
> +

If the endpoint object is not part of the dictionary then you will
have to fix client/player.c as well since the signature will be
changing, so I think it is better we include it as part of properties
itself.

>         dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, "{sv}", &dict);
>
>         g_dbus_dict_append_basic_array(&dict, DBUS_TYPE_STRING, &key,
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index 8edc7b72e..691fec2fa 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -4058,6 +4058,7 @@ static bool match_pac(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
>  }
>
>  int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> +                       const char *remote_ep_path,
>                         bt_bap_pac_select_t func, void *user_data)
>  {
>         if (!lpac || !rpac || !func)
> @@ -4067,6 +4068,7 @@ int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
>                 return -EOPNOTSUPP;
>
>         lpac->ops->select(lpac, &rpac->qos, rpac->data, rpac->metadata,
> +                                       remote_ep_path,
>                                         func, user_data, lpac->user_data);
>
>         return 0;
> diff --git a/src/shared/bap.h b/src/shared/bap.h
> index ff4bac330..da5fe1431 100644
> --- a/src/shared/bap.h
> +++ b/src/shared/bap.h
> @@ -122,6 +122,7 @@ struct bt_bap_pac *bt_bap_add_pac(struct gatt_db *db, const char *name,
>  struct bt_bap_pac_ops {
>         int (*select) (struct bt_bap_pac *pac, struct bt_bap_pac_qos *qos,
>                         struct iovec *caps, struct iovec *metadata,
> +                       const char *remote_ep_path,

Hmm, Id avoid passing D-Bus specific path here since this API is
suppose to be generic and at some point we might want to create a unit
tests that don't involve the daemon, so instead I think it is better
that we pass the rpac and addition to the lpac and then perhaps have a
function which can attach custom user_data to the rpac e.g.: void
bt_bap_pac_set_user_data(void *); void *bt_bap_pac_get_user_data();

>                         bt_bap_pac_select_t cb, void *cb_data, void *user_data);
>         int (*config) (struct bt_bap_stream *stream, struct iovec *cfg,
>                         struct bt_bap_qos *qos, bt_bap_pac_config_t cb,
> @@ -188,6 +189,7 @@ int bt_bap_pac_get_codec(struct bt_bap_pac *pac, uint8_t *id,
>
>  /* Stream related functions */
>  int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> +                       const char *remote_ep_path,
>                         bt_bap_pac_select_t func, void *user_data);
>
>  struct bt_bap_stream *bt_bap_config(struct bt_bap *bap,
> --
> 2.25.1
>


-- 
Luiz Augusto von Dentz