bluez5-device, bluez5-discover: add hsp_source_buffer_msec argument

Submitted by Georg Chini on April 9, 2018, 4:16 p.m.

Details

Message ID 20180409161649.17194-1-georg@chini.tk
State New
Series "bluez5-device, bluez5-discover: add hsp_source_buffer_msec argument"
Headers show

Commit Message

Georg Chini April 9, 2018, 4:16 p.m.
Currently the PA bluetooth code calls source_push() for each HSP source packet.
The default HSP MTU is 48 bytes, this means that source_push() is called every
3ms, which leads to increased CPU load especially on embedded systems.

This patch adds a hsp_source_buffer_msec argument to module-bluez5-discover and
module-bluez5-device. The argument gives the number of milliseconds of audio which
are buffered, before source_push() is called. The value can range from 0 to 100ms,
and is rounded down to the next multiple of the MTU size. Therefore a value of less
than 2*MTU time corresponds to the original behavior.
---
 src/modules/bluetooth/module-bluetooth-discover.c |  1 +
 src/modules/bluetooth/module-bluez5-device.c      | 68 +++++++++++++++++------
 src/modules/bluetooth/module-bluez5-discover.c    | 14 ++++-
 3 files changed, 64 insertions(+), 19 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/modules/bluetooth/module-bluetooth-discover.c b/src/modules/bluetooth/module-bluetooth-discover.c
index 63195d3e..14c0a38f 100644
--- a/src/modules/bluetooth/module-bluetooth-discover.c
+++ b/src/modules/bluetooth/module-bluetooth-discover.c
@@ -32,6 +32,7 @@  PA_MODULE_LOAD_ONCE(true);
 PA_MODULE_USAGE(
     "headset=ofono|native|auto (bluez 5 only)"
     "autodetect_mtu=<boolean> (bluez 5 only)"
+    "hsp_source_buffer_msec=<0 - 100> (bluez5 only)"
 );
 
 struct userdata {
diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
index b81c233c..d40bbb0c 100644
--- a/src/modules/bluetooth/module-bluez5-device.c
+++ b/src/modules/bluetooth/module-bluez5-device.c
@@ -54,7 +54,8 @@  PA_MODULE_DESCRIPTION("BlueZ 5 Bluetooth audio sink and source");
 PA_MODULE_VERSION(PACKAGE_VERSION);
 PA_MODULE_LOAD_ONCE(false);
 PA_MODULE_USAGE("path=<device object path>"
-                "autodetect_mtu=<boolean>");
+                "autodetect_mtu=<boolean>"
+                "hsp_source_buffer_msec=<0 - 100>");
 
 #define FIXED_LATENCY_PLAYBACK_A2DP (25 * PA_USEC_PER_MSEC)
 #define FIXED_LATENCY_PLAYBACK_SCO  (25 * PA_USEC_PER_MSEC)
