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

[Tanu Kaskinen]

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.

-- 
Tanu