On Mon, 2016-01-18 at 13:06 +0530, arun at accosted.net wrote:
> From: Arun Raghavan <git at arunraghavan.net>
>
> Needed for upcoming beamforming code.
> ---
> Â src/modules/echo-cancel/echo-cancel.h |Â Â 2 +-
>  src/modules/echo-cancel/webrtc.cc     | 14 ++++++++------
> Â 2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/src/modules/echo-cancel/echo-cancel.h b/src/modules/echo-cancel/echo-cancel.h
> index 4693516..613f7e3 100644
> --- a/src/modules/echo-cancel/echo-cancel.h
> +++ b/src/modules/echo-cancel/echo-cancel.h
> @@ -65,7 +65,7 @@ struct pa_echo_canceller_params {
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â * to C++ linkage. apm is a pointer to an AudioProcessing object */
> Â Â Â Â Â Â Â Â Â Â Â Â Â void *apm;
> Â Â Â Â Â Â Â Â Â Â Â Â Â int32_t blocksize; /* in frames */
> -Â Â Â Â Â Â Â Â Â Â Â Â pa_sample_spec rec_ss, play_ss;
> +Â Â Â Â Â Â Â Â Â Â Â Â pa_sample_spec rec_ss, play_ss, out_ss;
> Â Â Â Â Â Â Â Â Â Â Â Â Â bool agc;
> Â Â Â Â Â Â Â Â Â Â Â Â Â bool trace;
> Â Â Â Â Â Â Â Â Â Â Â Â Â bool first;
> diff --git a/src/modules/echo-cancel/webrtc.cc b/src/modules/echo-cancel/webrtc.cc
> index 5741f45..5f00286 100644
> --- a/src/modules/echo-cancel/webrtc.cc
> +++ b/src/modules/echo-cancel/webrtc.cc
> @@ -333,6 +333,7 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> Â Â Â Â Â ec->params.webrtc.apm = apm;
> Â Â Â Â Â ec->params.webrtc.rec_ss = *rec_ss;
> Â Â Â Â Â ec->params.webrtc.play_ss = *play_ss;
> +Â Â Â Â ec->params.webrtc.out_ss = *out_ss;
> Â Â Â Â Â ec->params.webrtc.blocksize =
> Â Â Â Â Â Â Â Â Â (uint64_t) (pa_bytes_per_second(out_ss) / pa_frame_size(out_ss)) * BLOCK_SIZE_US / PA_USEC_PER_SEC;
> Â Â Â Â Â *nframes = ec->params.webrtc.blocksize;
> @@ -372,17 +373,18 @@ void pa_webrtc_ec_play(pa_echo_canceller *ec, const uint8_t *play) {
> Â void pa_webrtc_ec_record(pa_echo_canceller *ec, const uint8_t *rec, uint8_t *out) {
> Â Â Â Â Â webrtc::AudioProcessing *apm = (webrtc::AudioProcessing*)ec->params.webrtc.apm;
> Â Â Â Â Â webrtc::AudioFrame out_frame;
> -Â Â Â Â const pa_sample_spec *ss = &ec->params.webrtc.rec_ss;
> +Â Â Â Â const pa_sample_spec *rec_ss = &ec->params.webrtc.rec_ss;
> +Â Â Â Â const pa_sample_spec *out_ss = &ec->params.webrtc.out_ss;
> Â Â Â Â Â pa_cvolume v;
> Â Â Â Â Â int old_volume, new_volume;
> Â
> -Â Â Â Â out_frame.num_channels_ = ss->channels;
> -Â Â Â Â out_frame.sample_rate_hz_ = ss->rate;
> +Â Â Â Â out_frame.num_channels_ = rec_ss->channels;
> +Â Â Â Â out_frame.sample_rate_hz_ = rec_ss->rate;
> Â Â Â Â Â out_frame.interleaved_ = true;
> Â Â Â Â Â out_frame.samples_per_channel_ = ec->params.webrtc.blocksize;
> Â
> Â Â Â Â Â pa_assert(out_frame.samples_per_channel_ <= webrtc::AudioFrame::kMaxDataSizeSamples);
Not directly related to this patch, but this assertion seems wrong. We
compare kMaxDataSizeSamples to the number of frames, but
kMaxDataSizeSamples is calculated as frames * channels.
Also, kMaxDataSizeSamples assumes that the maximum sample spec is 16-
bit, stereo, 32 kHz, and that the maximum buffer size is 60 ms.
However, we seem to allow using 48 kHz, and I didn't find any
limitations for the channel count. We seem to use 10 ms buffers, so
that gives some headroom. webrtc_ec_fixate_spec() should check all
this, and make sure that our parameters don't exceed
kMaxDataSizeSamples.
The changes in this patch all look good.
--
Tanu
