[pulseaudio-discuss] commit: resampler: Move some peak resampler asserts around

Maarten Bosmans mkbosmans at gmail.com
Wed Dec 21 14:39:17 PST 2011


This commit:
http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=dc8edf4b43bf20e9edf6efd31836218924fad1f0

resampler: Move some peak resampler asserts around
This moves a couple of asserts from peak_resample() to peaks_init()
since they're resampler parameters that shouldn't change after
initialisation.

is wrong IMHO.

The whole point of asserts is to protect from programmer errors. This
can be null pointers (the other asserts in that function), but also
parameters that can have only certain values. The fact that the assert
can also be placed at init time is irrelevant. As the commit message
says, they _shouldn't_ change after init, the assert just makes sure
that they really haven't.

Furthermore, the assert also functions as a code comment. As further
down in the function there is code like:

if (r->work_format == PA_SAMPLE_S16NE) {
  ...
} else {
  ...
}

With the assert
pa_assert(r->work_format == PA_SAMPLE_S16NE || r->work_format ==
PA_SAMPLE_FLOAT32NE);
in the same function, it is clear that the else block handles the
float32 part, without the assert, that fact is hidden from the
programmer.

I consider this second documentation aspect of the assert as least as
important as the correctness aspect of it.

It would be nice if patches like this could be submitted to the
mailing list first, instead of committing directly. That way they can
gather feedback. If there's no reaction within a week or so, just
commit it.

Maarten


More information about the pulseaudio-discuss mailing list