[PATCH BlueZ v2 1/2] client/player: Fix calculation of number of packet

[PATCH BlueZ v2 1/2] client/player: Fix calculation of number of packet

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

The calculation shall attempt to round to number of packets to the
closest integer otherwise it can result in 0 packets to be sent at each
latency.
---
 client/player.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/client/player.c b/client/player.c
index 1f56bfd270f6..65f771039258 100644
--- a/client/player.c
+++ b/client/player.c
@@ -63,6 +63,7 @@
 #define NSEC_USEC(_t) (_t / 1000L)
 #define SEC_USEC(_t)  (_t  * 1000000L)
 #define TS_USEC(_ts)  (SEC_USEC((_ts)->tv_sec) + NSEC_USEC((_ts)->tv_nsec))
+#define ROUND_CLOSEST(_x, _y) (((_x) + (_y / 2)) / (_y))
 
 #define EP_SRC_LOCATIONS 0x00000003
 #define EP_SNK_LOCATIONS 0x00000003
@@ -5031,8 +5032,9 @@ static bool transport_timer_read(struct io *io, void *user_data)
 		return false;
 	}
 
-	/* num of packets = latency (ms) / interval (us) */
-	num = (qos.ucast.out.latency * 1000 / qos.ucast.out.interval);
+	/* num of packets = ROUND_CLOSEST(latency (ms) / interval (us)) */
+	num = ROUND_CLOSEST(qos.ucast.out.latency * 1000,
+				qos.ucast.out.interval);
 
 	ret = transport_send_seq(transport, transport->fd, num);
 	if (ret < 0) {
-- 
2.44.0

[PATCH BlueZ v2 2/2] client/player: Fix using unicast QoS for broadcast

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

The code needs to identify first if the parameters refers to a broadcast
stream or unicast.
---
 client/player.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/client/player.c b/client/player.c
index 65f771039258..6b70e9ed3f9d 100644
--- a/client/player.c
+++ b/client/player.c
@@ -34,6 +34,7 @@
 
 #include "lib/bluetooth.h"
 #include "lib/uuid.h"
+#include "lib/iso.h"
 
 #include "profiles/audio/a2dp-codecs.h"
 #include "src/shared/lc3.h"
@@ -143,6 +144,7 @@ struct transport {
 	struct io *io;
 	uint32_t seq;
 	struct io *timer_io;
+	int num;
 };
 
 static void endpoint_unregister(void *data)
@@ -5008,7 +5010,6 @@ static bool transport_timer_read(struct io *io, void *user_data)
 	struct bt_iso_qos qos;
 	socklen_t len;
 	int ret, fd;
-	uint32_t num;
 	uint64_t exp;
 
 	if (transport->fd < 0)
@@ -5032,11 +5033,7 @@ static bool transport_timer_read(struct io *io, void *user_data)
 		return false;
 	}
 
