[Clam-devel] Re: [PATCH] Re: adding spectrum magnitude to annotator discussion

[David García Garzón]

Bennet i committed most of it but it needs urgent fixes.

First of all PoolFloatArrayDataSource::updateData takes FirstBinOffset and 
BinGap from the parent attribute instead of the attribute you are addressing. 
The parent attribute is the FrameDivision, it is an attribute that says how 
the frames are placed along time, which uses a similar description in time (a 
FirstBinOffset and a BinGap) than the spectrum distribution in frequency.

So, instead of taking them on 'updateData', you should take such attributes on 
PoolFloatArrayDataSource::setDataSource method, the method where the schema 
is set, and take them as we currently take the bin labels.

setDataSource: The source pool has changed, the schema must be updated
updateData: The data (or current frame) has changed, data must be updated

Besides that, such members of PFADS class (FirstBinOffset and BinGap) are 
pointers to values that exist outside of the object. That's very dangerous as 
the external object may change its structure. Keep a copy of the value 
instead. If it is not present, then set a default value (0 and 1 are a nice 
guess) and if you need to know whether they are set (HasX) then use a bool 
member.

So many errors should have ended in something noticeable but in fact we are 
not having any error because we are not using them. They should be used on 
PoolFloatArrayDataSource::getLabel, but now it is computing the label by 
returning the string formatting of NBins, instead the string formatting of 
offset+i*bingap.

And just for keeping the code clear, all the code to get the nBins at 
BarGraphPlugin::ConfigureDialog is useless. We need it on the other plugins 
as we also filter by the number of bins (12 for the Tonnetz and 24 for the 
KeySpace). But that's not the case here. I'll remove that code before 
committing it. That might cause a svn conflict on you sandbox. Try to fix the 
rest.

David.


On Thursday 05 July 2007 06:16:02 bennett kolasinski wrote:
> Attached is an update to ClamExtractorExample that incorporates your
> suggestions from this email and IRC discussion as well as changes to
> InstantViewPlugin.  I'm not seeing SpectrumMagnitude as one of the
> attributes that can be visualized using BarGraph-- I'll look more into it
> in the morning-- but as far as I can tell everything else is working fine.
>
> B
>
> On 7/4/07, David García Garzón <dgarcia at iua.upf.edu> wrote:
> > On Wednesday 04 July 2007 07:40:40 bennett kolasinski wrote:
> > > Hi David-
> > > Adding SpectrumMagnitude to ClamExtractorExample seems to really blow
> > > things up.  First off, ClamExtractorExample seems to be hanging on the
> >
> > call
> >
> > > to restoring the XML file now (with the last patch I sent):
> > > CLAM::XMLStorage::Restore(toValidadDescription, poolFile);
> > >
> > > I'm not sure if it's actually hung or just taking a really long time to
> > > write what appears to be a 40.2 MB pool for a 3mb MP3 file.  Actually,
> > > looking at the file that was allocated, it looks like it hasn't been
> > > touched since it was initially written-- maybe the file has been
> >
> > written?
> >
> > > do you have any reason to believe why XMLStorage::Restore() would hang
> > > (maybe because it's such a large XML file)?  I just set it to run with
> >
> > the
> >
> > > 'time' command and will see if it finishes up (and how long it takes)
> > > by the time I wake up in a few hours.  Interestingly the pool file that
> > > was allocated on my hard drive was the same size whether I ran it with
> > > 512
> >
> > bins
> >
> > > or 10 bins.
> >
> > Suspicious. Take a look to the actual content of the xml.
> >
> > > Have you had a chance to run the patched ClamExtractorExample?  Am I
> > > running it correctly by simply passing the desired audio file into it?
> > >
> > > ./ClamExtractorExample mp3file.mp3
> > >
> > > ClamExtractorExample -s produces what appears to be a valid schema...
> > >
> > > BTW, I believe the code changes to PoolFloatArrayDataSource that you
> > > suggested are complete, but I don't want to send that patch until I get
> >
> > a
> >
> > > fresh look at it in the morning.
> >
> > Well, if you send it, we could help you more on giving you an
> >
> > > Any ideas would be appreciated!  Is SpectrumMagnitude too... 'dense'
> > > (?) for Annotator?  Do we need a more efficient way of storing it than
> > > a plaintext file?
> >
> > Yes we should have a more efficient way, but, the short goal now is not
> > storing spectrums but having FloatArrays with numeric bin labels and
> > being able to represent them. So a smaller array should do the work. So,
> > keep your
> > focus and minimize distractions from it.
> >
> > BTW, the TODO wiki is a nice tool set such focus. I would recommend you
> > to mark your current task and subtasks in a special way. Also, writting
> > down all
> > those tasks you see but that go away from your current focus is a good
> > practice as let you take them out of your mind. Also to marking done task
> > with [[Image:done.png]] helps you to have the sensation of progress which
> > can
> > motivate you a lot.
> >
> > David.