On 09.03.2017 05:37, Arun Raghavan wrote:
> If the ALSA device supports granular pointer reporting, we end up in a
> situation where we write out a bunch of data, iterate, and then find a
> small amount of data available in the buffer (consumed while we were
> writing data into the available buffer space). We do this 10 times
> before quitting the write loop.
>
> This is inefficient in itself, but can also have wider consequences. For
> example, with module-combine-sink, this will end up pushing the same
> small chunks to all other devices too.
>
> Given both of these, it just makes sense to not try to write out data
> unless a minimum threshold is available. This could potentially be a
> fragment, but it's likely most robust to just work with a fraction of
> the total available buffer size.
> ---
> src/modules/alsa/alsa-sink.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
> index 886c735..43cd965 100644
> --- a/src/modules/alsa/alsa-sink.c
> +++ b/src/modules/alsa/alsa-sink.c
> @@ -88,6 +88,8 @@
> #define DEFAULT_REWIND_SAFEGUARD_BYTES (256U) /* 1.33ms @48kHz, we'll never rewind less than this */
> #define DEFAULT_REWIND_SAFEGUARD_USEC (1330) /* 1.33ms, depending on channels/rate/sample we may rewind more than 256 above */
>
> +#define DEFAULT_WRITE_ITERATION_THRESHOLD 0.03 /* don't iterate write if < 3% of the buffer is available */
> +
> struct userdata {
> pa_core *core;
> pa_module *module;
> @@ -580,12 +582,19 @@ static int mmap_write(struct userdata *u, pa_usec_t *sleep_usec, bool polled, bo
> break;
> }
>
> - if (++j > 10) {
> + j++;
> +
> + if (j > 10) {
> #ifdef DEBUG_TIMING
> pa_log_debug("Not filling up, because already too many iterations.");
> #endif
>
> break;
> + } else if (j >= 1 && (n_bytes < (DEFAULT_WRITE_ITERATION_THRESHOLD * (u->hwbuf_size - u->hwbuf_unused)))) {
Why do you check for j >=1 here? This should always be true. The same
applies for unix_write(). Otherwise looks good.
> +#ifdef DEBUG_TIMING
> + pa_log_debug("Not filling up, because <%g%% available.", DEFAULT_WRITE_ITERATION_THRESHOLD * 100);
> +#endif
> + break;
> }
>
> n_bytes -= u->hwbuf_unused;
> @@ -754,12 +763,19 @@ static int unix_write(struct userdata *u, pa_usec_t *sleep_usec, bool polled, bo
> break;
> }
>
> - if (++j > 10) {
> + j++;
> +
> + if (j > 10) {
> #ifdef DEBUG_TIMING
> pa_log_debug("Not filling up, because already too many iterations.");
> #endif
>
> break;
> + } else if (j >= 1 && (n_bytes < (DEFAULT_WRITE_ITERATION_THRESHOLD * (u->hwbuf_size - u->hwbuf_unused)))) {
> +#ifdef DEBUG_TIMING
> + pa_log_debug("Not filling up, because <%g%% available.", DEFAULT_WRITE_ITERATION_THRESHOLD * 100);
> +#endif
> + break;
> }
>
> n_bytes -= u->hwbuf_unused;
