[pulseaudio-discuss] [PATCH 3/3] alsa-sink/source, sink, source: Consider sample format for avoid-resampling

[Sangchul Lee]

Thanks for the comments.
Before sharing new patchset, I have two questions about your comment. :)

>> @@ -113,11 +113,20 @@ struct userdata {
>>
>>      pa_sample_format_t *supported_formats;
>>      unsigned int *supported_rates;
>> +    struct {
>> +        pa_sample_spec sample_spec;
>> +        size_t fragment_size;
>> +        size_t nfrags;
>> +        size_t tsched_size;
>> +        size_t tsched_watermark;
>> +        size_t rewind_safeguard;
>> +    } initial_info;
>
> I'm not sure this is needed. What breaks if you don't save the initial
> info?
>> +    if (u->initial_info.sample_spec.rate == ss->rate && u->initial_info.sample_spec.format == ss->format) {
>> +        /* use initial values including module arguments */
>> +        u->fragment_size = u->initial_info.fragment_size;
>> +        u->hwbuf_size = u->initial_info.nfrags * u->fragment_size;
>> +        u->tsched_size = u->initial_info.tsched_size;
>> +        u->tsched_watermark = u->initial_info.tsched_watermark;
>> +        u->rewind_safeguard = u->initial_info.rewind_safeguard;
>> +    } else {
>> +        u->fragment_size = pa_usec_to_bytes(u->core->default_fragment_size_msec * PA_USEC_PER_MSEC, ss);
>> +        u->hwbuf_size = u->core->default_n_fragments * u->fragment_size;
>> +        u->tsched_size = pa_usec_to_bytes(DEFAULT_TSCHED_BUFFER_USEC, ss);
>> +        u->tsched_watermark = pa_usec_to_bytes(DEFAULT_TSCHED_WATERMARK_USEC, ss);
>> +        u->rewind_safeguard = PA_MAX(DEFAULT_REWIND_SAFEGUARD_BYTES, pa_usec_to_bytes(DEFAULT_REWIND_SAFEGUARD_USEC, ss));
>> +    }
>
> The module arguments should be taken into account where applicable also
> when changing the rate or format

I though these parameters are related to user preference(used for
tuning) with a specific format and rate from module argument.
So I intended to keep these values only for the initial format and
rate from module argument. Therefore default variables are used
when the format or rate is changed to another.
As per your comment, does it need to be changed to keep using original
variables(frag size, tsched size, and so on..) of module argument
in case of the change of format or rate?

>>
>> -    } else if (avoid_resampling && (spec->rate >= default_rate || spec->rate >= alternate_rate)) {
>> +    } else if (avoid_resampling && (spec->format != s->sample_spec.format ||
>> +                spec->rate >= default_rate || spec->rate >= alternate_rate)) {
>>          /* We just try to set the sink input's sample rate if it's not too low */
>> +        desired_spec.format = spec->format;
>>          desired_spec.rate = spec->rate;
>
> desired_spec.rate shouldn't be set if spec->rate is less than
> default_rate and alternate_rate. Now the rate will be set always if the
> format needs changing.

If having the condition with default rate and alternate_rate, it can
not be set with new sample format of a stream
even if it is different with previous sample format of sink/source. I
though it is no little limitation. For example, if the default
sample rate is 48k and alternative sample rate is 48k or 96k, with a
rate of 44.1k stream will be resampled anyway even though
avoid-resampling feature is enabled and this condition also affects
the case of different sample format similarly.
Is there any reason for this..?

Regards,
Sangchul