@@ -68,6 +69,7 @@  PA_MODULE_USAGE("path=<device object path>"
 static const char* const valid_modargs[] = {
     "path",
     "autodetect_mtu",
+    "hsp_source_buffer_msec",
     NULL
 };
 
@@ -123,6 +125,9 @@  struct userdata {
     pa_card *card;
     pa_sink *sink;
     pa_source *source;
+    uint32_t source_buffer_usec;
+    uint32_t source_buffered_blocks;
+    pa_memchunk source_buffer;
     pa_bluetooth_profile_t profile;
     char *output_port_name;
     char *input_port_name;
@@ -314,10 +319,10 @@  static int sco_process_render(struct userdata *u) {
 /* Run from IO thread */
 static int sco_process_push(struct userdata *u) {
     ssize_t l;
-    pa_memchunk memchunk;
     struct cmsghdr *cm;
     struct msghdr m;
     bool found_tstamp = false;
+    uint32_t max_blocks;
     pa_usec_t tstamp = 0;
 
     pa_assert(u);
@@ -326,11 +331,17 @@  static int sco_process_push(struct userdata *u) {
     pa_assert(u->source);
     pa_assert(u->read_smoother);
 
-    memchunk.memblock = pa_memblock_new(u->core->mempool, u->read_block_size);
-    memchunk.index = memchunk.length = 0;
+    max_blocks = u->source_buffer_usec / pa_bytes_to_usec(u->read_block_size, &u->source->sample_spec);
+    if (max_blocks == 0)
+        max_blocks = 1;
+
+    if (!u->source_buffer.memblock) {
+        u->source_buffer.memblock = pa_memblock_new(u->core->mempool, max_blocks * u->read_block_size);
+        u->source_buffer.index = u->source_buffer.length = 0;
+    }
 
     for (;;) {
-        void *p;
+        uint8_t *p;
         uint8_t aux[1024];
         struct iovec iov;
 
@@ -343,11 +354,11 @@  static int sco_process_push(struct userdata *u) {
         m.msg_control = aux;
         m.msg_controllen = sizeof(aux);
 
-        p = pa_memblock_acquire(memchunk.memblock);
-        iov.iov_base = p;
-        iov.iov_len = pa_memblock_get_length(memchunk.memblock);
+        p = pa_memblock_acquire(u->source_buffer.memblock);
+        iov.iov_base = p + u->source_buffer.index;
+        iov.iov_len = u->read_block_size;
         l = recvmsg(u->stream_fd, &m, 0);
-        pa_memblock_release(memchunk.memblock);
+        pa_memblock_release(u->source_buffer.memblock);
 
         if (l > 0)
             break;
@@ -356,8 +367,6 @@  static int sco_process_push(struct userdata *u) {
             /* Retry right away if we got interrupted */
             continue;
 
-        pa_memblock_unref(memchunk.memblock);
-
         if (l < 0 && errno == EAGAIN)
             /* Hmm, apparently the socket was not readable, give up for now. */
             return 0;
@@ -366,7 +375,7 @@  static int sco_process_push(struct userdata *u) {
         return -1;
     }
 
-    pa_assert((size_t) l <= pa_memblock_get_length(memchunk.memblock));
+    pa_assert((size_t) l <= u->read_block_size);
 
     /* In some rare occasions, we might receive packets of a very strange
      * size. This could potentially be possible if the SCO packet was
@@ -376,11 +385,10 @@  static int sco_process_push(struct userdata *u) {
      * packet */
     if (!pa_frame_aligned(l, &u->sample_spec)) {
         pa_log_warn("SCO packet received of unaligned size: %zu", l);
-        pa_memblock_unref(memchunk.memblock);
         return -1;
     }
 
-    memchunk.length = (size_t) l;
+    u->source_buffer.index += (size_t) l;
     u->read_index += (uint64_t) l;
 
     for (cm = CMSG_FIRSTHDR(&m); cm; cm = CMSG_NXTHDR(&m, cm))
@@ -397,11 +405,19 @@  static int sco_process_push(struct userdata *u) {
         tstamp = pa_rtclock_now();
     }
 
-    pa_smoother_put(u->read_smoother, tstamp, pa_bytes_to_usec(u->read_index, &u->sample_spec));
-    pa_smoother_resume(u->read_smoother, tstamp, true);
+    u->source_buffered_blocks++;
 
-    pa_source_post(u->source, &memchunk);
-    pa_memblock_unref(memchunk.memblock);
+    if (u->source_buffered_blocks >= max_blocks) {
+        pa_smoother_put(u->read_smoother, tstamp, pa_bytes_to_usec(u->read_index, &u->sample_spec));
+        pa_smoother_resume(u->read_smoother, tstamp, true);
+
+        u->source_buffer.length = u->source_buffer.index;
+        u->source_buffer.index = 0;
+        pa_source_post(u->source, &u->source_buffer);
+        pa_memblock_unref(u->source_buffer.memblock);
+        pa_memchunk_reset(&u->source_buffer);
+        u->source_buffered_blocks = 0;
+    }
 
     return l;
 }
@@ -784,6 +800,12 @@  static void teardown_stream(struct userdata *u) {
         pa_memchunk_reset(&u->write_memchunk);
     }
 
+    if (u->source_buffer.memblock) {
+        pa_memblock_unref(u->source_buffer.memblock);
+        pa_memchunk_reset(&u->source_buffer);
+    }
+    u->source_buffered_blocks = 0;
+
     pa_log_debug("Audio stream torn down");
     u->stream_setup_done = false;
 }
@@ -947,6 +969,7 @@  static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
                 ri = pa_bytes_to_usec(u->read_index, &u->sample_spec);
 
                 *((int64_t*) data) = u->source->thread_info.fixed_latency + wi - ri;
+                *((int64_t*) data) += u->source_buffered_blocks * pa_bytes_to_usec(u->read_block_size, &u->source->sample_spec);
             } else
                 *((int64_t*) data) = 0;
 
@@ -2362,6 +2385,7 @@  int pa__init(pa_module* m) {
     const char *path;
     pa_modargs *ma;
     bool autodetect_mtu;
+    uint32_t source_buffer_msec;
 
     pa_assert(m);
 
@@ -2397,7 +2421,15 @@  int pa__init(pa_module* m) {
         goto fail_free_modargs;
     }
 
+    source_buffer_msec = 0;
+    if (pa_modargs_get_value_u32(ma, "hsp_source_buffer_msec", &source_buffer_msec) < 0 || source_buffer_msec > 100) {
+        pa_log("Invalid value for hsp_source_buffer_msec parameter, value must be <= 100");
+        goto fail;
+    }
+
     u->device->autodetect_mtu = autodetect_mtu;
+    u->source_buffer_usec = source_buffer_msec * PA_USEC_PER_MSEC;
+    u->source_buffered_blocks = 0;
 
     pa_modargs_free(ma);
 
diff --git a/src/modules/bluetooth/module-bluez5-discover.c b/src/modules/bluetooth/module-bluez5-discover.c
index 44578214..8108627d 100644
--- a/src/modules/bluetooth/module-bluez5-discover.c
+++ b/src/modules/bluetooth/module-bluez5-discover.c
@@ -36,11 +36,14 @@  PA_MODULE_VERSION(PACKAGE_VERSION);
 PA_MODULE_LOAD_ONCE(true);
 PA_MODULE_USAGE(
     "headset=ofono|native|auto"
+    "autodetect_mtu=<boolean>"
+    "hsp_source_buffer_msec=<0 - 100>"
 );
 
 static const char* const valid_modargs[] = {
     "headset",
     "autodetect_mtu",
+    "hsp_source_buffer_msec",
     NULL
 };
 
@@ -51,6 +54,7 @@  struct userdata {
     pa_hook_slot *device_connection_changed_slot;
     pa_bluetooth_discovery *discovery;
     bool autodetect_mtu;
+    uint32_t source_buffer_msec;
 };
 
 static pa_hook_result_t device_connection_changed_cb(pa_bluetooth_discovery *y, const pa_bluetooth_device *d, struct userdata *u) {
@@ -71,7 +75,7 @@  static pa_hook_result_t device_connection_changed_cb(pa_bluetooth_discovery *y,
     if (!module_loaded && pa_bluetooth_device_any_transport_connected(d)) {
         /* a new device has been connected */
         pa_module *m;
-        char *args = pa_sprintf_malloc("path=%s autodetect_mtu=%i", d->path, (int)u->autodetect_mtu);
+        char *args = pa_sprintf_malloc("path=%s autodetect_mtu=%i hsp_source_buffer_msec=%u", d->path, (int)u->autodetect_mtu, u->source_buffer_msec);
 
         pa_log_debug("Loading module-bluez5-device %s", args);
         pa_module_load(&m, u->module->core, "module-bluez5-device", args);
@@ -102,6 +106,7 @@  int pa__init(pa_module *m) {
     const char *headset_str;
     int headset_backend;
     bool autodetect_mtu;
+    uint32_t source_buffer_msec;
 
     pa_assert(m);
 
@@ -128,10 +133,17 @@  int pa__init(pa_module *m) {
         goto fail;
     }
 
+    source_buffer_msec = 0;
+    if (pa_modargs_get_value_u32(ma, "hsp_source_buffer_msec", &source_buffer_msec) < 0 || source_buffer_msec > 100) {
+        pa_log("Invalid value for hsp_source_buffer_msec parameter, value must be <= 100");
+        goto fail;
+    }
+
     m->userdata = u = pa_xnew0(struct userdata, 1);
     u->module = m;
     u->core = m->core;
     u->autodetect_mtu = autodetect_mtu;
+    u->source_buffer_msec = source_buffer_msec;
     u->loaded_device_paths = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
 
     if (!(u->discovery = pa_bluetooth_discovery_get(u->core, headset_backend)))

Comments

Luiz Augusto von Dentz April 10, 2018, 12:52 p.m.
Hi George,

On Mon, Apr 9, 2018 at 7:16 PM, Georg Chini <georg@chini.tk> wrote:
> Currently the PA bluetooth code calls source_push() for each HSP source packet.
> The default HSP MTU is 48 bytes, this means that source_push() is called every
> 3ms, which leads to increased CPU load especially on embedded systems.
>
> This patch adds a hsp_source_buffer_msec argument to module-bluez5-discover and
> module-bluez5-device. The argument gives the number of milliseconds of audio which
> are buffered, before source_push() is called. The value can range from 0 to 100ms,
> and is rounded down to the next multiple of the MTU size. Therefore a value of less
> than 2*MTU time corresponds to the original behavior.

Well SCO is, as the name suggests, synchronous or to be more precise
it isochronous so I wonder if this has been tested? It seems to me
this will start to behave like A2DP which buffer frames for a while
before sending, though A2DP is not really for voice where latency may
cause problems like lip sync issues or talking over someone else on a
call because the audio has a few second delay. I get that using 3ms
when the  default-fragment-size-msec is bigger than that may not make
any difference so we should probably use that instead.

> ---
>  src/modules/bluetooth/module-bluetooth-discover.c |  1 +
>  src/modules/bluetooth/module-bluez5-device.c      | 68 +++++++++++++++++------
>  src/modules/bluetooth/module-bluez5-discover.c    | 14 ++++-
>  3 files changed, 64 insertions(+), 19 deletions(-)

I do have patches changing the bluez5 modules to bluetooth, I wonder
what happen to that.


> diff --git a/src/modules/bluetooth/module-bluetooth-discover.c b/src/modules/bluetooth/module-bluetooth-discover.c
> index 63195d3e..14c0a38f 100644
> --- a/src/modules/bluetooth/module-bluetooth-discover.c
> +++ b/src/modules/bluetooth/module-bluetooth-discover.c
> @@ -32,6 +32,7 @@ PA_MODULE_LOAD_ONCE(true);
>  PA_MODULE_USAGE(
>      "headset=ofono|native|auto (bluez 5 only)"
>      "autodetect_mtu=<boolean> (bluez 5 only)"
> +    "hsp_source_buffer_msec=<0 - 100> (bluez5 only)"
>  );
>
>  struct userdata {
> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
> index b81c233c..d40bbb0c 100644
> --- a/src/modules/bluetooth/module-bluez5-device.c
> +++ b/src/modules/bluetooth/module-bluez5-device.c
> @@ -54,7 +54,8 @@ PA_MODULE_DESCRIPTION("BlueZ 5 Bluetooth audio sink and source");
>  PA_MODULE_VERSION(PACKAGE_VERSION);
>  PA_MODULE_LOAD_ONCE(false);
>  PA_MODULE_USAGE("path=<device object path>"
> -                "autodetect_mtu=<boolean>");
> +                "autodetect_mtu=<boolean>"
> +                "hsp_source_buffer_msec=<0 - 100>");
>
>  #define FIXED_LATENCY_PLAYBACK_A2DP (25 * PA_USEC_PER_MSEC)
>  #define FIXED_LATENCY_PLAYBACK_SCO  (25 * PA_USEC_PER_MSEC)
> @@ -68,6 +69,7 @@ PA_MODULE_USAGE("path=<device object path>"
>  static const char* const valid_modargs[] = {
>      "path",
>      "autodetect_mtu",
> +    "hsp_source_buffer_msec",
>      NULL
>  };
>
> @@ -123,6 +125,9 @@ struct userdata {
>      pa_card *card;
>      pa_sink *sink;
>      pa_source *source;
> +    uint32_t source_buffer_usec;
> +    uint32_t source_buffered_blocks;
> +    pa_memchunk source_buffer;
>      pa_bluetooth_profile_t profile;
>      char *output_port_name;
>      char *input_port_name;
> @@ -314,10 +319,10 @@ static int sco_process_render(struct userdata *u) {
>  /* Run from IO thread */
>  static int sco_process_push(struct userdata *u) {
>      ssize_t l;
> -    pa_memchunk memchunk;
>      struct cmsghdr *cm;
>      struct msghdr m;
>      bool found_tstamp = false;
> +    uint32_t max_blocks;
>      pa_usec_t tstamp = 0;
>
>      pa_assert(u);
> @@ -326,11 +331,17 @@ static int sco_process_push(struct userdata *u) {
>      pa_assert(u->source);
>      pa_assert(u->read_smoother);
>
> -    memchunk.memblock = pa_memblock_new(u->core->mempool, u->read_block_size);
> -    memchunk.index = memchunk.length = 0;
> +    max_blocks = u->source_buffer_usec / pa_bytes_to_usec(u->read_block_size, &u->source->sample_spec);
> +    if (max_blocks == 0)
> +        max_blocks = 1;
> +
> +    if (!u->source_buffer.memblock) {
> +        u->source_buffer.memblock = pa_memblock_new(u->core->mempool, max_blocks * u->read_block_size);
> +        u->source_buffer.index = u->source_buffer.length = 0;
> +    }
>
>      for (;;) {
> -        void *p;
> +        uint8_t *p;
>          uint8_t aux[1024];
>          struct iovec iov;
>
> @@ -343,11 +354,11 @@ static int sco_process_push(struct userdata *u) {
>          m.msg_control = aux;
>          m.msg_controllen = sizeof(aux);
>
> -        p = pa_memblock_acquire(memchunk.memblock);
> -        iov.iov_base = p;
> -        iov.iov_len = pa_memblock_get_length(memchunk.memblock);
> +        p = pa_memblock_acquire(u->source_buffer.memblock);
> +        iov.iov_base = p + u->source_buffer.index;
> +        iov.iov_len = u->read_block_size;
>          l = recvmsg(u->stream_fd, &m, 0);
> -        pa_memblock_release(memchunk.memblock);
> +        pa_memblock_release(u->source_buffer.memblock);
>
>          if (l > 0)
>              break;
> @@ -356,8 +367,6 @@ static int sco_process_push(struct userdata *u) {
>              /* Retry right away if we got interrupted */
>              continue;
>
> -        pa_memblock_unref(memchunk.memblock);
> -
>          if (l < 0 && errno == EAGAIN)
>              /* Hmm, apparently the socket was not readable, give up for now. */
>              return 0;
> @@ -366,7 +375,7 @@ static int sco_process_push(struct userdata *u) {
>          return -1;
>      }
>
> -    pa_assert((size_t) l <= pa_memblock_get_length(memchunk.memblock));
> +    pa_assert((size_t) l <= u->read_block_size);
>
>      /* In some rare occasions, we might receive packets of a very strange
>       * size. This could potentially be possible if the SCO packet was
> @@ -376,11 +385,10 @@ static int sco_process_push(struct userdata *u) {
>       * packet */
>      if (!pa_frame_aligned(l, &u->sample_spec)) {
>          pa_log_warn("SCO packet received of unaligned size: %zu", l);
> -        pa_memblock_unref(memchunk.memblock);
>          return -1;
>      }
>
> -    memchunk.length = (size_t) l;
> +    u->source_buffer.index += (size_t) l;
>      u->read_index += (uint64_t) l;
>
>      for (cm = CMSG_FIRSTHDR(&m); cm; cm = CMSG_NXTHDR(&m, cm))
> @@ -397,11 +405,19 @@ static int sco_process_push(struct userdata *u) {
>          tstamp = pa_rtclock_now();
>      }
>
> -    pa_smoother_put(u->read_smoother, tstamp, pa_bytes_to_usec(u->read_index, &u->sample_spec));
> -    pa_smoother_resume(u->read_smoother, tstamp, true);
> +    u->source_buffered_blocks++;
>
> -    pa_source_post(u->source, &memchunk);
> -    pa_memblock_unref(memchunk.memblock);
> +    if (u->source_buffered_blocks >= max_blocks) {
> +        pa_smoother_put(u->read_smoother, tstamp, pa_bytes_to_usec(u->read_index, &u->sample_spec));
> +        pa_smoother_resume(u->read_smoother, tstamp, true);
> +
> +        u->source_buffer.length = u->source_buffer.index;
> +        u->source_buffer.index = 0;
> +        pa_source_post(u->source, &u->source_buffer);
> +        pa_memblock_unref(u->source_buffer.memblock);
> +        pa_memchunk_reset(&u->source_buffer);
> +        u->source_buffered_blocks = 0;
> +    }
>
>      return l;
>  }
> @@ -784,6 +800,12 @@ static void teardown_stream(struct userdata *u) {
>          pa_memchunk_reset(&u->write_memchunk);
>      }
>
> +    if (u->source_buffer.memblock) {
> +        pa_memblock_unref(u->source_buffer.memblock);
> +        pa_memchunk_reset(&u->source_buffer);
> +    }
> +    u->source_buffered_blocks = 0;
> +
>      pa_log_debug("Audio stream torn down");
>      u->stream_setup_done = false;
>  }
> @@ -947,6 +969,7 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
>                  ri = pa_bytes_to_usec(u->read_index, &u->sample_spec);
>
>                  *((int64_t*) data) = u->source->thread_info.fixed_latency + wi - ri;
> +                *((int64_t*) data) += u->source_buffered_blocks * pa_bytes_to_usec(u->read_block_size, &u->source->sample_spec);
>              } else
>                  *((int64_t*) data) = 0;
>
> @@ -2362,6 +2385,7 @@ int pa__init(pa_module* m) {
>      const char *path;
>      pa_modargs *ma;
>      bool autodetect_mtu;
> +    uint32_t source_buffer_msec;
>
>      pa_assert(m);
>
> @@ -2397,7 +2421,15 @@ int pa__init(pa_module* m) {
>          goto fail_free_modargs;
>      }
>
> +    source_buffer_msec = 0;
> +    if (pa_modargs_get_value_u32(ma, "hsp_source_buffer_msec", &source_buffer_msec) < 0 || source_buffer_msec > 100) {
> +        pa_log("Invalid value for hsp_source_buffer_msec parameter, value must be <= 100");
> +        goto fail;
> +    }
> +
>      u->device->autodetect_mtu = autodetect_mtu;
> +    u->source_buffer_usec = source_buffer_msec * PA_USEC_PER_MSEC;
> +    u->source_buffered_blocks = 0;
>
>      pa_modargs_free(ma);
>
> diff --git a/src/modules/bluetooth/module-bluez5-discover.c b/src/modules/bluetooth/module-bluez5-discover.c
> index 44578214..8108627d 100644
> --- a/src/modules/bluetooth/module-bluez5-discover.c
> +++ b/src/modules/bluetooth/module-bluez5-discover.c
> @@ -36,11 +36,14 @@ PA_MODULE_VERSION(PACKAGE_VERSION);
>  PA_MODULE_LOAD_ONCE(true);
>  PA_MODULE_USAGE(
>      "headset=ofono|native|auto"
> +    "autodetect_mtu=<boolean>"
> +    "hsp_source_buffer_msec=<0 - 100>"
>  );
>
>  static const char* const valid_modargs[] = {
>      "headset",
>      "autodetect_mtu",
> +    "hsp_source_buffer_msec",
>      NULL
>  };
>
> @@ -51,6 +54,7 @@ struct userdata {
>      pa_hook_slot *device_connection_changed_slot;
>      pa_bluetooth_discovery *discovery;
>      bool autodetect_mtu;
> +    uint32_t source_buffer_msec;
>  };
>
>  static pa_hook_result_t device_connection_changed_cb(pa_bluetooth_discovery *y, const pa_bluetooth_device *d, struct userdata *u) {
> @@ -71,7 +75,7 @@ static pa_hook_result_t device_connection_changed_cb(pa_bluetooth_discovery *y,
>      if (!module_loaded && pa_bluetooth_device_any_transport_connected(d)) {
>          /* a new device has been connected */
>          pa_module *m;
> -        char *args = pa_sprintf_malloc("path=%s autodetect_mtu=%i", d->path, (int)u->autodetect_mtu);
> +        char *args = pa_sprintf_malloc("path=%s autodetect_mtu=%i hsp_source_buffer_msec=%u", d->path, (int)u->autodetect_mtu, u->source_buffer_msec);
>
>          pa_log_debug("Loading module-bluez5-device %s", args);
>          pa_module_load(&m, u->module->core, "module-bluez5-device", args);
> @@ -102,6 +106,7 @@ int pa__init(pa_module *m) {
>      const char *headset_str;
>      int headset_backend;
>      bool autodetect_mtu;
> +    uint32_t source_buffer_msec;
>
>      pa_assert(m);
>
> @@ -128,10 +133,17 @@ int pa__init(pa_module *m) {
>          goto fail;
>      }
>
> +    source_buffer_msec = 0;
> +    if (pa_modargs_get_value_u32(ma, "hsp_source_buffer_msec", &source_buffer_msec) < 0 || source_buffer_msec > 100) {
> +        pa_log("Invalid value for hsp_source_buffer_msec parameter, value must be <= 100");
> +        goto fail;
> +    }
> +
>      m->userdata = u = pa_xnew0(struct userdata, 1);
>      u->module = m;
>      u->core = m->core;
>      u->autodetect_mtu = autodetect_mtu;
> +    u->source_buffer_msec = source_buffer_msec;
>      u->loaded_device_paths = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
>
>      if (!(u->discovery = pa_bluetooth_discovery_get(u->core, headset_backend)))
> --
> 2.14.1
>
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Georg Chini April 10, 2018, 1:01 p.m.
On 10.04.2018 14:52, Luiz Augusto von Dentz wrote:
> Hi George,
>
> On Mon, Apr 9, 2018 at 7:16 PM, Georg Chini <georg@chini.tk> wrote:
>> Currently the PA bluetooth code calls source_push() for each HSP source packet.
>> The default HSP MTU is 48 bytes, this means that source_push() is called every
>> 3ms, which leads to increased CPU load especially on embedded systems.
>>
>> This patch adds a hsp_source_buffer_msec argument to module-bluez5-discover and
>> module-bluez5-device. The argument gives the number of milliseconds of audio which
>> are buffered, before source_push() is called. The value can range from 0 to 100ms,
>> and is rounded down to the next multiple of the MTU size. Therefore a value of less
>> than 2*MTU time corresponds to the original behavior.
> Well SCO is, as the name suggests, synchronous or to be more precise
> it isochronous so I wonder if this has been tested? It seems to me
> this will start to behave like A2DP which buffer frames for a while
> before sending, though A2DP is not really for voice where latency may
> cause problems like lip sync issues or talking over someone else on a
> call because the audio has a few second delay. I get that using 3ms
> when the  default-fragment-size-msec is bigger than that may not make
> any difference so we should probably use that instead.

The communication between PA and the head set is not changed by the
patch. It is only between PA and the client. Instead of pushing the data to
the client immediately, with this option PA buffers some blocks before
sending them on.

>
>> ---
>>   src/modules/bluetooth/module-bluetooth-discover.c |  1 +
>>   src/modules/bluetooth/module-bluez5-device.c      | 68 +++++++++++++++++------
>>   src/modules/bluetooth/module-bluez5-discover.c    | 14 ++++-
>>   3 files changed, 64 insertions(+), 19 deletions(-)
> I do have patches changing the bluez5 modules to bluetooth, I wonder
> what happen to that.
>
>
If your patch gets into the tree first, I'll re-base my patch (again).