Closed GoogleCodeExporter closed 8 years ago
Just realized maybe I should throw in some screenshots...here are a few that
pretty much cover the functionality. There's an example of it playing nicely
with the waveform overlay as well.
Original comment by cincoden...@gmail.com
on 19 Apr 2013 at 6:55
Attachments:
The spectrogram patch works fine on Mac OS X after installing the extra
dependency.
The close button code looks fine but it will malfunction if it is used while a
dialog is showing inside EOF. For this to work properly, the 'eof_quit_mustask'
variable would need to be reset to 0 after every blocking dialog call.
Original comment by xander4j...@yahoo.com
on 19 Apr 2013 at 2:40
Hmm, yes I noticed that when working on it yesterday...when I closed it it just
waited until I closed the dialog to present the confirmation message, and then
worked normally. I'll take a look at it.
Original comment by cincoden...@gmail.com
on 19 Apr 2013 at 6:44
How did we want to integrate FFTW with EOF's source code? Put the source files
in or make it built separately (like Allegro and its dependencies)?
Original comment by raynebc
on 19 Apr 2013 at 7:21
I would rather have it be a dependency since it is a large library with many
source files. I think it is a well-established library so it shouldn't be a big
deal to require people to have it installed if they want to build EOF.
Original comment by xander4j...@yahoo.com
on 19 Apr 2013 at 9:12
It would be easy to add an EOF_SPECTROGRAM compiler flag so that the related
code only compiles if it is wanted. What changes would need to be made to the
makefiles so that the FFTW and spectrogram source code are only linked if they
were wanted? Would separate makefiles be needed?
Original comment by raynebc
on 19 Apr 2013 at 9:39
Separate library makes sense to me - it's readily available in Ubuntu's
repositories, for instance, starting with Lucid (April 2010) and is backported
to Hardy LTS (April 2008), so it shouldn't be difficult for people to get on
Linux anyway. I just tried a fresh clone/build on a different machine here and
it worked fine with the default package.
As for conditionals, can compile flags take care of the dependencies? There's
just -lm and -lfftw3, plus the added o-files. I'm not terribly familiar with
them. There are calls to spectrogram functions peppered throughout the main
code, everywhere the waveform calls are - I could either conditionally stub
those out in spectrogram.c as I mentioned, or put #ifdefs around all of the
calls and just not compile spectrogram.c.
Original comment by cincoden...@gmail.com
on 19 Apr 2013 at 10:02
I was able to get EOF built and running with the great spectrogram code
(updated and tested my Code::Blocks project and tested one of the makefiles),
so r1131 adds the spectrogram logic. We can iron out the details of
deactivating the spectrogram logic based on compiler/linker settings in the
next few commits.
Original comment by raynebc
on 19 Apr 2013 at 11:54
The next EOF release candidate is long overdue, so for now, I'm going to bundle
FFTW's license and copyright notices and readme file in a subfolder of the
binary package. I'm not sure if there's a more appropriate means to meet the
requirements since the source code itself isn't going to be maintained in EOF's
code base.
Original comment by raynebc
on 20 Apr 2013 at 3:23
For GPL you just need to include a copy of the license. Since EOF is
open-source and free, I don't think it will be an issue. The GPL license has to
be included with both source and binary releases.
It may be a good idea to mention in the license file which part is licensed
under GPL so users can have a clearer picture of the copyrights on all of the
code.
Original comment by xander4j...@yahoo.com
on 20 Apr 2013 at 4:12
To fix the close button, I would recommend just wrapping the few blocking
dialog functions we use and grep-ing the source to replace calls to those
functions with our own wrapped versions. The wrappers would fit nicely in
'dialog.c'.
The main dialog function we use for custom dialogs is already there so we just
need to deal with alert() and whatever else we are using.
Original comment by xander4j...@yahoo.com
on 20 Apr 2013 at 4:16
I'm not sure where in the source the GPL license should go, the root of the src
folder?
Original comment by raynebc
on 20 Apr 2013 at 5:25
It should go with the binary files only so it is easier to package. The source
code includes the binary data so it will work this way.
Original comment by xander4j...@yahoo.com
on 20 Apr 2013 at 6:23
File names like "COPYRIGHT" aren't very informative, should I keep the files in
the FFTW subfolder so people know what they're about? Or should we rename the
files (ie. "FFTW COPYRIGHT")?
Original comment by raynebc
on 20 Apr 2013 at 7:07
I would just rename it. Something like 'FFTW_license.txt' should be fine (I
prefer the filename not to have spaces).
The main license file can mention that the FFTW library is used under the terms
of the GPL and reference the name of the file so users can check it out if they
care to.
Original comment by xander4j...@yahoo.com
on 20 Apr 2013 at 8:13
The license stuff should be covered as of r1134.
Original comment by raynebc
on 20 Apr 2013 at 10:55
After the changes that have been made to 5of0's original spectrogram logic,
there are some differences in how the graph looks, especially with smaller
window sizes (see attachments). I reviewed all the changes I made and fixed
one cached value issue with r1136, but I don't see where I made any other
mistakes so I can't tell if this change was due to a bug in the original code
or a bug in the changes I've made. Would you be able to glance at it to see if
I goofed it up?
Original comment by raynebc
on 21 Apr 2013 at 2:06
Attachments:
Yeah, I'll take a look - I've been doing some other improvements that may also
fix things. I'll pull down your changes and look at it in the next couple days.
Original comment by cincoden...@gmail.com
on 21 Apr 2013 at 6:23
Actually, I can't reproduce that difference anymore even though I kept the
executable created with r1131. I must have just been mistaken about which
settings or audio were being used in each.
Original comment by raynebc
on 22 Apr 2013 at 10:03
Hey all, not sure if this is the best place to update things. I've been off on
vacation and otherwise keeping busy, but I've also ported over a few more
features of the spectrogram display from my first attempt, as well as adding
some more cacheing to hopefully improve (somewhat) on the speed while
displaying it. I've finally gotten them all wrapped up into a clean patch on
r1148.
There are quite a few changes here, which are again more parted out over on
GitHub, but I've tried to comment helpfully. Changes of note:
* Added a little more docs to the html documentation - this is actually merging
what I had written while you were adding your own docs, so edit as you see fit.
* Adding ability to view just a given y-range of the spectrogram, specified by
note names, with my supporting helper includes.
* Caching the colorscale table, to avoid calculating colors every time
* Caching the y-axis pixel->frequency map, to avoid calculating that every time
* Removing the averaging if there were multiple bins in a pixel to improve
speed (this doesn't seem to affect the display much at all)
* Rearranging the settings dialog for new options, removing the built-in 32
window size (not a useful window size)
I also noticed the re-creation bug you'd fixed as well while I was debugging
things - thanks :) I've attached a patch with all my changes - it's a big
patch, but I've tried to comment well and such. Feel free to ask about things,
here or I'll be in #fofix on and off this week. There are probably more
optimizations/caching/etc that could be done, but I'm not really sure what
would be the best way to approach it - it's similar to the waveform in a lot of
ways, but is obviously more intensive to render.
Original comment by cincoden...@gmail.com
on 19 May 2013 at 11:42
Attachments:
Thanks, I uploaded your changes in r1149. Rendering performance is HUGELY
increased! One thing though is that no matter which options I pick, the graph
looks a little different (ie. grainier) than it used to when using a 1024
window size. I'm not sure why that is.
Original comment by raynebc
on 20 May 2013 at 10:42
Heh, nice little benchmarking tool you threw together there. I did some
comparative runs with gprof and the numbers looked good, but the image sequence
dual-purposing is a much more accurate measure. Glad things are looking good
there too!
Off the top of my head, the graininess may be a symptom of disabling averaging.
Did you try checking the avg bins option? I know that option does
*something*, but I may have inadvertently changed how the averaging works.
I'll compare some screenshots between builds and see myself.
Original comment by cincoden...@gmail.com
on 20 May 2013 at 11:11
I tried every combination of the options (average, log plot, both, neither),
and I never saw identical graphics compared to your original spectrogram code,
so there may be some problem that creeped up.
Original comment by raynebc
on 22 May 2013 at 7:17
Minor graphical differences aside, I don't think there's much else to
accomplish with the spectrogram. Should we go ahead and close this out?
Original comment by raynebc
on 25 Apr 2014 at 10:17
I'll go ahead and close this out, it seems about as optimized as it will get.
Original comment by raynebc
on 17 Dec 2014 at 9:48
Original issue reported on code.google.com by
cincoden...@gmail.com
on 19 Apr 2013 at 6:43Attachments: