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-boun...@lists.clam-project.org 
[clam-devel-boun...@lists.clam-project.org] En nom de Murray Meehan 
[murray.mee...@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
_______________________________________________
clam-devel mailing list
clam-devel@lists.clam-project.org
http://lists.clam-project.org/listinfo.cgi/clam-devel-clam-project.org

Reply via email to