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

[Murray Meehan]

Hi David,

Thanks for the information! I will update the patch as soon as I am back
from a conference, so some time next week hopefully.

- Murray

On Fri, Aug 19, 2011 at 9:46 PM, David Garcia Garzon <
david.garcia at barcelonamedia.org> wrote:

> 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
> _______________________________________________
> clam-devel mailing list
> clam-devel at lists.clam-project.org
> http://lists.clam-project.org/listinfo.cgi/clam-devel-clam-project.org
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.clam-project.org/pipermail/clam-devel-clam-project.org/attachments/20110819/c763b611/attachment-0004.htm>