[BlueZ,v2] client: fix ISO send data rate

Commit Message

Pauli Virtanen May 11, 2024, 3:45 p.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.
---

Notes:
    v2: Buffer one interval immediately as before.
    
    This assumes kernel shall set qos.interval to SDU_Interval and not
    ISO_Interval.

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

Comments

[e-mail] May 11, 2024, 5:43 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=852558

---Test result---

Test Summary:
CheckPatch                    PASS      0.45 seconds
GitLint                       FAIL      0.48 seconds
BuildEll                      PASS      24.55 seconds
BluezMake                     PASS      1719.88 seconds
MakeCheck                     PASS      13.12 seconds
MakeDistcheck                 PASS      176.82 seconds
CheckValgrind                 PASS      251.27 seconds
CheckSmatch                   PASS      353.65 seconds
bluezmakeextell               PASS      120.99 seconds
IncrementalBuild              PASS      1503.96 seconds
ScanBuild                     PASS      1033.85 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,v2] client: fix ISO send data rate

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
22: B2 Line has trailing whitespace: "    "


---
Regards,
Linux Bluetooth

[e-mail] May 13, 2024, 2:10 p.m. UTC | #2

Hello:

This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[e-mail]>:

On Sat, 11 May 2024 18:45:46 +0300 you 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.
> 
> [...]

Here is the summary with links:
  - [BlueZ,v2] client: fix ISO send data rate
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=e453a336c404

You are awesome, thank you!

Patch

diff --git a/client/player.c b/client/player.c
index 7f67425aa..df5cb8924 100644
--- a/client/player.c
+++ b/client/player.c
@@ -5066,21 +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);
 
+	/* One extra packet to buffers immediately */
 	return transport_send_seq(transport, fd, 1);
 }