[pulseaudio-discuss] [PATCH v4 05/23] echo-cancel: Relax restrictions on webrtc AEC stream config

[Arun Raghavan]

On Sat, 2016-02-20 at 15:35 +0200, Tanu Kaskinen wrote:
> On Wed, 2016-02-17 at 19:46 +0530, arun at accosted.net wrote:
> > From: Arun Raghavan <git at arunraghavan.net>
> > 
> > This exposes the range of configurations the library actually supports
> > (8, 16, 32 and 48 kHz).
> 
> That doesn't seem to explain all changes in the code. I'll analyze all
> the variables:
> 
> play_ss.format is handled as before, fixed to s16le.
> 
> play_ss.rate was earlier set to out_ss.rate, now it's set to one of the
> four possible rates based on the initial rec_ss.rate value.
> 
> play_ss.channels and play_map were earlier set to out_map, now they are
> unchanged.
> 
> rec_ss.format is handled as before, fixed to s16le.
> 
> rec_ss.rate was earlier set to out_ss.rate, now it's set to one of the
> four possible rates based on the initial rec_ss.rate value.
> 
> rec_ss.channels and rec_map were earlier set to out_map, now they are
> unchanged.
> 
> out_ss.format is handled as before, fixed to s16le.
> 
> out_ss.rate was earlier kept unchanged, now it's set to one of the four
> possible rates based on the initial rec_ss.rate value.
> 
> out_ss.channels and out_map were earlier kept unchanged, now they are
> set to the original value of rec_map.
> 
> In summary: no changes in sample format configuration, rate is based on
> the original rec_ss instead of the original out_ss and limited to the
> four possible rates (earlier there was no such limitation, so is the
> commit message wrong saying "relax restrictions"?), channels are based
> on rec_map instead of out_map.

Functionally, there is no difference. out_ss/map and rec_ss/map
essentially had the same value, and I just used them interchangeably
(which now IMO is confusing).

In the refactoring, I'm expressing the constraints in what I see to be
a more natural way -- rec_ss expresses what we're feeding the
canceller, so it makes sense to apply the constraints on what the
canceller accepts there. This then propagates to the output spec.

This all seems fairly minor to me, but I can add it to the commit
message as well if you want.

> There is and there was no limitation on the channel count - but should
> there be? I don't think the processing can handle very many channels,
> at least before switching to deinterleaved processing.

The AudioProcessing API doesn't document constraints on the channel
count (I suppose there are constraints internally, though, given what
you'd posted earlier w.r.t. the AudioFrame buffer size). I think fixing
that in this commit would be wasted effort since, as you point out, the
deinterleaved processing does make that irrelevant.

-- Arun