[pulseaudio-discuss] Couple problems when using 0.9.11 and fixes

[Marc-André Lureau]

Jyri,

I suggest you to attach the patch next time.

You can do that with git-format-patch / git-send-email (or manually).

Regards,

On Thu, Sep 4, 2008 at 2:56 PM, Jyri Sarha <lepbtetfmvbz at spammotel.com> wrote:
> Hi,
>        I found couple of problems when upgrading to pulseaduio 0.9.11. The
> both are related to a case when tsched turned off on alsa-sink. First,
> problem appears when trying the select a specific number fragment with
> module-alsa-sink parameters, for instance this:
>
> load-module module-alsa-sink device=hw:0 rate=44100 fragment_size=882 fragments=2 tsched=0
>
> The above produces three fragments of 588 bytes each. The actual bug
> appears to be in alsa-lib, but it can be by-passed without breaking
> things when alsa-lib gets fixed. To do that apply
> fix_incremented_number_of_fragments.patch.
>
> The other problem appears when trying to use alsa-sink with tsched=0 on a
> slow CPU. After writing a set of samples to device alsa-sink checks whether
> there is room to write some more samples. On a slow CPU there is always room
> for couple of samples more and alsa-sink keeps busy looping in the write
> function. "parameter_to_set_minimum_write_size.patch" adds a parameter to
> module-alsa-sink to set the minimum number of samples to write on device,
> setting this parameter to a reasonable value fixes the above problem,
> like this:
>
> load-module module-alsa-sink device=hw:0 rate=44100 fragment_size=882 fragments=2 tsched=0 hwbuf_min_frames_to_write=32
>
> Alsa-source would need a similar patch too. In the end I decided
> to forward port the old alsa- sink and source from 0.9.10 to 0.9.11,
> because they seem to work more reliably and generate much less load, when
> fragment-size and number of fragments is pushed to the minimum and
> tsched is not used. I can post those patches too if anybody is interested.
>
> Cheers,
>        Jyri
>
>
> ===File ~/fix_incremented_number_of_fragments.patch=========
> diff --git a/src/modules/alsa-util.c b/src/modules/alsa-util.c
> index 8abf834..64b4eff 100644
> --- a/src/modules/alsa-util.c
> +++ b/src/modules/alsa-util.c
> @@ -369,12 +369,15 @@ int pa_alsa_set_hw_params(
>         goto finish;
>
>     if (_periods > 0) {
> -        dir = 1;
> -        if ((ret = snd_pcm_hw_params_set_periods_near(pcm_handle, hwparams, &_periods, &dir)) < 0) {
> -            dir = -1;
> -            if ((ret = snd_pcm_hw_params_set_periods_near(pcm_handle, hwparams, &_periods, &dir)) < 0)
> -                goto finish;
> -        }
> +       dir = 0;
> +       if ((ret = snd_pcm_hw_params_set_periods_near(pcm_handle, hwparams, &_periods, &dir)) < 0) {
> +           dir = 1;
> +           if ((ret = snd_pcm_hw_params_set_periods_near(pcm_handle, hwparams, &_periods, &dir)) < 0) {
> +               dir = -1;
> +               if ((ret = snd_pcm_hw_params_set_periods_near(pcm_handle, hwparams, &_periods, &dir)) < 0)
> +                   goto finish;
> +           }
> +       }
>     }
>
>     if (_period_size > 0)
> ============================================================
>
> ===File ~/parameter_to_set_minimum_write_size.patch=========
> diff --git a/src/modules/module-alsa-sink.c b/src/modules/module-alsa-sink.c
> index 8e66f79..0135a0b 100644
> --- a/src/modules/module-alsa-sink.c
> +++ b/src/modules/module-alsa-sink.c
> @@ -69,7 +69,8 @@ PA_MODULE_USAGE(
>         "tsched=<enable system timer based scheduling mode?> "
>         "tsched_buffer_size=<buffer size when using timer based scheduling> "
>         "tsched_buffer_watermark=<lower fill watermark> "
> -        "mixer_reset=<reset hw volume and mute settings to sane defaults when falling back to software?>");
> +        "mixer_reset=<reset hw volume and mute settings to sane defaults when falling back to software?>"
> +       "hwbuf_min_frames_to_write=<minimum number of available frames in hwbuf to fill>");
>
>  static const char* const valid_modargs[] = {
>     "sink_name",
> @@ -86,6 +87,7 @@ static const char* const valid_modargs[] = {
>     "tsched_buffer_size",
>     "tsched_buffer_watermark",
>     "mixer_reset",
> +    "hwbuf_min_frames_to_write",
>     NULL
>  };
>
> @@ -132,6 +134,7 @@ struct userdata {
>     uint64_t since_start;
>
>     snd_pcm_sframes_t hwbuf_unused_frames;
> +    snd_pcm_sframes_t hwbuf_min_frames_to_write;
>  };
>
>  static void fix_tsched_watermark(struct userdata *u) {
> @@ -278,7 +281,7 @@ static int mmap_write(struct userdata *u, pa_usec_t *sleep_usec) {
>             if (pa_bytes_to_usec(left_to_play, &u->sink->sample_spec) > process_usec+max_sleep_usec/2)
>                 break;
>
> -        if (PA_UNLIKELY(n <= u->hwbuf_unused_frames))
> +        if (PA_UNLIKELY(n <= u->hwbuf_min_frames_to_write || n <= u->hwbuf_unused_frames))
>             break;
>
>         n -= u->hwbuf_unused_frames;
> @@ -390,7 +393,7 @@ static int unix_write(struct userdata *u, pa_usec_t *sleep_usec) {
>             if (pa_bytes_to_usec(left_to_play, &u->sink->sample_spec) > process_usec+max_sleep_usec/2)
>                 break;
>
> -        if (PA_UNLIKELY(n <= u->hwbuf_unused_frames))
> +        if (PA_UNLIKELY(n <= u->hwbuf_min_frames_to_write || n <= u->hwbuf_unused_frames))
>             break;
>
>         n -= u->hwbuf_unused_frames;
> @@ -1103,6 +1106,7 @@ int pa__init(pa_module*m) {
>     pa_bool_t use_mmap = TRUE, b, use_tsched = TRUE, d, mixer_reset = TRUE;
>     pa_usec_t usec;
>     pa_sink_new_data data;
> +    int32_t hwbuf_min_frames_to_write = 0;
>
>     snd_pcm_info_alloca(&pcm_info);
>
> @@ -1162,6 +1166,11 @@ int pa__init(pa_module*m) {
>         goto fail;
>     }
>
> +    if (pa_modargs_get_value_s32(ma, "hwbuf_min_frames_to_write", &hwbuf_min_frames_to_write) < 0) {
> +        pa_log("Failed to parse hwbuf_min_frames_to_write argument");
> +       goto fail;
> +    }
> +
>     u = pa_xnew0(struct userdata, 1);
>     u->core = m->core;
>     u->module = m;
> @@ -1316,6 +1325,7 @@ int pa__init(pa_module*m) {
>     u->fragment_size = frag_size = period_frames * frame_size;
>     u->nfragments = nfrags;
>     u->hwbuf_size = u->fragment_size * nfrags;
> +    u->hwbuf_min_frames_to_write = hwbuf_min_frames_to_write;
>     u->hwbuf_unused_frames = 0;
>     u->tsched_watermark = tsched_watermark;
>     u->frame_index = 0;
> ============================================================
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at mail.0pointer.de
> https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
>



-- 
Marc-André Lureau