CVS compile issues
I was returning to the freeamp source code, and I have updated the musicbrainz library from cvs, but I can't get current freeamp cvs to compile. The error: c++ -I. -I. -I./config -DUNIX_LIBDIR=\"/opt/freeamp/lib\" -Dlinux -I. -I./lib/gdbm -I./base/include -I./config -I./io/include -I./ui/include -I./lmc/include -I./io/soundcard/unix/linux/include -I./lmc/xingmp3/include -I./lmc/cd/include -I./plm/portable/pmp300/sba -I./lib/xml/include -I./lib/zlib/include -I./lib/unzip/include -I./io/cd/unix/include -I./base/aps -I./io/wavout/include -I./ui/lcd/include -I./ui/irman/include -I./lib/http/include -I./io/signature/include -I./lmc/vorbis/include -I./base/unix/include -I./base/unix/linux/include -I./io/esound/include -I./ui/musicbrowser/unix/include -I./ui/freeamp/include -I./ui/freeamp/unix/include -I./ui/download/unix/include -I./ui/musicbrowser/include -I./ftc/kjofol -I/usr/athena/lib/glib/include -I/usr/athena/include -I/usr/include -I./io/alsa/unix/linux/include -Wall -g -O2 -I/usr/lib/glib/include -I/usr/X11R6/include -I/usr/lib/glib/include -I/usr/X11R6/include -D_REENTRANT -fPIC -c lmc/vorbis/src/vorbislmc.cpp -o lmc/vorbis/! src/vorbislmc.o c++ -shared -Wl,-export-dynamic -o plugins/vorbis.lmc lmc/vorbis/src/vorbislmc.o lmc/vorbis/src/lib/libvorbis.la make[1]: *** No rule to make target `plm/metadata/mbcd/mbcd.o', needed by `plugins/mbcd.mdf'. Stop. rm lmc/xingmp3/src/mdctasm.asm1 lmc/xingmp3/src/msisasm.asm1 lmc/xingmp3/src/cwinasm.asm1 lmc/xingmp3/src/cwin8asm.asm1 lmc/xingmp3/src/cdctasm.asm1 make[1]: Leaving directory `/usr/local/src/freeamp/freeamp' make: *** [plugins-cc] Error 2 / Makefile-plugins says: #mbcd MBCDOBJ = plm/metadata/mbcd/mbcd.o plugins/mbcd.mdf: $(MBCDOBJ) $(LINKMOD) -o $@ $(MBCDOBJ) $(STATICLD) -lmusicbrainz /// But "ls plm/metadata/" shows only: CVS cddb id3v1 id3v2 misc vorbis // By the way, this problem exists even after a forced reconfigure: [root@kuklewicz freeamp]# find . -name '*mbcd*' [root@kuklewicz freeamp]# rm Makefile-plugins rm: remove `Makefile-plugins'? y [root@kuklewicz freeamp]# rm Makefile rm: remove `Makefile'? y [root@kuklewicz freeamp]# rm Makefile.header rm: remove `Makefile.header'? y [root@kuklewicz freeamp]# rm Makefile.towav rm: remove `Makefile.towav'? y [root@kuklewicz freeamp]# rm config.cache rm: remove `config.cache'? y [root@kuklewicz freeamp]# ./configure ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: More searching musings...
Now all this assumes you want to start each new search at the top of the playlist. If you want to start it form the currently (or most recently played) song, then you could have a one function API: "Go to first song after m_lastindex that matches the pattern/type/casematters and play it" Or you could mimic this by cylicing through the vector of results in PlayFirst so the next song after m_lastindex is played. Okay, this is a well-thought out solution. However, I'm wondering how efficient it is to be allocating/dealllocating these search structures when the actual search will be changing often. This is because for a text-based player, searching is not a method of finding songs, it's a method of moving around. The only functions for changing the song in text mode are "previous" and "next". There are not even "10 forward" and "10 back" options. (Although these would obviously be easy to do.) I figure, why go to the bother of creating these search structures and having to StartSearch/StopSearch when the search target is probably only one song? This is why I envision FindSong as part of PlaylistManager: It would start searching from the song immediatly after the current song, and search the list until it found something or hit the current song. The time spent searching is less, because you do not have to search the entire playlist, but instead search until the first result. It allows iteration because the same search run again will find the next song that matches, and then the song after that... And it doesn't require any memory allocation. Okay, so you want the "Go to first song after m_lastindex that matches the pattern/type/casematters and play it" function. Excellent. Now there is a clear plan: have both a "return all results as a playlist" and the above solution. The playlist is much more complex, so first create the function that the console needs. Here is a question to all: For the pop-up search window I discussed before, what would be the best design to allow all platforms to have the same thing? Try to be specific. -- Chris ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: More searching musings...
On Wed, Oct 04, 2000 at 02:40:10PM -0500, David A. Walker wrote: After pondering the searching issue some more, I came up with a function prototype that will (hopefully) address everyone's needs: int FindSong(char *pattern, int type, int casematters) And the returned int is the index of the first or next matching song? As this would be a C++ method, what new or existing object would own it? Pattern is just the search pattern, type is what type of search to perform, and casematters indicates whether the search is case-sensitive. The types would work like this: the bool works for me Type 0 is a simple search. If the pattern occurs anywhere in the track name, it's a match. Type 1 is a wildcard search. The pattern must match the _entire_ track name, * matches anything (including nothing), and ? matches one character. \ makes the next character have its literal meaning. Type 2 would be a regex search. Anything goes. Use enums for better legibility, but yes this is fine. This could be done easily using the regex routines which are part of libc. Additionally, regex.c (from the sed package) could be included to link against for platforms that lack built-in regex support. Does this address everyone's searching desires? Also, after seeing the various searching suggestions that were posted (especially the one about returning search results as a playlist), it struck me that there are two fundamentally different types of searching support. Lets see how fundamentally differnt they are. The first is a very flexible, powerful search that could look at any (or all) of the metadata fields in the MusicCatalog and create a playlist based on the results. This is great for graphical players, which have a need for on-the-fly playlist modification and the ability to import/export from the catalog. Yes, that was my idea. However, it is nearly useless for text-based players which have a fixed playlist and no good way to import/export from a db. The other method is a fast-and-dirty search that would be quick and easy to use and only check a specific attribute of the song. (eg., the pathname) This method would only return one result, but it could be used multiple times to access different songs that matched the pattern. the API for that is larger than just FindSong. You need to start/stop a given search. This is very useful for the text-based players, which lack any method of jumping to a given song in the playlist, but it is virtually useless for graphical players, where the user can simply click on a song to play it. I believe that most people want the first kind of searching support, because most people like their GUIs. For backwards people like me who still live in the stone age, the latter type is an absolute necessity. Does this sound reasonable? I have never used Freeamp on the console. Perhaps I should. A playlist seems to just a structure like (from PlaylistManager) : vectorPlaylistItem* m_masterList So what you actually want is an API like this vectorPlaylistItem** PlaylistMananger::StartSearch(pattern,type,casematters) Then make a function PlayFirst that takes the returned pointer, pops the first PlaylistItem* off the vector, and jump to that song and starts playing it. Then passing the returned pointer PlayFirst over and over will move forward through the search results. This is the behavior you want. Bind that to the a key, and you are set. To let it loop from the last item to the first item again, move the front item to the end of the vector instead of popping it off (might want a deque instead of a vector). Then make a StopSeach function that cleans up the vector when you are done with the previous search and need a new one. Since you call StopSearch right before StartSearch, you could even combine them into the same function. If you focus on the need to go through each search result in turn, then having an internal view of the playlist like this (a sub-playlist) makes sense. Now all this assumes you want to start each new search at the top of the playlist. If you want to start it form the currently (or most recently played) song, then you could have a one function API: "Go to first song after m_lastindex that matches the pattern/type/casematters and play it" Or you could mimic this by cylicing through the vector of results in PlayFirst so the next song after m_lastindex is played. I like the design discussion that is going on. -- Chris ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: more ?? about strings and memory
I have been working on other features, but I would guess that IF
strings are COW then when I replaced the strings in metadata with char
* in HashStore, THEN strings NOT in metadata could no longer refer to
Metadata and had to start making a private copy of the data. For
instance the multimapstring,string for guid to url.
So my patch might have made some things better and other things worse.
A more complete patch may be needed to actually get any noticeable
benefit.
But the truth is that I do not understand what is going on.
On Tue, Oct 03, 2000 at 09:10:22AM -0700, Mark B. Elrod wrote:
so i whipped up a quick test program that created 10 string objects
and assigned them the same literal value... used about 15 megs of
memory. also tried assigning them the same string value as in:
string foo = "Hello, this is a test of the string memory management in
the STL.";
for(int i = 0; i 10; i++)
{
string* s = new string;
*s = foo;
}
this also took about 15 megs...
so this begs the question of why the memory patch by chris made such
little difference. I would expect for a large catalog that it would
really make a difference.
elrod
___
[EMAIL PROTECTED]
http://www.freeamp.org/mailman/listinfo/freeamp-dev
___
[EMAIL PROTECTED]
http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: Ways to reduce memory
On Tue, Oct 03, 2000 at 08:57:46AM -0700, Mark B. Elrod wrote: 1) As I have said before make plugins load on demand rather than all at once on startup Absolutely. And unload if we can, for instance the themes 2) Ref count and share PlayListItems between music catalog and playlist manager Moving the refcount to a higher level...interesting proposal. The modifications might have be larger, but the benefit is more obvious. 3) Only create playlist items for areas of the tree that are visible or that are added to the playlist. For a large catalog this could make a real difference. It would also speed up load time. We would need to alter the DB though probably bc this really requires a relational DB so the added complexity might not be worth it. This is a "keep the music catalog on disk, not in RAM" proposal. For truly huge playlists this is a win. For small playlists, the kernel would cache the database file and keep things speedy. By having alot of redundancy in the database file, you might be able to avoid the needs for a relational db. Hmm... An intermediate step that would save memory could be this: Since gtk widgets are copying the data, only instantiate the visible ones and then destroy them when they are not visible in the tree. Or wait for the MVC widgets in gtk+ version 2. For someone with a large catalog: What is the RAM difference with no music (remove ~/.freeamp and ~/MyMusic) and the large catalog with the browser open and everything in the playlist? ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Unexpected error handling in HttpInput::Open
When looking at an old playlist, with an old bad http entry,
the words "timed out" were sent to the terminal, and a window
popped up reporting the error, then freeamp exited.
The only "timed out" string in the code is in HttpInput::Open
and this is the problem (from my Doxygen pages):
In /freeamp/io/http/httpinput.cpp :
00571 if (conattempt 50)
00572 {
00573 cout "timed out\n";
00574 ReportError("Host not answering: %s", szHostName);
00575 closesocket(m_hHandle);
00576 return (Error)httpError_CannotConnect;
00577 }
The calling function must be exiting freeamp...I am still tracking that
part down, if anyone can fix it fasterbe my guest
___
[EMAIL PROTECTED]
http://www.freeamp.org/mailman/listinfo/freeamp-dev
Patch to add menu option in music browser
Would someone with win32 building add this option to that option menu
as well?
Index: base/include/playlist.h
===
RCS file: /src/repository/freeamp/base/include/playlist.h,v
retrieving revision 1.62
diff -u -r1.62 playlist.h
--- base/include/playlist.h 2000/09/24 19:26:24 1.62
+++ base/include/playlist.h 2000/10/03 16:38:07
@@ -341,8 +341,10 @@
boolIsEmpty();
uint32 CountItems();
PlaylistItem* ItemAt(uint32 index);
- uint32 IndexOf(const PlaylistItem* item);
+uint32 IndexOf(const PlaylistItem* item);
boolHasItem(const PlaylistItem* item);
+uint32 IndexOfURL(const string url);
+
void RetrieveMetaData(PlaylistItem* item);
void RetrieveMetaData(vectorPlaylistItem** list);
Index: base/src/playlist.cpp
===
RCS file: /src/repository/freeamp/base/src/playlist.cpp,v
retrieving revision 1.113
diff -u -r1.113 playlist.cpp
--- base/src/playlist.cpp 2000/09/28 08:08:00 1.113
+++ base/src/playlist.cpp 2000/10/03 16:38:08
@@ -2350,6 +2350,33 @@
return (IndexOf(item) != kInvalidIndex);
}
+uint32 PlaylistManager::IndexOfURL(const string url)
+{
+// Added by [EMAIL PROTECTED]
+vectorPlaylistItem** alist=m_activeList;
+uint32 result = kInvalidIndex;
+uint32 index = 0;
+uint32 size = 0;
+
+assert(alist);
+
+if(alist)
+{
+vectorPlaylistItem*::iterator i = alist-begin();
+size = alist-size();
+
+for(index = 0; i != alist-end();i++, index++)
+{
+if(url == (*i)-URL())
+{
+result = index;
+break;
+}
+}
+}
+
+return result;
+}
bool PlaylistManager::CanUndo()
{
Index: ui/musicbrowser/unix/include/gtkmusicbrowser.h
===
RCS file: /src/repository/freeamp/ui/musicbrowser/unix/include/gtkmusicbrowser.h,v
retrieving revision 1.56
diff -u -r1.56 gtkmusicbrowser.h
--- ui/musicbrowser/unix/include/gtkmusicbrowser.h 2000/09/25 08:41:42 1.56
+++ ui/musicbrowser/unix/include/gtkmusicbrowser.h 2000/10/03 16:38:15
@@ -325,6 +325,8 @@
void AddTracksPlaylistEvent(vectorPlaylistItem * *newlist,
bool end = false, bool forcePlay = false,
bool forceNoPlay = false);
+/// Added by [EMAIL PROTECTED]
+void PlayInListEvent(vectorPlaylistItem * *newlist);
void AddTracksDoubleClick(vectorPlaylistItem * *newlist);
void PlayEvent();
void StartMusicSearch(bool runMain = true, bool intro = false);
Index: ui/musicbrowser/unix/src/browsertree.cpp
===
RCS file: /src/repository/freeamp/ui/musicbrowser/unix/src/browsertree.cpp,v
retrieving revision 1.32
diff -u -r1.32 browsertree.cpp
--- ui/musicbrowser/unix/src/browsertree.cpp2000/10/02 08:03:45 1.32
+++ ui/musicbrowser/unix/src/browsertree.cpp2000/10/03 16:38:16
@@ -1529,6 +1529,12 @@
p-AddTracksPlaylistEvent(newlist, true, true, false);
}
+static void play_in_list_pop(GTKMusicBrowser *p, guint action, GtkWidget *w)
+{
+vectorPlaylistItem * *newlist = p-GetTreeSelection();
+p-PlayInListEvent(newlist);
+}
+
static void remove_pop(GTKMusicBrowser *p, guint action, GtkWidget *w)
{
p-DeleteEvent();
@@ -1636,6 +1642,7 @@
GtkItemFactoryEntry track_items[] = {
{"/Add to Playlist",NULL, (void(*)(...))add_pop, 0, 0 },
{"/Add and Play Now",NULL, (void(*)(...))add_play_pop, 0, 0 },
+ {"/Play in Playlist",NULL, (void(*)(...))play_in_list_pop, 0, 0 },
{"/sep1", NULL,0,0, "Separator" },
{"/Remove", NULL,(void(*)(...))remove_pop, 0, 0 },
{"/sep2", NULL,0,0, "Separator" },
Index: ui/musicbrowser/unix/src/gtkmusicbrowser.cpp
===
RCS file: /src/repository/freeamp/ui/musicbrowser/unix/src/gtkmusicbrowser.cpp,v
retrieving revision 1.118
diff -u -r1.118 gtkmusicbrowser.cpp
--- ui/musicbrowser/unix/src/gtkmusicbrowser.cpp2000/10/02 08:17:46 1.118
+++ ui/musicbrowser/unix/src/gtkmusicbrowser.cpp2000/10/03 16:38:17
@@ -1335,6 +1335,45 @@
}
}
+void GTKMusicBrowser::PlayInListEvent(vectorPlaylistItem * *newlist)
+{
+if (!newlist)
+return;
+if (0=newlist-size())
+{
+// save the first song that was passed
+vectorPlaylistItem *::iterator item=newlist-begin();
+string firsturl;
+if (*item)
+{
+firsturl=(*item)-URL();
+}
+// remove items in newlist that exist in the playlist
+for (; item != newlist-end(); )
+{
+if
Re: Freeamp html by Doxygen 1.2.2
On Thu, Sep 28, 2000 at 10:36:04PM -0700, [EMAIL PROTECTED] wrote: On 29 Sep, Chris Kuklewicz wrote: This is hot off the presses, and on my personal machine: http://kuklewicz.mit.edu/freeamp/ [ It takes as long a full compile to generate the html docs ] To really appreciate it, use a graphical browser instead of lynx... Sweet -- are you going to keep this up to date? If so, I'll link to it from the FreeAmp developers page. This can come in really handy... I will not be able to always track cvs, but I will keep Apache on. So go ahead and link. I have not booted in Win95 for quite a while, but if I need to do so, it will be temporarily unavailable. I will update my root page to point to it, and add a link to the Doxygen file that cranked it out. (The wonderful plots use IBM's (?) graphviz as a helper program). So don't wget the docs from my machine, I'll just post a "micro-HOWTO" so you can generate them locally yourself, teach a man to fish and all that... hehe, I'm the only one to use namespaces... ;) -Sean I noticed the extra namespace. Good Job! :) --- If you know Javadoc, then it is easy for you to add a one line description of things in the header files. Doxygen will use this. As I patch things, I'll leak such deltas into the patch. I only turned on the html generation. Doxygen can also crank out man pages. Think "man 3 PlatlistItem" and such. Any takers? If you know LaTeX, you might be interested to know it can produce LaTeX from the code comments, including typeset functions. Or for the less ambitious, RTF output is another option. There is a doxysearch thing, which I will learn to setup. -- Chris Kukelwicz ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: What *is* the GUID
On Thu, Sep 28, 2000 at 03:36:47PM -0400, Sean Ward wrote: The GUID-url map is because the metadatabase is keyed off URL. When you want to retrieve the metadata, or at least the filename of a given song, given the song GUID, you use that map to get the filename. The GUID is unique, in that any files which share the same GUID are duplicates of each other, perhaps the same track on multiple albums. However, they sound the same, so it is irrelevant which one is retrieved when, for instance, inserting into a playlist a list of songs, as identified by the song GUID. -Sean I am worried not about insertion, but removal. Thanks for all the clarification, everyone. I can proceed now. ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: URL of patch for HashStore
On Thu, Sep 28, 2000 at 04:42:07PM -0400, Isaac Richards wrote: Okay, I've checked this stuff out.. Few helpful tips =) - When generating a patch against CVS, please remember to update your local copy. Guilty. Very guilty. I knowingly and cowardly and deliberately violated best practice. I am pretending that I am on a branch, since my changes are so scattered. I will bite that bullet in future cases. - Remember to remove your editors temporary files.. I tried with commands like: rm `find -name '*~' -o -name '.#*'` - look at your patch before posting it =) Guilty as charged. I only glanced at it. ie., freeamp-hashstore-1.cvs.patch.gz is 525k. If you remove all the junk in the patch, it goes down to 35k. Oooh. On to comments 'bout what it's doing. - Shouldn't MetaData::SetCharStore be called before the music catalog is loaded? I call that in unix main bootstrap.cpp and in the player ctor. That could be fixed. - Memory usage before and after using the patch is identical. Tested by loading freeamp, adding all tracks in MyMusic to the current playlist, and playing it. CVS: Size: 22624 RSS: 18832 CVS+patch: Size: 22608 RSS: 18808 Damn. What compile flags were use? Not that it really matters Since my coplies take forever I have not really tested the memory usage. Are duplicate STL strings copy on write? Hmmm... As for the MVC widgets in gtk...we could just wait for the next version: http://mail.gnome.org/archives/gtk-app-devel-list/2000-September/msg00084.html From: Havoc Pennington hp redhat com To: bl tod dk Cc: gtk-app-devel-list gnome org Subject: Re: New version of GTK+ Date: 08 Sep 2000 22:20:28 -0400 Bo Lorentsen [EMAIL PROTECTED] writes: Havoc Pennington wrote: The new text/tree widgets are totally incompatible with the old ones. (But again, the old ones remain in GTK+, so you don't have to port until you want to.) The text widget code is in CVS in mostly final form, and there's a testtext.c to look at. The tree widget is in the 'gtree' module in CVS and is still very much in flux. Does this mean that then next version will have real MVC layout, to make it possible to display lager data lists (alias database's) ? Yes, this feature was the primary motivation for the new widget. (Or at least one of the most important motivations.) Havoc ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: URL of patch for HashStore
On Thu, Sep 28, 2000 at 04:04:54PM -0700, Mark B. Elrod wrote:
so i did some reading up on stl strings and it appears they are copy on write.
I learn something new every day...usually 7 days after it would have
been most useful to learn it.
i do
not know if this is only for instances where you do this:
string foo, bar;
foo = "hello";
bar = foo;
and not for instances like this:
string foo, bar;
foo = hello;
bar = hello;
the results that isaac posted seem to indicate that the second case is copy on write
in which case the char store stuff won't save us any memory h need more
research to confirm/deny this theory.
I have too little time for the next few days to find that.
elrod
Except for the case where the string is empty, in which case I
allocate 1 byte which all the empty strings point to ( + 4 bytes for
each char * in the program of course).
I bet C.O.W. string does not compress duplicates:
string a,b;
char * x,y;
x=strdup("Hello");
y=strdup(x);
a=x;
b=y;
free(x)
free(y);
Since to detect this, b would have to search the actual contents of a.
Initializing a string from a char * has to make a copy. So...damn.
Perhaps HashStore should have used string internally. (Like MetaData
did).
Now the guid is in the HashStore as char* so the guid string key in
m_guidTable must make a full copy of it, where before it coould copy
on write. So if HashTable is used, I *have* to fix m_guidTable to use
char* as its key as well. Eliminating duplicate url paths by
splitting into path and filename is independent of this.
Note : m_guidTable is only used 18 lines of code in musiccatalog.cpp
and nowhere else, so it is much easier to change than MetaData was.
But if you split the filename into pathfilename then PlaylistItem
would need fixing internally.
I suggest either
a) Reverting my patch and forgetting the whole mess.
b) Keeping HashStore with m_guidTable as multimapchar*, string
instead of multimapstring,string
--
C.O.W. strins does not fix Gtk widgets. If the current CList and CTree
are duplicating the passed pointers, then for a full playlist there
are 3 copies of the strings.
Too bad the widgets are not using the std::string class.
---
Better Quesion: Can anything else be done about the RAM use? Or is it
all a bloated std c++ library?
I am not up to speed on loadable modules as they are used here. Do they
contain redundant copies of some freeamp .o files?
What is a good memory profiler to find out what object files are using
more memory (for code or data or whatever it can measure) at run time?
-
Anyway, I have no time to code again on this until Saturday.
If we toss out HashStore I can go back to working on GUI improvements,
and thinking about a search interface.
--
Chris.
___
[EMAIL PROTECTED]
http://www.freeamp.org/mailman/listinfo/freeamp-dev
Freeamp html by Doxygen 1.2.2
The cvsweb is nice, since it is always up to date. (It would be nicer if template brackets were not stripped as bad html) But to help dig through the multitude of code, I built the latest Doxygen and had it try and index and diagram (it makes pictures!) the souce tree. The multi-platform code duplication may have wierd consequences on the namespace. If so, I may have run a copy with win32 and beos directories excluded. This is hot off the presses, and on my personal machine: http://kuklewicz.mit.edu/freeamp/ [ It takes as long a full compile to generate the html docs ] To really appreciate it, use a graphical browser instead of lynx... :) -- Chris ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: Request for testing.
On Wed, Sep 27, 2000 at 11:28:32PM +0300, Valters Vingolds wrote:
Hello Chris,
Wednesday, September 27, 2000, 17:47:04, you wrote:
CK My changes to Metadata to use the HashStore for string storage
CK are far enough along to start thinking about getting someone else
CK to help test it.
(sorry, haven't looked at it or compiled freeamp lately somehow - it's
too much of moving target, i guess...;)
Tell you what - I hear a lot from you how your patch segfaults and
segfaults.
I don't really hear how it saves megabytes and megabytes of memory.
Do you really think introducing the unstability that will have to be
debugged and cleaned up for months is absolutely beneficial?
Elrod Wrote
FreeAmp really does have too large of a memory footprint. It is a shame
that the STL strings are such memory hogs. I think it is worth seeing how
big of an improvement his suggestion can bring. If it is not that much of
an improvement we drop itright? If it is a big improvement we can all
hammer on it and track down any lingering issues...
Considering how late in the game Chris is jumping into the code I am
impressed by how quickly he is modifying the code and getting up to speed.
It is only natural for there to be bugs and issues when trying to make
such a large change. Let's give his idea a chance and as much support as
we can lend and see where we end up.
elrod
These are both good comments. I will clean it up and post the patch
url tonight. [ I don't think anyone over the internet has ever
reviewed my code before. Hmmm scary]
shren [EMAIL PROTECTED] Wrote:
I've mostly ignored the freeamp list, but I'll throw in my two bits.
I would have to see some serious numbers on how much memory the patch
saved. The advantages of using STL strings are very, very numerous. They
are available for almost every platform, they are pretty well tested, and
most programmers know how to work with them. My guess is that changing
string storage methods is not going to make a significant dent in the
footprint, given that the sound buffer for the music is probably at least
a half a meg and will only get bigger as better quality music becomes
common. (At least I'm assuming the buffer size is seconds * bits/second,
it is in my code.)
String are nice but they are not an optimal database. For instance
freeamp uses gdbm for storing the strings on disk because they form a
DB. My music library had 9000 empty string objects allocated before
the HashStore patch. Now all those (char *) point to the same null byte
in RAM.
The metadata strings have numerous empty fields and numerous duplicates.
(If done right, the same Artist and Album string will appear over and over).
There are "get" methods that look like
const string MetaData::Artist() {}
That allow the rest of the program to use strings for local processing
without having strings as the inner implementation. (It used to be
const string when implemented as strings) I emphasise: the rest
of the program can use "string" as local / temprary storage.
In practice, 96% of the rest of the program called Artist().c_str() or
Artist().size() which eliminated the value of using "string"s. So the
numerous advantages of STL strings are missed. I did not cook up a new
string class: I am using char *. Amateur string classes are a bad
idea.
From looking at the code, I can see freeamp is meant to scale up to
large music catalogs. For these, the overhead of strings for each
field is not worth it.
---
My perspective:
When I first grabbed the freeamp tarball I could play music, but it
segfaulted as I played with it, so I kept listening to Xmms.
I did much prefer the music browser. The xmms playlist is too primitive.
So I decided my new hobby was helping fix Freeamp. After a few
patches I finially even tracked down the killer DownloadtoString
segfault that the StreamTimer triggered that was my original problem.
The memory footprint is huge. The string storage is being improved,
slowly, with my curent patch. More can be done later with the url
storage. Even more could be done by replacing GtkLabel with a MVC
label that did not have a private copy of the string. (Perhaps some
gtk+ mailing list atchinve has MVC discussions we could search).
Most of the memory footprint will still remain. The goal should be to
get it down to around 10MB. I have seen the beta eat 40MB. For
instance, memory is eaten by playing with Themes that seems not to be
released (I say "seems" since I have only test that causally on one
occasion). Large themes cost RAM, that is not the issue, but trying 6
themes should not eat 6 x large RAM.
-
My HashStore patch is ...
a) ... easy because only metadata talks to it.
b) ... Hard because C++ memory management is always hard. It only looks
easy when only one strategy is used, and the objects are debugged.
Freeamp talks to too many C libraries to be fully object oriented.
d) ...hard because I
URL of patch for HashStore
The patches can be downloaded from my MIT AFS account : http://web.mit.edu/chrisk/www/ http://web.mit.edu/chrisk/www/freeamp-hashstore-1.cvs.patch.gz http://web.mit.edu/chrisk/www/freeamp-hashstore-1.beta9.patch.gz Or my full tarball (from my personal machine, more space) : http://kuklewicz.mit.edu/ http://kuklewicz.mit.edu/freeamp-hashstore-1.tar.gz freeamp-hashstore-1.cvs.patch.gz is against latest CVS at 8:20PM EST on Sept. 27,2000 (made with diff -uPr -x CVS freeamp freeamp-hashstore | gzip /home/shared/mit/freeamp-hashstore-1.cvs.patch.gz) freeamp-hashstore-1.beta9.patch.gz is against 2.1 beta 9 tar.gz If people need a full ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: Implementing Better String Storage
[ Is the #freeamp hidden? It was not on the list, but I could join
it... ]
Theme for this whole thread of discussion:
[chrisk@kuklewicz ~/www]$ fortune
Nothing endures but change.
-- Heraclitus
An update on my progress,
Yesterday I replaced how metadata is stored internally and much of how
it is accessed of outside (e.g. PeekAlbum() not Album().c_str() and
Album_length() not Album().size() and so on). Before I submit I will
temporarily disable the const string Album() style access methods and
follow the compiler errors to audit how the returned strings are used.
The strings are now *temporary* objects, instead of "const string"
references to MetaData fields.
With help from Isaac I managed to get it compiling and it ran loaded
the playlist just find, but editing a comment segfaulted.
I have it cleaner today, with a slightly more evolved design and it
could edit metadata okay, but opening the streams part of the tree
eventually segfaulted. It seems I did need to be thread safe. I
observed the two [New Thread xxx (runnable)] notices, and the names of
the functions in the back trace (at end of message). So I now use a
Mutex in the HashStore to protect everything. Also, thank you to
whomever already added the AutoMutex class!
I also added a static const char empty[1]=""; to the HashStore for
caching (merge duplicates) of all zero length strings. I also went
back and changes the HashStore methods to use const everywhere again.
Old example output from "cerr *m_store" after it loaded my database
on startup:
HashStore report
Logical Strings held: 2317
Strings in HSet: 1009
Strings in HMap: 146
So very soon I will have a whopper of a patch. Any special handling
requests?
Currently: I did make clean make, and now I sleep.
Most recent crash as seen by gdb, as evidence that thread safety is required:
@ ui/musicbrowser/unix/src/browsertree.cpp : 1370 : void GTKMusicBrowser::StreamTimer()
LWP 15638 exited.
@ ui/musicbrowser/unix/src/browsertree.cpp : 1535 : void
GTKMusicBrowser::FillRelatable(bool = false)
[New Thread 28701 (runnable)]
Delayed SIGSTOP caught for LWP 15639.
@ ui/musicbrowser/unix/src/browsertree.cpp : 1331 : static void
GTKMusicBrowser::stream_timer_func(void *)
@ lib/http/src/Http.cpp : 92 : enum Error Http::DownloadToString(const class string ,
class string )
args: url=http://www.freeamp.org/streams.xml, page=, m_bytesInBuffer=0,
m_bufferSize=0, (void *)m_buffer=(nil)
@ lib/http/src/Http.cpp : 181 : enum Error Http::Download(const class string , bool)
args: url=http://www.freeamp.org/streams.xml, fileDownload=0
@ lib/http/src/Http.cpp : 182 : enum Error Http::Download(const class string , bool)
args: m_bufferSize=0, m_bytesInBuffer=0, (void*)m_buffer=(nil)
@ ui/musicbrowser/unix/src/browsertree.cpp : 1535 : void
GTKMusicBrowser::FillRelatable(bool = false)
[New Thread 29726 (runnable)]
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 29726 (runnable)]
0x80d4043 in hashtablechar *, char *, hashchar *, identitychar *, EqualString,
__default_alloc_templatetrue, 0 ::erase (this=0x813aa50, it=@0xbbfffaa4)
at /usr/include/g++-2/stl_hashtable.h:777
777 node* next = cur-next;
(gdb) bt
#0 0x80d4043 in hashtablechar *, char *, hashchar *, identitychar *,
EqualString, __default_alloc_templatetrue, 0 ::erase (this=0x813aa50,
it=@0xbbfffaa4)
at /usr/include/g++-2/stl_hashtable.h:777
#1 0x80d5ddd in hashtablechar *, char *, hashchar *, identitychar *,
EqualString, __default_alloc_templatetrue, 0 ::erase (this=0x813aa50,
it=@0xbbfffacc)
at /usr/include/g++-2/stl_hashtable.h:829
#2 0x80d5da7 in hash_setchar *, hashchar *, EqualString,
__default_alloc_templatetrue, 0 ::erase (this=0x813aa50, it={cur = 0x81ef448, ht =
0x813aa50})
at /usr/include/g++-2/stl_hash_set.h:160
#3 0x809ecf2 in HashStore::unref (this=0x813aa50,
s=0x81eb4a8 "\030\e\b\036@0's (Rob's Pick) ")
at base/src/charstore.cpp:204
#4 0x80e48fc in MetaData::setentry (dest=@0xbbfffcf4,
src=0x40e3b7c0 "Upbeat 80's (Rob's Pick)") at base/include/metadata.h:166
#5 0x80e4319 in MetaData::SetTitle (this=0xbbfffcec,
title=0x40e3b7c0 "Upbeat 80's (Rob's Pick)") at base/include/metadata.h:83
#6 0x4096f063 in Misc::ReadMetaData (this=0x8148ff0,
url=0x40e3b730 "http://216.32.166.94:8076/\"", metadata=0xbbfffcec)
at plm/metadata/misc/misc.cpp:227
#7 0x80b2721 in PlaylistManager::MetaDataThreadFunction (this=0x81452f8,
list=0x81e0f28) at base/src/playlist.cpp:2472
#8 0x80b292b in PlaylistManager::metadata_thread_function (arg=0x81f34f0)
at base/src/playlist.cpp:2519
---Type return to continue, or q return to quit---
#9 0x80c4b4d in pthreadThread::InternalThreadFunction (this=0x8b93788)
at base/unix/src/pthreadthread.cpp:73
#10 0x80c4b1d in pthreadThread::internalThreadFunction (arg=0x8b93788)
at base/unix/src/pthreadthread.cpp:62
#11 0x40028cd5 in pthread_start_thread (arg=0xbbfffe40) at manager.c:241
#12
Re: Implementing Better String Storage
On Tue, Sep 26, 2000 at 08:13:08AM -0700, Mark B. Elrod wrote: So I am interested in hearing how much memory savings we are seeing from all this. Can you run freeamp on your machine pre-patch and post-patch with the same playlists, etc. and let us know the memory footprints? elrod Patience. The new compile segfaulted before the GUI came up. Once I see it stop segfaulting from my patch I will compile clean cvs and my tree with the same optimizations. Right now I expect only minor memory improvements, with the removal of many string objects with no data being the main benefit. In the future the mapstring,string for GUID-(string URL) for GetFilename will be replaced with GUID-pairchar* Path, char* File mapping to allow refcounting of duplicate paths. Unless your collection of songs is really strangely organized, this will be a benefit. [ If you need to scale to 10^5 + pathname then you ought to check out how the locate command caches the data ] Right now I am using "g++ -O0 -ggdb3" and tarball gdb from cvs snapshot on 9/20/2000. It seems to mostly work, but some things still cannot be examined with "print". The install directory /opt/freeamp-mine is 61 MB. The nirvana of g++ 3.0 is still elusive. -- Chris Kuklewicz ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: Is support for searching a good idea?
On Tue, Sep 26, 2000 at 11:45:12PM -0400, Isaac Richards wrote: Um. That's the desired behaviour, else there's going to be a mem leak. the 'meta' in this instance is a local variable -- why set it to null when the function returns a line later? If you're having problems with this, I'd say it's because you changed the MetaData class so that it needs an explicit copy constructor... Oh...the extra return statements..okay. And of course meta has a new explicit copy constructor and assignment operator. It has to unref the old string (if any) and reference the new string. I had the music browser up and with edit info seeming to work, but now I made a few "improvements" and I have more segfaults in the way to clear out. after loading, it prints out a report: HashStore report Logical Strings held: 11979 Empty String count: 9660 Strings in HSet: 0 Strings in HMap: 1156 Strings marked in KList: 0 So have 9660 string allocation totally avoided. They are all the same char * value. And I have 11979-9660=2319 other references. But the char* are all in HMap, so each had at least 2 ref's at one point. (When added they are in HSet, when ++refcount they move to HMap, but when --refcount they stay in HMap). Freeamp is playing right now...But opening the music browser causes a segfault. Ah..found the cause...compiling... Isaac On 27-Sep-2000 Chris Kuklewicz wrote: First: WARNING the function Error MusicCatalog::AddSong(const char *url) says "delete meta;" inside and after the look... twice...segfault STRONG SUGGESTION : after a call to delete set the pointer to NULL. Only exception is if it is manifestly obvious from looking at the screen that it cannot possibly screw up. (like a two line function). Setting meta=NULL after a delete prevents this problem here. ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: Title scrolling
On Tue, Sep 26, 2000 at 09:18:40PM -0700, gunnm wrote: I have the current stable version of freeamp, 2.0.8, and I think the program rules! No really, it is way better than winamp, I am very impressed. I think you need to improve your "marketing" though, since I just heard about it recently from a slashdot post and would have used it a long time ago if I knew it existed. On to my question. I cannot seem to stop the title from scrolling repeatedly during song playback. Is there a way to stop the scrolling that I haven't figured out? I find it very distracting. If it isn't possible now, would it be difficult to implement? Sounds like a nice feature. I am so twisted up chasing segfaults (that I created) that I forget if the current beta has that switch. ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
More String debugging
On Wed, Sep 27, 2000 at 12:22:06AM -0400, Chris Kuklewicz wrote:
Freeamp is playing right now...But opening the music browser causes a segfault.
Ah..found the cause...compiling...
Ah..the music plays, as before.
Ah...now the music browser comes up without segfaulting.
The tree looks good but the list is not there...just the grey background.
Weird. Add an extra show widget command for kicks. Weird. Add tracing
output to the filling of the list.
I'll ask for a second opinion: when a pointer is passed as the string
data for a column of the clist widget, what memory management is
required? I am assuming it keeps the pointer, and you are not
supposed to alter the string it points to. Metadata strings are all
allocated by HashStore and only removed when the all metadata classes
referring to it call unref.
At the moment:
[ Rest of program ] --- [ MetaData ] --- [ HashStore ]
With every metadata (char *) being allocated and freed by HashStore.
There are other things that HashStore can do, but this is the only
model in use.
Oh...another segfault... This is the end of the unix bootstrap "main" function:
if (!allow_mult) {
if (pCmdLine)
shmdt(pCmdLine);
unsem.val = 0;
semctl (iCmdSem, 0, IPC_RMID, unsem);
shmctl (iCmdMem, IPC_RMID, 0);
}
delete pP;
delete context;
cerr "Normal Exit." endl flush;
return 0;
}
And I saw "Normal Exit" and then the segfault...sigh.
I will add some tracing printouts to some destructors...
Compiling take so looonnng.
___
[EMAIL PROTECTED]
http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: Implementing Better String Storage
On Mon, Sep 25, 2000 at 01:30:32AM -0400, Isaac Richards wrote: On 25-Sep-2000 Chris Kuklewicz wrote: What is the difference between a .o and a .lo object? Nothing, really. The .lo is just an extra extension so that I can compile the few object files that are shared in the main executible and plugins with or without -fPIC as needed. Stuff in the main executible gets .o, and objects that need to be compiled again for a plugin get .lo. Arg. I attached the short error message to this message. You forgot to initialize m_store.. Stick: HashStore *MetaData::m_store = NULL; in player.cpp or something.. Isaac Doh! Thank you very very much. It should go into base/src/metadata.cpp, which already has the initializer for MetaData::empty. I added two static items, but only one initializer! While waiting for that fix, I have created another right click option menu item that I wanted last time I used the browsertree. I'll send in the patch for that when I'm done. It is essentially the same as "add to playlist and play now" but it will not add items that are already in the list. I just ran cvs update on my hacking tree. The make has a small glitch: gcc -I. -I. -I./config -DUNIX_LIBDIR=\"/opt/freeamp-mine//lib\" -Dlinux -I. -I./lib/gdbm -I./base/include -I./config -I./io/include -I./ui/include -I./lmc/include -I./base/unix/include -I./base/unix/linux/include -I./io/esound/include -I./ui/musicbrowser/unix/include -I./ui/freeamp/include -I./ui/freeamp/unix/include -I./ui/download/unix/include -I./ui/musicbrowser/include -I./ftc/kjofol -I./io/alsa/unix/linux/include -I./io/soundcard/unix/linux/include -I./lmc/xingmp3/include -I./lmc/cd/include -I./plm/portable/pmp300/sba -I./lib/xml/include -I./lib/zlib/include -I./lib/unzip/include -I./io/cd/unix/include -I./base/aps -I./io/wavout/include -I./ui/lcd/include -I./ui/irman/include -I./lib/http/include -I./io/signature/include -I./lmc/vorbis/include -Wall -ggdb3 -O0 -fno-inline -fno-inline-functions -I/usr/lib/glib/include -I/usr/X11R6/include -I/usr/lib/glib/include -I/usr/X11R6/include -D_REENTRANT -fPIC -c lmc/vorbis/src/lib/psy.c -o lmc/vorbis/src/lib/psy.o lmc/vorbis/src/lib/psy.c: In function `_vp_psy_init': lmc/vorbis/src/lib/psy.c:175: structure has no member named `ath_att' lmc/vorbis/src/lib/psy.c:272: structure has no member named `toneatt_125Hz' lmc/vorbis/src/lib/psy.c:273: structure has no member named `toneatt_250Hz' lmc/vorbis/src/lib/psy.c:274: structure has no member named `toneatt_500Hz' lmc/vorbis/src/lib/psy.c:275: structure has no member named `toneatt_1000Hz' lmc/vorbis/src/lib/psy.c:276: structure has no member named `toneatt_2000Hz' lmc/vorbis/src/lib/psy.c:277: structure has no member named `toneatt_4000Hz' lmc/vorbis/src/lib/psy.c:278: structure has no member named `toneatt_8000Hz' lmc/vorbis/src/lib/psy.c:280: structure has no member named `noiseatt_125Hz' lmc/vorbis/src/lib/psy.c:281: structure has no member named `noiseatt_250Hz' lmc/vorbis/src/lib/psy.c:282: structure has no member named `noiseatt_500Hz' lmc/vorbis/src/lib/psy.c:283: structure has no member named `noiseatt_1000Hz' lmc/vorbis/src/lib/psy.c:284: structure has no member named `noiseatt_2000Hz' lmc/vorbis/src/lib/psy.c:285: structure has no member named `noiseatt_4000Hz' lmc/vorbis/src/lib/psy.c:286: structure has no member named `noiseatt_8000Hz' lmc/vorbis/src/lib/psy.c:298: structure has no member named `peakatt_125Hz' lmc/vorbis/src/lib/psy.c:299: structure has no member named `peakatt_250Hz' lmc/vorbis/src/lib/psy.c:300: structure has no member named `peakatt_500Hz' lmc/vorbis/src/lib/psy.c:301: structure has no member named `peakatt_1000Hz' lmc/vorbis/src/lib/psy.c:302: structure has no member named `peakatt_2000Hz' lmc/vorbis/src/lib/psy.c:303: structure has no member named `peakatt_4000Hz' lmc/vorbis/src/lib/psy.c:304: structure has no member named `peakatt_8000Hz' lmc/vorbis/src/lib/psy.c: In function `_vp_apply_floor': lmc/vorbis/src/lib/psy.c:623: structure has no member named `noisefit_threshdB' lmc/vorbis/src/lib/psy.c:650: structure has no member named `noisefitp' lmc/vorbis/src/lib/psy.c:651: structure has no member named `noisefit_subblock' lmc/vorbis/src/lib/psy.c:665: structure has no member named `noisefit_subblock' make[1]: *** [lmc/vorbis/src/lib/psy.o] Error 1 When did this happen? Or have I messed up the build tree? Anyway..time to sleep while the compile continues. Again: THANKS FOR THE FIX ! :) -- [chrisk@kuklewicz ~/www]$ fortune Conway's Law: In any organization there will always be one person who knows what is going on. This person must be fired. And that person certainly is not me! ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: Column selection?
On Sun, Sep 24, 2000 at 01:19:05PM +0100, Steve Kemp wrote: On Sat, 23 Sep 2000 [EMAIL PROTECTED] wrote: Once again, its time for a freeamp release. We're now up to Beta 9 and the number of reported bugs is seriously dwindling. We don't have many bugs left to go before we call this 2.1 -- so, if you haven't looked at the 2.1 series, please download it today and check it out. Download page: http://www.freeamp.org/index.html?mode=download Are there any plans to roll in my music browser changes, (which allow column selection, on Win32) to future releases? What is this column selection behavior? I haven't updated this for a while, but I could if there was interest/demand.. (Originally I thought I could duplicate the changes on Linux; but I don't think I'll be able to do that .. I haven't been able to build freeamp on Linux for a _long_ time.) If it sounds useful enough, I might enable it on linux. ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: char* Freestore proposal
On Sat, Sep 23, 2000 at 10:48:25AM -0700, [EMAIL PROTECTED] wrote: On 22 Sep, Chris Kuklewicz wrote: Have a hash_setchar* which holds new refcount 1 strings (and thus has no int overhead), and a hash_mapchar*, unsigned int which holds any refcount string. A duplicate has to be searched for in both the hash_map and hash_set when adding a new string. When ++refcount on char* in the hash_set, move char* to the hash_map with refcount 2. Note: If the stored refcount in hash_map is zero, it means the string is static and need never be freed, and the refcount stays at zero. When --refcount, do not move char* back from hash_map to hash_set, just leave it in hash_set with a refcount of 1. Interesting idea -- do you know how much memory overhead there is for a hash_map? I really like the not duplicating strings -- in the case of the data that we're throwing at it that would be a good way to save memory. Would you be interested in hacking on this? I'm cooking up the storage classes right now. ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: Column selection?
On Sun, Sep 24, 2000 at 10:27:14PM +0100, Steve Kemp wrote: On Sun, 24 Sep 2000, Chris Kuklewicz wrote: What is this column selection behavior? If it sounds useful enough, I might enable it on linux. Basically it allows the selection of the columns which are displayed in the musicbrowser. eg. Genre, Year, etc, and allows the order to be set. Sounds super-customizable. I assume you mean the sort the playlist part of the browser. Hiding columns is easy in gtk. Re-ordering may be trickier. (I think it also adds the ability to sort the lists by clicking on a column header - but this may be the default behaviour). I already know how to implement column sorting in Gtk for linux. I also see that there is a menu chock full of Sort by X items. Theres a couple more, minor, changes that go along with this; such as adding a "Find .." dialog - to allow the browser to highlight the next entry in the playlist which contains the entered text. I really really want to add a great search capability. I would like it to use regex searching. I think I prefer seeing a separate list of all the matches at once. I have not decided on the best interface yet. I think I will hold off until the string storage has been rewritten (the "freestore" proposals). Could the windows version access a regex library? (Is that the GNU version or POSIX or BSD?) Otherwise only the unix port would have regex. This all works by having a new page in the options - originally I wanted to have the user be able to drag the columns to re-arrange them, but I found this tricky to implement in the pure C api that Freeamp uses on Windows. (Normally I'm an MFC/ASM guy ;) http://GNUSoftware.com/ -- GNU Software for Windows Users http://www.steve.org.uk/ -- All about Steve ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Implementing Better String Storage
First: I have written/compiled charstore.h and charstore.cpp which implement the API for storage of reference counted char* string. The overall idea is to reduce storage size even though operations may take longer. There is an abstract interface and two actual classes: AbstractStore which defines the api methods, and documents the semantics. HashStore which uses both a hash_set and a hash_map so it is space and time efficient for large string collections. The hash set of refcount==1 strings is accessed first so it is fairly optimised for strings that are never duplicated (e.g. GUID). StringStore which uses a single map to keep the strings in a sorted order. It is less space efficient than the hash_set in HashStore if there are few or no duplicated strings. Keeping it always sorted is slower than using a hash based container. The advantage is only for items that need to be read out in a sorted order. (e.g. ???) The API works best for dynamically allocated strings, but it is kludged to handle "static" strings that are never freed. The API is not for storing "mutable" strings. If you want to change the title, you have to unref the old title and store the new one. Is this a problem anywhere? Second: Now how to perform the radical surgery? Following the "data path"...the Read/WriteMetadataFromDatabase use the MetaData get/set functions to store the strings. So the MetaData class is the primary target: keep the api but change the storage to the new charstore.h classes. So how does the lightweight metadata class lean about the concrete storage classes? Simple: Declare the storage classes during program initialization and put pointers into the FAContext. Then make a class static variable m_context in MetaData that points to FAContext and set it during program initialization. This also seems to solve telling the windows dll's where the freestore is. So the Meta class has storage for Artist, Album, Title, Comment, Genre, FormatExtension (?), Year, GUID. Do any of those need to be pulled out in sorted order? Artist: we need a sorted artist list for the musicbrowser tree. Title: Is the "all songs" tree list in sorted order Also, what is FormatExtension? At first, I will simply store all of the fields in one huge HashStore. If the hash is reasonably efficient the large size will not affect performance. Note: Splitting the url into pathname and filename for efficiency will not be attemped till much later. I have not studied the m_guidTable closely yet. Third: Thread safety. How is the current sting storage protected? I can easily add a mutex to protect the stores, but I ought to understand the current system. Help? ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: Implementing Better String Storage
On Sun, Sep 24, 2000 at 07:18:12PM -0400, Isaac Richards wrote: On 24-Sep-2000 Chris Kuklewicz wrote: Also, what is FormatExtension? The extension of the file associated w/ the metadata -- I wasn't aware anything used this. So..there are current lots of copies of the string "mp3" or "ogg" being stored? Wow... ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: Implementing Better String Storage
On Sun, Sep 24, 2000 at 06:31:05PM -0400, Chris Kuklewicz wrote:
First:
I have written/compiled charstore.h and charstore.cpp which implement
the API for storage of reference counted char* string. The overall
idea is to reduce storage size even though operations may take longer.
I have not tried compiling everything yet.
I have finished retrofitting metadata.h using an expanded API like:
Error SetTitle(const char* title){
set(m_title,title);
return kError_NoErr; }
Error GetTitle(char* buf, uint32* len) const { return SetBuffer(buf, m_title,
len); }
const char* PeekTitle() const { return m_title; }
const string Title() const { return m_title; }
size_t Title_length() const { return (m_title?strlen(m_title):0); }
The const string return value is now just const string. Beware of the fact this
is now a temporary.
The previous use of Title().size() or Title().length() is now simply
Title_length()
The very common previous Title().c_str() is now PeekTitle()
Alot of search/replace has replaces all uses of c_str() and size() or
length() with the new functions for Artist,Album,etc...
Yes, that took a while.
Now to correct a large number of compile errors
--
Chris Kuklewicz
The party adjourned to a hot tub, yes. Fully clothed, I might add.
-- IBM employee, testifying in California State Supreme Court
___
[EMAIL PROTECTED]
http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: char* Freestore proposal
On Sat, Sep 23, 2000 at 10:48:25AM -0700, [EMAIL PROTECTED] wrote: On 22 Sep, Chris Kuklewicz wrote: Have a hash_setchar* which holds new refcount 1 strings (and thus has no int overhead), and a hash_mapchar*, unsigned int which holds any refcount string. A duplicate has to be searched for in both the hash_map and hash_set when adding a new string. When ++refcount on char* in the hash_set, move char* to the hash_map with refcount 2. Note: If the stored refcount in hash_map is zero, it means the string is static and need never be freed, and the refcount stays at zero. When --refcount, do not move char* back from hash_map to hash_set, just leave it in hash_set with a refcount of 1. Interesting idea -- do you know how much memory overhead there is for a hash_map? I really like the not duplicating strings -- in the case of the data that we're throwing at it that would be a good way to save memory. Would you be interested in hacking on this? If the library hash_map has too much overhead, then someone could replace it with a specialized version. Replacing string with char* saves a good deal of overhead in this scheme. The only outside code change that needs to happen is that when someone creates a metadata/playlist object, the freestore pointer needs to get passed in. Hmmmhere you seem to want the context (which is currently singular?) to not be a guaranteed singleton. But the freestore could be a singleton, why not? Then you could access it without needing to pass its address to MetaData/PlaylistItem every single time. extern freestore get_album_freestore(); // Global function or method of singleton. The only problem is that global functions are not global under windows across DLL boundaries. While DLLs get mapped into the same address space, globals/global functions are not visible unless explicitly exported. We could pass this in as part of the initialization of each of the plugin DLLS, but that's kinda ugly and a pain too. Unless explicitly exported? Sounds like an option. What does the plugin initialization do at the moment? If it passes in any structure (context?), just add a field/method to that structure to let it find the freestore if it needs it. ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
SEGFAULT in StreamTimer
As a hunter stalks his prey, drawing ever nearer, I have managed to located a segfault. The StreamTimer is called every 30 seconds, and after several times being called it creates a segfault. 1) Start freeamp 2) Stop the first song which has started playing 3) Open MyMusic browser 4) Wait for a long timetop shows no memory leak and it does not segfault 5) Click to open the top of the streams tree, which starts the timer 6) Watch a slow memory leak in top. roughly 4 to 40 bytes per minute. 7) Freeamp segfaults In the last 3 runs, StreamTimer was called 6, 5, then 3 time before dying. The offending code is in Error FreeAmpStreams::PCData(string data) where it does not check m_info, which is NULL the "last" it is called, causing the segfault. I have not solved the problem yet, but I am looking at it. suggestionPlease "assert" all the pointers you expect to be valid in any function before using them./suggestion ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Evil in Http.cpp?
Oh this is just evil. Run freeamp, Open MyMusic, Click on triangle to expand streams. Wait for 1-3 sec. Segfault. setting print statements and compiling with -O0 and attaching to a running freeamp with gdb helped locate the segfault. even looking at m_buffer (which was not null) caused a crash, It was a bad memory address. how can this be? It is set only by m_buffer = new unsigned char[m_bufferSize]; When StreamTimer calls DownloadToString in browsertree, the Download call in DownloadToString returns with m_bytesinBuffer zero. how can m_buffer be messed up? perhaps another command is putting bytes into that memory space. So I start looking in Http::Download and I find: const char* kHTTPQuery = "GET %s HTTP/1.1\r\n" "Host: %s\r\n" "Accept: */*\r\n" "User-Agent: FreeAmp/%s\r\n"; // the magic 256 is enough for a time field that // we got from the server char* query = new char[ strlen(kHTTPQuery) + strlen(file) + strlen(localname) + strlen(FREEAMP_VERSION)+ 2]; sprintf(query, kHTTPQuery, file, localname, FREEAMP_VERSION); strcat(query, "\r\n"); It would be better to use snprintf. Also the +2 in the size of the query array size has nothing to do with why query does not overflow. It does not overflow becase the three "%s" substrings are deleted, making room. If the +2 were needed to fit the "\r\n" then the last '\0' byte written by strcat would overflow. So this code works, but it looks like the author's thought process was wrong, and the +2 is very very misleading. Why not just add another \r\n to the kHTTPQuery format string and remove the strcat? Oh..and use snprint. Or at least strncat. And for my sake, put paranoid check before calling memcpy. And check the return values! My Http.cpp is a bit hacked up right now so I can't easily submit a patch for that right now. Right before Download calls WriteToBuffer I see: m_bufferSize=0, m_bytesInBuffer=0, m_buffer=(null) Right after Download calls WriteToBuffer I see: m_bufferSize=1804, m_bytesInBuffer=0, m_buffer= Segmentation Fault I'm going to make clean;make to reset the binaries ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: Evil in Http.cpp? HELP!
I need help diagnosing this segfault. Because I have failed.
It is clockwork reproducible.
Does anyone else (in unix / linux) have the same segfault after
opening the streams tree in the music browser?
I think something is compiling / linking / loading wrong. I notice
that when I touch Http.cpp, that the Http.o and freeamp executable and
plugins/musicbrowser.ui and plugins/freeamp.ui are re-compiled.
When I add output to this function:
int Http::WriteToBuffer(unsigned char *buffer, unsigned int size)
{
cerr "WriteToBuffer " size endl flush;
if (m_buffer == NULL)
{
_you_are_here;
m_bufferSize = kBufferSize;
m_buffer = new unsigned char[m_bufferSize];
}
_show_args1(m_buffer);
...
...
return size;
}
That it does NOT output any message. This function is called from
this sequence in Download (line numbers for me in left column):
483: _show_args1(wcount);
484: _show_args3(m_bufferSize,m_bytesInBuffer,m_buffer);
485: wcount = this-Http::WriteToBuffer((unsigned char *)cp,
486: total - (cp-buffer));
487: _show_args1(wcount);
488: _show_args3(m_bufferSize,m_bytesInBuffer,m_buffer);
This does produce output:
@ lib/http/src/Http.cpp : 483 : enum Error Http::Download(const class string , bool)
args: wcount=0
@ lib/http/src/Http.cpp : 484 : enum Error Http::Download(const class string , bool)
args: m_bufferSize=0, m_bytesInBuffer=0, m_buffer=(null)
@ lib/http/src/Http.cpp : 487 : enum Error Http::Download(const class string , bool)
args: wcount=1804
@ lib/http/src/Http.cpp : 488 : enum Error Http::Download(const class string , bool)
args: m_bufferSize=1804, m_bytesInBuffer=0, m_buffer=
[2]+ Segmentation fault /opt/freeamp-mine/bin/freeamp
So even though the WriteToBuffer does not seems to execute the output
to cerr, it does change m_buffer and return size to wcount. I cannot
trace this in gdb, I set a breakpoint on Http::Download but it passed
right over it.
I am guessing my dificulty may lie in the way plugins are loaded and
symbols are resolved? Is gdb setting a breakpoint in freeamp's version
of Http.o and not seeing the one in musicbrowser.ui?
I still do not understand how musicbrowser.ui writes to cerr in
Download but not in WriteToBuffer.
I am out of ideas.
I am attaching the header file that defines the macros I am using,
for completeness, and my copy of Http.cpp.
I am running RH 6.2/HelixGnome with gcc 2.91.66
The make output from touching Http.cpp is
[1525|ckuklewicz /usr/local/src/freeamp/freeamp-mine]$ make
for p in base base/beos base/beos/src base/src base/unix base/unix/src dlm dlm/rmp io
io/alsa io/alsa/unix io/alsa/unix/linux io/alsa/unix/linux/src io/esound io/esound/src
io/http io/local io/obs io/src io/soundcard io/soundcard/beos io/soundcard/unix
io/soundcard/beos io/soundcard/unix/linux io/soundcard/unix/linux/src
io/soundcard/beos/src io/soundcard/unix/linux/src io/wavout io/wavout/src
io/wavout/include io/signature lib lib/xml lib/xml/src lib/http lib/http/src lib/zlib
lmc lmc/xingmp3 lmc/xingmp3/src lib/gdbm lib/zlib/src lmc/vorbis plm plm/metadata
plm/playlist plm/portable plm/portable/pmp300 lmc/xingmp3/src plm/metadata/id3v1
plm/metadata/id3v2 plm/metadata/misc plm/playlist/m3u plm/portable/pmp300/sba ui
ui/download ui/download/unix ui/freeamp ui/freeamp/beos ui/cmdline
ui/download/unix/src ui/freeamp/beos/src ui/freeamp/tools ui/freeamp/unix ui/irman
ui/lcd ui/musicbrowser ui/freeamp/src ui/freeamp/tools/src ui/freeamp/unix/src
ui/musicbrowser/src ui/musicbrowser/unix ui/mpg123 plm/playlist/pls ui/irman/src
ui/lcd/src ui/musicbrowser/unix/src ui/ncurses update update/unix lmc/cd lmc/cd/src
io/cd io/cd/unix io/cd/unix/src lib/unzip lib/unzip/src ftc ftc/kjofol ftc/winamp
plm/metadata/cddb base/aps lmc/vorbis/src io/signature/src lib/musicbrainz
lib/musicbrainz/expat lib/musicbrainz/expat/xmlparse lib/musicbrainz/expat/xmltok
lib/musicbrainz/osdep lib/musicbrainz/lib; do \
test -d $p || mkdir $p; \
done
c++ -I. -I. -I./config -DUNIX_LIBDIR=\"/opt/freeamp-mine//lib\" -Dlinux -I.
-I./lib/gdbm -I./base/include -I./config -I./io/include -I./ui/include -I./lmc/include
-I./base/unix/include -I./base/unix/linux/include -I./io/esound/include
-I./ui/musicbrowser/unix/include -I./ui/freeamp/include -I./ui/freeamp/unix/include
-I./ui/download/unix/include -I./ui/musicbrowser/include -I./ftc/kjofol
-I./io/soundcard/unix/linux/include -I./lmc/xingmp3/include -I./lmc/cd/include
-I./plm/portable/pmp300/sba -I./lib/xml/include -I./lib/zlib/include
-I./lib/unzip/include -I./io/cd/unix/include -I./base/aps -I./io/wavout/include
-I./ui/lcd/include -I./ui/irman/include -I./lib/http/include -I./io/signature/include
-I./lib/musicbrainz/lib -I./lib/musicbrainz/expat/xmltok
-I./lib/musicbrainz/expat/xmlparse -I./lmc/vorbis/include -Wall -g -O0
-I/usr/lib/glib/include -I/usr/X11R6/include -I/usr/lib/glib/include
-I/usr/X11R6/include -D_REENTRANT
Re: Evil in Http.cpp? HELP!
On Thu, Sep 21, 2000 at 11:41:19AM -0700, [EMAIL PROTECTED] wrote: On 21 Sep, Isaac Richards wrote: Only problem is that namespaces don't work with current versions of g++. Yea. They don't work at all? Where can I find some details on this? Wellit depends on the definition of "current". Look at http://gcc.gnu.org/releases.html Redhat 6.2 is build using the egcs-1.1.2 with gcc version 2.91.66 My C++ code on RH6.2 handles almost all namespace / using commands. Redhat rawhide (and I assume pinstripe, expected to be released as RH7.0 on monday) uses gcc 2.95.2 But...back when I had RH 5.2 the compiler did not handle namespaces. I think RH5.2 had egcs 1.0.3a with gcc 2.90.xx ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
StreamTimer
Once the stream part of the music browser is opened, it downloads and parses the xml file every 30 seconds. Doesn't it just need to hit it once? Also, everytime it reloads, the tree is collapsed, which the user is not expecting. ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Segfaults. Search capability. Memory footprint.
Two down, alot of segfaults to go I have seen a number of additional segfaults, but I have not been able to reliably reproduce one yet, so I am temporarily stalled (that and I had no free time this last weekend). Mostly I am mucking about in the music browser when freeamp dies. Is there any clear documentation anywhere about what is running in what thread? Just a paragraph or so of explanation would do. If one of the segfaults comes from dereference a NULL, then I may just have to add strict checking of pointers in each function to catch it in the act. By the way, are we all creating different debugging macros to help trace the code? If so, couldn't we just check in a version with macros for the strict pointer checks (setup to compile to nothing by default) ? Also, I did not notice any standard for handling errors/exceptions. Is there any? Enough talk, tomorrow I'll hammer on freeamp till I get a lead on one of these crashes. - When I have more than a few songs loaded in the music browser or the playlist, I want to be able to say "Find all the songs matching foo" as a quick navigation system. I actually wrote a simple helper application that does this with xmms. ( see xmms-gtk-playlist at http://web.mit.edu/chrisk/www/index.html ). Is there any plan to add a search feature? If not, then I could design and implement it. - I noticed the memory footprint was about 27MB (which includes shared libs). The footprint of xmms with the same playlist was about 8MB. These are rough numbers, but the difference is large enough to prompt me to ask on the mailing list. Any comments on how possible or desirable a smaller size would be? -- Chris Kuklewicz ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: Segfaults. Search capability. Memory footprint.
Open the music browser, play a few tunes, then select each Theme and return to the Freeamp theme. The theme memory is NOT released: PID USER PRI NI SIZE RSS SHARE STAT LIB %CPU %MEM TIME COMMAND 21310 ckuklewi 0 0 41260 38M 3528 S 0 0.0 50.4 0:05 freeamp 21311 ckuklewi 0 0 41260 38M 3528 S 0 0.0 50.4 0:00 freeamp 21312 ckuklewi 0 0 41260 38M 3528 S 0 0.0 50.4 0:00 freeamp 21313 ckuklewi 0 0 41260 38M 3528 S 0 0.0 50.4 0:00 freeamp 21314 ckuklewi 1 0 41260 38M 3528 S 0 2.8 50.4 0:02 freeamp 21315 ckuklewi 0 0 41260 38M 3528 S 0 0.0 50.4 0:00 freeamp 21320 ckuklewi 0 0 41260 38M 3528 S 0 1.1 50.4 0:21 freeamp On Tue, Sep 19, 2000 at 04:55:05AM -0400, Sean Ward wrote: Yep, freeamp as a whole is a lot more bloated than say, winamp or XMMS. The primary size difference comes from the musicbrowser, because it loads all the metadata entries at startup, so has a memory footprint proportional to the number of tracks in your collection. Additionally, there are a bit too many internal copies which go on, which are part of the general musicbrowser issue ;). In essence, updating this system is one of the top priorities for the next FreeAmp rev (outside of the 2.1 beta series) since it requires a fairly significant rewrite to the entire musiccatalog and browser code. In the interim, just consider the fact that freeamp will load all the metadata for a playlist faster than XMMS, and can recommend playlists for ya, so you get something extra for the memory usage ;) -Sean As always, Rob or Isaac please correct me if I'm in error, especially about error handling ;) ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Problems with ./configure?
While I wait for musicbrainz compile, I will ask about a few peculiar problems I saw last night. I did a make clean, and ./configure again, but this time I explicitly set --enable-esd. But it did not enable esound.pmo in Makefile-plugins. Leaving out --enable-esd fixed it. At the moment, I am preemptively adding code to check all use of pointers in the cpp files for the musicbrowser. Since gdb and the core file are useless to me, I need to catch the SEGFAULT before it dies, so it can tell me what part of what function is about kick the bucket. ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Re: Gdb
On Wed, Sep 20, 2000 at 01:24:49AM -0400, Isaac Richards wrote: see configure --help. basically, there's no --enable-esd, just --disable-esd. configure's silly and thinks they're interchangeable. I see. At the moment, I am preemptively adding code to check all use of pointers in the cpp files for the musicbrowser. Since gdb and the core file are useless to me, I need to catch the SEGFAULT before it dies, so it can tell me what part of what function is about kick the bucket. If you're willing to experiment, try grabbing the latest CVS of gdb. It helps on my machine. Isaac MI may try that. By the way..what version of egcs do you have? I have g++ 2.91 (the RedHat 6.2 standard). I have epkg working now, since the first victim was the musicbrainz library. With that flexibility, I think I can experiment. I found a reproducable segfault :) So I am tracking it down. ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
IMPORTANT: concurrency problem identified (?)
On Fri, Sep 15, 2000 at 01:42:03PM -0400, Isaac Richards wrote: On 14-Sep-2000 Chris Kuklewicz wrote: Ok...The browsermenu.cpp code for delete_sel had a small mistake. It used a forward iterator on the p-mbSelection vector of items, which were being deleted as it worked (apparantly by a callback in another thread in browsertree.cpp in ctree_unselected). Ya. seems gtk's going into two callbacks at once, which it shouldn't be able to do. Not good. I thought that two threads touching the container w/o a mutex looked like a concurrency bug. See the end of this message for an idea of how they are executing sequentially. In a nutshell: the unselection event is fired for the music browser window during the main loop for the confirmation dialog box. (If the message box were "modal" then the other window may not have its event fired. As it is, one has to expect this). So if you select alot of items and use the edit menu to remove them, the iterator "i" would become lost, and could wander past the end of the vector. Then (*i) would be null, which would be derefenced, and thus segfault. Most loops are reverse_iterators, which seems to fix this here. But I do not like this fix here since it seems too fragile. The reverse_iterators are safe enough, just in this case I didn't want to delete things backwards for some reason... The user expects the list to be presented in the order on screen. When I tried reverse iterators, it felt odd to be asked about the songs in the "wrong" order. The better solution is to avoid alot of threads mucking with the list at the same time. I am submitting a patch that creates a duplicate of the mbSelection list (or the m_plSelected list in the other case in delete_sel). The function can then iterate through this list without problems. I applied the section pertinent to this particular case, and left out the rest. If there's still problems in this section of code, I'll get em in, but for now, I don't think the rest of the patches are necessary. Like I said, I did not test the other sections for problems. I just tried to identify where similar code existed. If the concurrecy problem is solved, then none of these should be a problem. If what I explain below is right, then the problem is executing the gtk main loop (for the dialog) during the iteration over the container. This would make most other loops in the code "safe". == EUREKA : I have just had some insight about the gtk loop! I think this is what is happening (Working in my head, not with the code, so details may be slightly off): I always selected four songs to delete, since this created very reproducable behavior. Before the patch: Note: i is an iterator over the vector of selections. (*i) should never be NULL * User selects the songs (vector holds ABCD) and clicks remove in the menu. * In code, get vector0]=song A and pop up confirm dialog (synchronously, of course) * User is in main loop, and clicks Yes to delete song A * Back in code, erase song A from catalog and post the info event * In code, i++, Get vector[1]=song B (still correct song name), pop up dialog * User is in main loop, which causes info event to process. The main window is redrawn without song A and so the selection is updated so the vector is now BCD * User clicks Yes to delete song B * Back in code, erase song B (by name, so it works) from catalog and post info event * In code, i++, Get third song as vector[2]=D (which skips song C) and pop up dialog * User is in main loop, the window redraws and the selection is updated without song B so that vector is now CD. * User clicks Yes to delete song D * In code, erase song D (by name, so it works) from catalog and post info event * In code, i++. Here everything starts to go wrong. The code would put up the dialog a fourth time (so D would be removed from the visible selection so the vector is no C). But when I said to delete it, it would segfault. I traced this at one point to (*i) now being a null pointer and causing a segfault. ___ [EMAIL PROTECTED] http://www.freeamp.org/mailman/listinfo/freeamp-dev
Cosmetic Patch for gtkmusicbrowser.cpp
It looks like a gtk+ bug, but when the pane is turned via the view
menu in the method
void GTKMusicBrowser::ExpandCollapseEvent(void)
and the vertical scroll bar was visible, then the vertical scroll bar
is still drawn, but it now overlaps the left side of the playlist.
Like I said, this looks a gtk+ bug.
Turning off the scroll bar is an easy workaround, and is done in this
small patch.
Index: gtkmusicbrowser.cpp
===
RCS file: /src/repository/freeamp/ui/musicbrowser/unix/src/gtkmusicbrowser.cpp,v
retrieving revision 1.111
diff -u -r1.111 gtkmusicbrowser.cpp
--- gtkmusicbrowser.cpp 2000/09/11 06:39:38 1.111
+++ gtkmusicbrowser.cpp 2000/09/15 21:56:45
@@ -766,6 +766,8 @@
gtk_paned_set_position(GTK_PANED(masterBox), lastPanedPosition);
gtk_paned_set_handle_size(GTK_PANED(masterBox), lastPanedHandle);
}
+gtk_scrolled_window_set_policy(GTK_SCROLLED_WINDOW(musicBrowserWindow),
+ GTK_POLICY_AUTOMATIC, GTK_POLICY_AUTOMATIC);
title += string(" - My Music: ");
GtkWidget *w = gtk_item_factory_get_widget(menuFactory,
"/View/View Playlist Only");
@@ -778,6 +780,8 @@
lastPanedHandle = ((GtkPaned *)masterBox)-handle_size;
gtk_paned_set_position(GTK_PANED(masterBox), 0);
gtk_paned_set_handle_size(GTK_PANED(masterBox), 0);
+gtk_scrolled_window_set_policy(GTK_SCROLLED_WINDOW(musicBrowserWindow),
+ GTK_POLICY_NEVER,GTK_POLICY_NEVER);
title += string(" - Playlist Editor: ");
GtkWidget *w = gtk_item_factory_get_widget(menuFactory,
"/View/View Playlist Only");
PGP signature
Re: Another patch for a unix segfault.
Oooops... I had not noticed the gpg key expiring...sorry. PGP signature
gtkmessagedialog keyboard accelerators
Ok...removing alot of from from MyMusic at once was both:
a) Annoying, I had to click "Yes" for each removal
b) Was quite buggy and then segfault
After the cvs update tonight there is a new snag:
c) Pressing MyMusic hangs freeamp while maxing out CPU usage. (This is
after "rm -rf ~/.freeamp ~/MyMusic"). I erased the install site and
did a fresh make install, with the same problem. Short of checking
with a make clean, this looks like a freshly added bug. The
Options/Download/Files buttons still bring up their dialogs.
For problem a)
The attached patch makes the first letter of all the standard dialog
buttons underlined, and Alt+Letter clicks the button. A button is
also set to capture the escape button. A button is passed to
gtk_widget_grab_focus to set the default button (simply press enter).
If there is an entry box, then it gets the focus instead (untested).
The segfaulting will have to wait until later.
(I expect to have to learn a few data structures to follow that one).
Pop Quiz:
Anyone have an indent mode already setup for freeamp in Emacs?
--
Chris Kuklewicz
Index: ui/musicbrowser/unix/src/gtkmessagedialog.cpp
===
RCS file: /src/repository/freeamp/ui/musicbrowser/unix/src/gtkmessagedialog.cpp,v
retrieving revision 1.6
diff -u -r1.6 gtkmessagedialog.cpp
--- ui/musicbrowser/unix/src/gtkmessagedialog.cpp 2000/08/30 13:45:31 1.6
+++ ui/musicbrowser/unix/src/gtkmessagedialog.cpp 2000/09/11 21:35:31
@@ -23,6 +23,7 @@
#include gtk/gtk.h
#include unistd.h
+#include gdk/gdkkeysyms.h
#include "gtkmessagedialog.h"
@@ -118,6 +119,26 @@
p-SetCheck(bool(i));
}
+/* Keyboard accelerators courtesy of Chris Kuklewicz [EMAIL PROTECTED] */
+static GtkWidget* new_button_with_accel(const char* uline_label, GtkAccelGroup
+*accel_group,
+bool useEscape=false)
+{
+GtkWidget *button;
+GtkWidget *label;
+button = gtk_button_new();
+label = gtk_label_new(NULL);
+gtk_widget_add_accelerator(button, "clicked", accel_group,
+ gtk_label_parse_uline(GTK_LABEL(label), uline_label),
+ GDK_MOD1_MASK,(GtkAccelFlags)0);
+if (useEscape)
+{
+gtk_widget_add_accelerator(button, "clicked", accel_group, GDK_Escape, 0,
+(GtkAccelFlags)0);
+}
+gtk_widget_show(label);
+gtk_container_add(GTK_CONTAINER(button), label);
+return button;
+}
+
MessageDialogReturnEnum GTKMessageDialog::
Show(const string oMessage,
const string oTitle,
@@ -125,6 +146,8 @@
bool inMain)
{
GtkWidget *window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
+GtkAccelGroup *accel_group = gtk_accel_group_new();
+gtk_window_add_accel_group(GTK_WINDOW(window), accel_group);
if (inMain)
gtk_window_set_modal(GTK_WINDOW(window), TRUE);
gtk_signal_connect(GTK_OBJECT(window), "destroy",
@@ -168,72 +191,80 @@
int iRet = 0;
switch (eType) {
case kMessageOk: {
-button = gtk_button_new_with_label("OK");
+button=new_button_with_accel("_Ok",accel_group,true);
gtk_signal_connect(GTK_OBJECT(button), "clicked",
GTK_SIGNAL_FUNC(ok_click), iRet);
gtk_signal_connect_object(GTK_OBJECT(button), "clicked",
- GTK_SIGNAL_FUNC(gtk_widget_destroy),
- GTK_OBJECT(window));
+ GTK_SIGNAL_FUNC(gtk_widget_destroy),
+ GTK_OBJECT(window));
gtk_container_add(GTK_CONTAINER(hbox), button);
gtk_widget_show(button);
+gtk_widget_grab_focus(button);
break; }
case kMessageYesNo: {
-button = gtk_button_new_with_label("Yes");
+button = new_button_with_accel("_Yes",accel_group);
gtk_signal_connect(GTK_OBJECT(button), "clicked",
GTK_SIGNAL_FUNC(yes_click), iRet);
gtk_signal_connect_object(GTK_OBJECT(button), "clicked",
- GTK_SIGNAL_FUNC(gtk_widget_destroy),
- GTK_OBJECT(window));
+ GTK_SIGNAL_FUNC(gtk_widget_destroy),
+ GTK_OBJECT(window));
gtk_container_add(GTK_CONTAINER(hbox), button);
gtk_widget_show(button);
+gtk_widget_grab_focus(button);
-button = gtk_button_new_with_label("No");
+button = new_button_with_accel("_No",accel_group,true);
gtk_signal_connect(GTK_
First segfault patch
I gave up on gdb. I brought over the printf/cerr macros I favor.
Only residual question from last email: what is an efficient
way to upgrade egcs / glibc on linux (e.g. Redhat 6.2) ?
I found the cause of the immedate segfault (open, close, segfault).
The ~Player was calling StopTimer with a null m_cdTimer, and StopTimer
would dereference it and die. A simple patch is attached.
Hmmm...freeamp is not leaving behind a core fileany ideas on that?
--
Chris Kuklewicz
Index: base/src/timer.cpp
===
RCS file: /src/repository/freeamp/base/src/timer.cpp,v
retrieving revision 1.13
diff -u -r1.13 timer.cpp
--- base/src/timer.cpp 2000/08/30 09:20:53 1.13
+++ base/src/timer.cpp 2000/09/10 21:36:38
@@ -29,7 +29,7 @@
#endif
#include "config.h"
-#include "timer.h"
+//#include "timer.h"
#if defined(__linux__) || defined(solaris) || defined(__FreeBSD__)
#include unistd.h
@@ -91,20 +91,25 @@
void TimerManager::StopTimer(TimerRef timer)
{
-timer-ticks = 0;
-timer-duration = 0;
+// CEK : debugging linux segfault when run close immediately
+// CEK : in ~Profile it calls StopTimer(NULL)
+if (NULL!=timer)
+{
+timer-ticks = 0;
+timer-duration = 0;
-vectorTimerRef::iterator i = m_list.begin();
+vectorTimerRef::iterator i = m_list.begin();
-for(; i != m_list.end(); i++)
-{
-if(*i == timer)
+for(; i != m_list.end(); i++)
{
-m_mutex.Acquire();
-m_list.erase(i);
-delete timer;
-m_mutex.Release();
-break;
+if(*i == timer)
+{
+m_mutex.Acquire();
+m_list.erase(i);
+delete timer;
+m_mutex.Release();
+break;
+}
}
}
}