-	/* num of packets = ROUND_CLOSEST(latency (ms) / interval (us)) */
-	num = ROUND_CLOSEST(qos.ucast.out.latency * 1000,
-				qos.ucast.out.interval);
-
-	ret = transport_send_seq(transport, transport->fd, num);
+	ret = transport_send_seq(transport, transport->fd, transport->num);
 	if (ret < 0) {
 		bt_shell_printf("Unable to send: %s (%d)\n",
 					strerror(-ret), ret);
@@ -5052,7 +5049,7 @@ static bool transport_timer_read(struct io *io, void *user_data)
 }
 
 static int transport_send(struct transport *transport, int fd,
-					struct bt_iso_qos *qos)
+					struct bt_iso_io_qos *qos)
 {
 	struct itimerspec ts;
 	int timer_fd;
@@ -5070,14 +5067,15 @@ static int transport_send(struct transport *transport, int fd,
 		return -errno;
 
 	memset(&ts, 0, sizeof(ts));
-	ts.it_value.tv_nsec = qos->ucast.out.latency * 1000000;
-	ts.it_interval.tv_nsec = qos->ucast.out.latency * 1000000;
+	ts.it_value.tv_nsec = qos->latency * 1000000;
+	ts.it_interval.tv_nsec = qos->latency * 1000000;
 
 	if (timerfd_settime(timer_fd, TFD_TIMER_ABSTIME, &ts, NULL) < 0)
 		return -errno;
 
 	transport->fd = fd;
-
+	/* num of packets = ROUND_CLOSEST(latency (ms) / interval (us)) */
+	transport->num = ROUND_CLOSEST(qos->latency * 1000, qos->interval);
 	transport->timer_io = io_new(timer_fd);
 
 	io_set_read_handler(transport->timer_io, transport_timer_read,
@@ -5131,8 +5129,20 @@ static void cmd_send_transport(int argc, char *argv[])
 			bt_shell_printf("Unable to getsockopt(BT_ISO_QOS): %s",
 							strerror(errno));
 			err = transport_send(transport, fd, NULL);
-		} else
-			err = transport_send(transport, fd, &qos);
+		} else {
+			struct sockaddr_iso addr;
+			socklen_t optlen;
+
+			err = getpeername(transport->sk, &addr, &optlen);
+			if (!err) {
+				if (!(bacmp(&addr.iso_bdaddr, BDADDR_ANY)))
+					err = transport_send(transport, fd,
+							     &qos.bcast.out);
+				else
+					err = transport_send(transport, fd,
+							     &qos.ucast.out);
+			}
+		}
 
 		if (err < 0) {
 			bt_shell_printf("Unable to send: %s (%d)",
-- 
2.44.0

RE: [BlueZ,v2,1/2] client/player: Fix calculation of number of packet

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 949 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=846226

---Test result---

Test Summary:
CheckPatch                    PASS      0.55 seconds
GitLint                       PASS      0.41 seconds
BuildEll                      PASS      24.69 seconds
BluezMake                     PASS      1676.42 seconds
MakeCheck                     PASS      12.84 seconds
MakeDistcheck                 PASS      178.48 seconds
CheckValgrind                 PASS      247.63 seconds
CheckSmatch                   PASS      353.43 seconds
bluezmakeextell               PASS      119.86 seconds
IncrementalBuild              PASS      2962.74 seconds
ScanBuild                     PASS      1017.77 seconds



---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ v2 1/2] client/player: Fix calculation of number of packet

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Fri, 19 Apr 2024 16:23:40 -0400 you wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> The calculation shall attempt to round to number of packets to the
> closest integer otherwise it can result in 0 packets to be sent at each
> latency.
> ---
>  client/player.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Here is the summary with links:
  - [BlueZ,v2,1/2] client/player: Fix calculation of number of packet
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=92c1c1c36611
  - [BlueZ,v2,2/2] client/player: Fix using unicast QoS for broadcast
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=04153538aaf4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH BlueZ v2 2/2] client/player: Fix using unicast QoS for broadcast

[Pauli Virtanen]

Hi,

pe, 2024-04-19 kello 16:23 -0400, Luiz Augusto von Dentz kirjoitti:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> The code needs to identify first if the parameters refers to a broadcast
> stream or unicast.
> ---
>  client/player.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/client/player.c b/client/player.c
> index 65f771039258..6b70e9ed3f9d 100644
> --- a/client/player.c
> +++ b/client/player.c
> @@ -34,6 +34,7 @@
>  
>  #include "lib/bluetooth.h"
>  #include "lib/uuid.h"
> +#include "lib/iso.h"
>  
>  #include "profiles/audio/a2dp-codecs.h"
>  #include "src/shared/lc3.h"
> @@ -143,6 +144,7 @@ struct transport {
>  	struct io *io;
>  	uint32_t seq;
>  	struct io *timer_io;
> +	int num;
>  };
>  
>  static void endpoint_unregister(void *data)
> @@ -5008,7 +5010,6 @@ static bool transport_timer_read(struct io *io, void *user_data)
>  	struct bt_iso_qos qos;
>  	socklen_t len;
>  	int ret, fd;
> -	uint32_t num;
>  	uint64_t exp;
>  
>  	if (transport->fd < 0)
> @@ -5032,11 +5033,7 @@ static bool transport_timer_read(struct io *io, void *user_data)
>  		return false;
>  	}
>  
> -	/* num of packets = ROUND_CLOSEST(latency (ms) / interval (us)) */
> -	num = ROUND_CLOSEST(qos.ucast.out.latency * 1000,
> -				qos.ucast.out.interval);
> -
> -	ret = transport_send_seq(transport, transport->fd, num);
> +	ret = transport_send_seq(transport, transport->fd, transport->num);
>  	if (ret < 0) {
>  		bt_shell_printf("Unable to send: %s (%d)\n",
>  					strerror(-ret), ret);
> @@ -5052,7 +5049,7 @@ static bool transport_timer_read(struct io *io, void *user_data)
>  }
>  
>  static int transport_send(struct transport *transport, int fd,
> -					struct bt_iso_qos *qos)
> +					struct bt_iso_io_qos *qos)
>  {
>  	struct itimerspec ts;
>  	int timer_fd;
> @@ -5070,14 +5067,15 @@ static int transport_send(struct transport *transport, int fd,
>  		return -errno;
>  
>  	memset(&ts, 0, sizeof(ts));
> -	ts.it_value.tv_nsec = qos->ucast.out.latency * 1000000;
> -	ts.it_interval.tv_nsec = qos->ucast.out.latency * 1000000;
> +	ts.it_value.tv_nsec = qos->latency * 1000000;
> +	ts.it_interval.tv_nsec = qos->latency * 1000000;
>  
>  	if (timerfd_settime(timer_fd, TFD_TIMER_ABSTIME, &ts, NULL) < 0)
>  		return -errno;
>  
>  	transport->fd = fd;
> -
> +	/* num of packets = ROUND_CLOSEST(latency (ms) / interval (us)) */
> +	transport->num = ROUND_CLOSEST(qos->latency * 1000, qos->interval);
>  	transport->timer_io = io_new(timer_fd);

This probably is intended to be writing at the average rate of one SDU
per SDU_Interval, so probably should be:

	transport->num = ...;

	ts.it_value.tv_nsec = transport->num * qos->interval * 1000;
	ts.it_interval.tv_nsec = transport->num * qos->interval * 1000;

Sorry for late comment.

>  
>  	io_set_read_handler(transport->timer_io, transport_timer_read,
> @@ -5131,8 +5129,20 @@ static void cmd_send_transport(int argc, char *argv[])
>  			bt_shell_printf("Unable to getsockopt(BT_ISO_QOS): %s",
>  							strerror(errno));
>  			err = transport_send(transport, fd, NULL);
> -		} else
> -			err = transport_send(transport, fd, &qos);
> +		} else {
> +			struct sockaddr_iso addr;
> +			socklen_t optlen;
> +
> +			err = getpeername(transport->sk, &addr, &optlen);
> +			if (!err) {
> +				if (!(bacmp(&addr.iso_bdaddr, BDADDR_ANY)))
> +					err = transport_send(transport, fd,
> +							     &qos.bcast.out);
> +				else
> +					err = transport_send(transport, fd,
> +							     &qos.ucast.out);
> +			}
> +		}
>  
>  		if (err < 0) {
>  			bt_shell_printf("Unable to send: %s (%d)",