Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-4535-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <9b88a0238679d24aa5d68a4c473483943a8ea2b6.1715427163.git.pav@iki.fi>
In-Reply-To: <9b88a0238679d24aa5d68a4c473483943a8ea2b6.1715427163.git.pav@iki.fi>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Sat, 11 May 2024 11:22:41 -0400
Message-ID: <CABBYNZ+zVn+GYG4sFubU4CMRkJBvhmjEX7UsAPROeQM1Pyjoqw@mail.gmail.com>
Subject: Re: [PATCH BlueZ] client: fix ISO send data rate
To: Pauli Virtanen <pav@iki.fi>
Cc: linux-bluetooth@vger.kernel.org, vlad.pruteanu@nxp.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Pauli,

On Sat, May 11, 2024 at 7:34=E2=80=AFAM 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 ~=3D 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 *trans=
port, int fd,
>         if (timer_fd < 0)
>                 return -errno;
>
> +       /* Send data in bursts of
> +        * num =3D ROUND_CLOSEST(Transport_Latency (ms) / SDU_Interval (u=
s))
> +        * with average data rate =3D 1 packet / SDU_Interval
> +        */
> +       transport->num =3D ROUND_CLOSEST(qos->latency * 1000, qos->interv=
al);
> +       if (!transport->num)
> +               transport->num =3D 1;
> +
>         memset(&ts, 0, sizeof(ts));
> -       ts.it_value.tv_nsec =3D qos->latency * 1000000;
> -       ts.it_interval.tv_nsec =3D qos->latency * 1000000;
> +       ts.it_value.tv_nsec =3D 1;
> +       ts.it_interval.tv_nsec =3D 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 =3D fd;
> -       /* num of packets =3D ROUND_CLOSEST(latency (ms) / interval (us))=
 */
> -       transport->num =3D ROUND_CLOSEST(qos->latency * 1000, qos->interv=
al);
>         transport->timer_io =3D 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
>
>


--=20
Luiz Augusto von Dentz