[pulseaudio-discuss] [PATCH v3 05/24] echo-cancel: Canceller may use different spec for playback and capture

[Arun Raghavan]

On 2 February 2016 at 18:06, Tanu Kaskinen <tanuk at iki.fi> wrote:
> On Tue, 2016-02-02 at 09:53 +0530, Arun Raghavan wrote:
>> On Tue, 2016-01-26 at 13:42 +0200, Tanu Kaskinen wrote:
>> > On Mon, 2016-01-18 at 13:06 +0530, arun at accosted.net wrote:
>> > > From: Arun Raghavan <git at arunraghavan.net>
>> > >
>> > > This sets up the default sink sample spec to match what we expect,
>> > > rather than assuming that the canceller will set this up (our
>> > > assumption
>> > > is that we'll use 32 kHz mono unless someone explicitly overrides
>> > > this).
>> >
>> > I have trouble understanding this description. Maybe rewording is
>> > needed?
>> >
>> > What does "what we expect" mean? You set the rate and channels to 32
>> > kHz mono, why is that our "expectation"? Does something break if this
>> > expectation is violated via user configuration? The sink and source
>> > rates seem to be configurable (matching rates between source and sink
>> > is enforced, though). The sink channel setup seems to be forced to be
>> > mono, while the source channel map can be configured. What are the
>> > reasons behind this particular mix of configurability and hardcoding?
>> > Some comments about this in the sample spec initialization code might
>> > be a good idea.
>> >
>> > I'm not sure why you describe the choice of using 32 kHz mono as an
>> > "assumption". Defaulting to 32 kHz mono is what the code does, in
>> > what
>> > way is that an assumption?
>> >
>> > Overall, it remains unclear why this change is done.
>>
>> In the original code, I picked 32 kHz mono as the format to keep
>> computational complexity at something that seemed would work well on
>> low-end hardware (basically netbooks) without losing too much in the
>> way of quality.
>>
>> Prior to this change, the code assumes that the canceller will pick
>> something similar for us on the sink side, based on what we're
>> requesting on the source side (which is that fixed format).
>>
>> Does that make more sense now?
>
> Maybe. Is this correct: The intention has always been to configure low
> enough parameters to keep CPU consumption down. Prior to this change,
> we assumed that the EC backend would override the sink parameters based
> on the source parameters to achieve this goal, and with this change we
> remove that assumption by forcing the default parameters for the sink
> to be low enough.
>
> I hope you rewrite the commit message.

Yep, I'll rewrite the message, probably just reusing this text.