On Wed, 2016-02-17 at 18:40 +0530, Arun Raghavan wrote:
> On Wed, 2016-02-17 at 18:36 +0530, Arun Raghavan wrote:
> > On Tue, 2016-02-16 at 14:20 +0200, Tanu Kaskinen wrote:
> > > On Mon, 2016-01-18 at 13:06 +0530, arun at accosted.net wrote:
> > > > +Â Â Â Â /* FIXME: we need to be able to modify playback samples */
> > >
> > > This comment should say also why we need to be able to modify them
> > > (and
> > > I also don't understand what's preventing us from doing so now).
> >
> > Reworded this as:
> >
> > /* FIXME: we need to be able to modify playback samples, which we can't
> > Â * currently do. This is because module-echo-cancel processes playback
> > Â * frames in the source thread, and just stores playback chunks as they
> > Â * pass through the sink. */
Mmmh, that still doesn't answer the question why we need to be able to
modify playback samples. What processing do we want to do that we can't
currently do?
> > > > @@ -573,4 +575,9 @@ void pa_webrtc_ec_done(pa_echo_canceller *ec)
> > > > {
> > > > Â Â Â Â Â Â Â Â Â delete (webrtc::AudioProcessing*)ec->params.webrtc.apm;
> > > > Â Â Â Â Â Â Â Â Â ec->params.webrtc.apm = NULL;
> > > > Â Â Â Â Â }
> > > > +
> > > > +Â Â Â Â for (i = 0; i < ec->params.webrtc.rec_ss.channels; i++)
> > > > +Â Â Â Â Â Â Â Â pa_xfree(ec->params.webrtc.rec_buffer[i]);
> > > > +Â Â Â Â for (i = 0; i < ec->params.webrtc.play_ss.channels; i++)
> > > > +Â Â Â Â Â Â Â Â pa_xfree(ec->params.webrtc.play_buffer[i]);
> > >
> > > I think it's not guaranteed that rec_ss and play_ss are initialized
> > > at
> > > this point. You should check that rec_buffer and play_buffer are
> > > non-
> > > NULL before accessing them.
> >
> > There should not be a path where rec_buffer and play_buffer are not
> > initialised before done, but I'll add that check in case this becomes
> > possible in the future.
>
> Shouldn't be necessary, actually. pa_xfree() is NULL-safe.
The problem I thought there to be was that the arrays themselves were
dynamically allocated and possibly NULL, but that's not the case, so
never mind :)
--Â
Tanu
