diff mbox series

[BlueZ] client: fix ISO send data rate

Message ID 9b88a0238679d24aa5d68a4c473483943a8ea2b6.1715427163.git.pav@iki.fi (mailing list archive)
State Superseded
Headers show
Series [BlueZ] client: fix ISO send data rate | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint success Gitlint PASS
tedd_an/BuildEll success Build ELL PASS
tedd_an/BluezMake success Bluez Make PASS
tedd_an/MakeCheck success Bluez Make Check PASS
tedd_an/MakeDistcheck success Make Distcheck PASS
tedd_an/CheckValgrind success Check Valgrind PASS
tedd_an/CheckSmatch success CheckSparse PASS
tedd_an/bluezmakeextell success Make External ELL PASS
tedd_an/IncrementalBuild success Incremental Build PASS
tedd_an/ScanBuild success Scan Build PASS

Commit Message

Pauli Virtanen May 11, 2024, 11:34 a.m. UTC
We are sending data to controller at wrong average rate not equal to
1 packet / SDU interval, if Transport_Latency is not an integer multiple
of SDU_Interval.  The calculation currently may also give zero, so no
data gets sent.

We are sending data in bursts of num ~= Transport_Latency/SDU_Interval
packets, in hopes that possibly larger timer interval makes things more
efficient.

Fix the data rate by sending num packets every num*SDU_Interval, so that
the average data rate is correct.

Also fix use of itimerspect.it_value with TFD_TIMER_ABSTIME.  The value
set previously is going to always be in the past in CLOCK_MONOTONIC so
just set it to 1, and leave sending the initial packets to the main
loop.
---

Notes:
    This assumes kernel shall set qos.interval to SDU_Interval and not
    ISO_Interval.

 client/player.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

Comments

bluez.test.bot@gmail.com May 11, 2024, 1:51 p.m. UTC | #1
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=852520

---Test result---

Test Summary:
CheckPatch                    PASS      0.41 seconds
GitLint                       PASS      0.29 seconds
BuildEll                      PASS      24.81 seconds
BluezMake                     PASS      1755.50 seconds
MakeCheck                     PASS      13.61 seconds
MakeDistcheck                 PASS      181.36 seconds
CheckValgrind                 PASS      252.17 seconds
CheckSmatch                   PASS      359.45 seconds
bluezmakeextell               PASS      122.46 seconds
IncrementalBuild              PASS      1490.77 seconds
ScanBuild                     PASS      1058.36 seconds



---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz May 11, 2024, 3:22 p.m. UTC | #2
Hi Pauli,

On Sat, May 11, 2024 at 7:34 AM Pauli Virtanen <pav@iki.fi> wrote:
>
> We are sending data to controller at wrong average rate not equal to
> 1 packet / SDU interval, if Transport_Latency is not an integer multiple
> of SDU_Interval.  The calculation currently may also give zero, so no
> data gets sent.
>
> We are sending data in bursts of num ~= Transport_Latency/SDU_Interval
> packets, in hopes that possibly larger timer interval makes things more
> efficient.
>
> Fix the data rate by sending num packets every num*SDU_Interval, so that
> the average data rate is correct.
>
> Also fix use of itimerspect.it_value with TFD_TIMER_ABSTIME.  The value
> set previously is going to always be in the past in CLOCK_MONOTONIC so
> just set it to 1, and leave sending the initial packets to the main
> loop.
> ---
>
> Notes:
>     This assumes kernel shall set qos.interval to SDU_Interval and not
>     ISO_Interval.
>
>  client/player.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/client/player.c b/client/player.c
> index 7f67425aa..8d17022de 100644
> --- a/client/player.c
> +++ b/client/player.c
> @@ -5066,22 +5066,30 @@ static int transport_send(struct transport *transport, int fd,
>         if (timer_fd < 0)
>                 return -errno;
>
> +       /* Send data in bursts of
> +        * num = ROUND_CLOSEST(Transport_Latency (ms) / SDU_Interval (us))
> +        * with average data rate = 1 packet / SDU_Interval
> +        */
> +       transport->num = ROUND_CLOSEST(qos->latency * 1000, qos->interval);
> +       if (!transport->num)
> +               transport->num = 1;
> +
>         memset(&ts, 0, sizeof(ts));
> -       ts.it_value.tv_nsec = qos->latency * 1000000;
> -       ts.it_interval.tv_nsec = qos->latency * 1000000;
> +       ts.it_value.tv_nsec = 1;
> +       ts.it_interval.tv_nsec = transport->num * qos->interval * 1000;
>
> -       if (timerfd_settime(timer_fd, TFD_TIMER_ABSTIME, &ts, NULL) < 0)
> +       if (timerfd_settime(timer_fd, TFD_TIMER_ABSTIME, &ts, NULL) < 0) {
> +               close(timer_fd);
>                 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,
>                                                 transport, NULL);
>
> -       return transport_send_seq(transport, fd, 1);

The above was actually done on purpose so we are always one interval
ahead to keep controller buffer full as much as possible.

> +       return 0;
>  }
>
>  static void cmd_send_transport(int argc, char *argv[])
> --
> 2.45.0
>
>
diff mbox series

Patch

diff --git a/client/player.c b/client/player.c
index 7f67425aa..8d17022de 100644
--- a/client/player.c
+++ b/client/player.c
@@ -5066,22 +5066,30 @@  static int transport_send(struct transport *transport, int fd,
 	if (timer_fd < 0)
 		return -errno;
 
+	/* Send data in bursts of
+	 * num = ROUND_CLOSEST(Transport_Latency (ms) / SDU_Interval (us))
+	 * with average data rate = 1 packet / SDU_Interval
+	 */
+	transport->num = ROUND_CLOSEST(qos->latency * 1000, qos->interval);
+	if (!transport->num)
+		transport->num = 1;
+
 	memset(&ts, 0, sizeof(ts));
-	ts.it_value.tv_nsec = qos->latency * 1000000;
-	ts.it_interval.tv_nsec = qos->latency * 1000000;
+	ts.it_value.tv_nsec = 1;
+	ts.it_interval.tv_nsec = transport->num * qos->interval * 1000;
 
-	if (timerfd_settime(timer_fd, TFD_TIMER_ABSTIME, &ts, NULL) < 0)
+	if (timerfd_settime(timer_fd, TFD_TIMER_ABSTIME, &ts, NULL) < 0) {
+		close(timer_fd);
 		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,
 						transport, NULL);
 
-	return transport_send_seq(transport, fd, 1);
+	return 0;
 }
 
 static void cmd_send_transport(int argc, char *argv[])