[clam-devel] Patch: unsigned int and bracket usage

[David Garcia Garzon]

i would love to commit most of the changes in the patch, which solves those nasty warnings in the wanted list for long. But the patch is quite extensive and includes some changes that should be reviewed.

Most of the changes consisting in commenting out an unused var declaration should just delete the declaration. We learned that commented code becomes evil with time.

CLAM/src/Data/BasicProcessing/SpectralPeakArray.cxx: It just contains a question in a comment. That is not to be commited but the question should be dropped to the list instead. i guess you spotted a cut and paste bug. Being that buggy i wonder whether the spectral peak addition operator is even used at all. So a first step would be comment out the operator, and if all compiles, and works remove it. But in a separate patch/commit so it can be easily undone independently from this warning patch/commit

CLAM/src/Processing/ArithOps/AudioBufferMixer.cxx: The portSize is the number of samples in each audio buffer, we take it from BAckendBufferSize because it is legacy code from AudioMixer (not buffer), so it can be dropped as you did, but as i said, not with comments.

Some processings had the factory registration commented out, which you activated again. They were disabled by reasons i ignore (and are not documented :-P ) but i remember that for most of them there was a reason for that, like in TimeStretch. Pau, Xavi, could you put some light here? The commented processings Murray uncommented are: ThreeBandGate, SpectralNotch and Deesser but there are more. While we don't know why, i would not commit that or I would prepare a separate patch.

CLAM/src/Processing/ArithOps/SpectrumProduct.cxx: You add a comment asking about CLAM_(DEBUG_)ASSERT. When removing unused vars we should be aware of such cases. CLAM_(DEBUG_)ASSERTS contains code which compiles or not depending on the scons configure options. CLAM_ASSERT are enable by default but can be disabled with checks=0 while CLAM_DEBUG_ASSERTS are disabled by default but enabled if release=0. So if a declaration, or any other code not in the assert expression, is just used in an assert, we should include it in between CLAM_BEGIN_(DEBUG_)CHECK and CLAM_END_(DEBUG_)CHECK. But, in this casei wouldn't even do that, i would use the references in the Do call, so that the code is simpler and the out reference it is used, removing the warning.

Please, could you update the patch?

Thanks.
David.

________________________________________
De: clam-devel-bounces at lists.clam-project.org [clam-devel-bounces at lists.clam-project.org] En nom de Murray Meehan [murray.meehan at gmail.com]
Enviat el: dimarts, 16 / agost / 2011 23:04
Per a: CLAM development list
Tema: [clam-devel] Patch: unsigned int and bracket usage

Hi,

See attached my patch of many minor problems which were causing compiler warnings.

Murray