On 08/12/2013 12:27 PM, Tanu Kaskinen wrote:
> On Mon, 2013-08-12 at 09:20 +0200, David Henningsson wrote:
>> On 08/09/2013 08:57 AM, Tanu Kaskinen wrote:
>>> ---
>>> src/pulsecore/sink.c | 4 ++--
>>> src/pulsecore/source.c | 4 ++--
>>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
>>> index a4ad21a..ae6ae8f 100644
>>> --- a/src/pulsecore/sink.c
>>> +++ b/src/pulsecore/sink.c
>>> @@ -1412,8 +1412,8 @@ bool pa_sink_update_rate(pa_sink *s, uint32_t rate, bool passthrough) {
>>> return false;
>>>
>>> if (!passthrough) {
>>> - pa_assert(default_rate % 4000 || default_rate % 11025);
>>> - pa_assert(alternate_rate % 4000 || alternate_rate % 11025);
>>> + pa_assert((default_rate % 4000 == 0) || (default_rate % 11025 == 0));
>>> + pa_assert((alternate_rate % 4000 == 0) || (alternate_rate % 11025 == 0));
>>
>> Looks correct, even though I don't understand why we need these
>> constraints in the first place. (Why can't we just allow any sample rate
>> as default or alternate?)
>
> I don't know. I would guess that this is a simple way to optimize
> multiple common cases (anything that is divisible by 4000 or 11025),
> while the general solution would be to optimize only two cases (the
> exact default and alternate rates).
>
I remember a weird USB interface (in some launchpad bug) that only
accepted a non-standard rate, I think it was around 46 kHz or something.
In case there is only one rate, I'm not sure that the rate can ever
update, but if it does (or in case a device would support two
non-standard rates), you have effectively enabled an assertion crash
that should perhaps not have been there in the first place. So I'm a bit
worried about the above.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
