[Clam-devel] [PATCH] ChordSegmentator... not sure whether to commit

[Roman Goj]

Hi!

I have a patch here I have some doubts about, so - not committing, and
I'd appreciate some feedback... :)

For now the patch isn't that bad, just adds a second parameter to the
doIt in ChordExtractor... but I'm afraid this is just an intro for
bigger (and worse things to come).

Maybe some explanation what I've been doing lately (with David, of course):
We want to make a new port or control on TonalAnalysis that would have
the chord segmentation as output (this is a DiscontinuousSegmentation
object). This output could then be used to display segmentation on the
in-development realtime Spectrogram (or on a miriad of other goodies
;) ). My problem is that the new refactored ChordSegmentator class
works by putting time-borders into the segmentation object. So - it
needs to know what point in time it is currently working on (thus
ChordSegmentator::doIt takes CLAM::TData currentTime and so does now
-in this patch- ChordExtractor::doIt). This works very nicely in the
offline Annotator - already tested - the output is the same as it was
before I even touched CLAM ;-) Now the real problem is that this also
needs to be done in realtime and AFAIK TonalAnalysis has no means of
telling which point in the audiofile/source it is currently
processing... (it could i.e. have a _time variable, initiate it to 0
and then increment using hop and sampling rate... but then if we play
the network with MonoAudioFileReader again... it won't know that it's
currently processing the 1st second again, it will keep adding
seconds... this is would be one solution, but I really think this
isn't good... though maybe I should stick with it for now?).

I guess that the only processing that actually knows at which point in
time it's stuck at any given moment is the audio source (like
MonoAudioFileReader) ... so maybe MonoAudioFileReader should
communicate it's knowledge to TonalAnalysis? So - either a new port
just for time (I'd say - not a very good idea... I might be wrong
though), or the audio out port should get a new GetCurrentTime (or
something) accessor?

I can write this... but this seems like a really big change... maybe
I'm missing some other obvious method of doing this? (so far I'm
something like 50-50 on this question this summer - sometimes I am
missing something obvious, sometimes the problem is as complicated as
it seems to me ;-) so - what will it be now? :) )

Also it seems this is not only my problem - Bennett will probably also
need some time information for his spectrogram (or have you solved
this already, Bennett? :-) )

So - changelog (I can commit it though, if you approve, or after
making necessary changes)
* new class ChordSegmentator in Processings/Analysis/Tonal (file -
ChordsSegmentator.hxx attached),  just a copy of the
ChordExtractorSegmentation class
* an instance of ChordSegmentator is now a member of ChordExtractor
* accessors for ChordSegmentator's output added to ChordExtractor
* ChordExtractor (the exec being part of Annotator) now uses the
segmentation through ChordExtractor (and, most surprisingly - to me at
least, since I've been writing it - it actually works ;-) ), this
means i.e. extractor.segmentation() instead of
chordSegmentation.segmentation()
* (this is also a sad part of the patch) ChordExtractor extractor is
no longer used as const in MainClam.cxx, because of the new function
closeLastSegment in ChordExtractor (just calling closeLastSegment in
ChordSegmentator), which can't be const (or - again - am I wrong about
this? I think I need to brush up on this qualifier, actually, strange
as it may seem, I've never used it in my own code so far...)
* (not related to the big issues) fixing a small bug I introduced -
HopRatio in TonalAnalysisConfigis now double and not unsigned (this
reduces the number of conversion warnings at least by 1 ;) )

These are actually three patches in one, sorry! I had really big
doubts though whether this would really work when I reached the point
of actually using it in Annotator's ChordExtractor - wanted to test
the code first, so as not to introduce (again) code I would have to
delete or change completely in the next patch...

Also I had some problems I'd like to ask you about... but this should
maybe be a separate post, as it's not really related to the changes...

Sorry for such a long post again!!
Even so - hoping for feedback!

cheers,
roman
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ChordExtractor-doIt-now-takes-currentTime-as-input.patch
Type: text/x-diff
Size: 6928 bytes
Desc: not available
URL: <http://lists.clam-project.org/pipermail/clam-devel-clam-project.org/attachments/20070803/29bae123/attachment-0002.patch>