Monday, July 10, 2006

Finding Bugs is Hard

Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -- Brian Kernighan

With thorough abuse comes success. After finding and fixing one deadlock condition I was able to find a second: start a song playing, press stop, then pause. After you've done this, you can't play songs any more. Easy enough to fix once it's reproducible.

I've done some more work on libmdnsd also. Initially all I wanted to do was make a shared object (the Unix version of a dll) out of it, but I found that some of its output was annoying me. For some reason the original mdnsd code would return network addresses and ports in the processor's native format (typically little-endian). I'm not sure why this was done as it makes displaying an IP address into a programming exercise (albeit a simple one) rather than a function call (inet_ntoa - found in any decent libc implementation). The other thing is that this makes results returned by the original mdnsd different from most other name resolution services, so I decided to fix it. IP addresses are returned in a proper structure and ports are big-endian. Other data returned should probably also be big-endian, but I'll deal with that when I (or someone) actually tries to use them.

"Fixing" libmdnsd naturally required changes to be made to wmamp, mostly for the better I think. Server information is stored a little more compactly (though I think it could still be improved - it's not just every cycle that counts - every byte does too) and on the whole I think it is a little more readable. So lots of changes under the hood there, but no real change in functionality. I didn't break anything (for long) so I can't complain.

I've been meaning to get playlists working, so I took a stab at it last night and I'm most of the way there. I can query the server for all playlists, and I think I can query the songs in the playlist, but I haven't been able to try it yet because I've run into an interface problem. wmamp's interface was originally strictly hierarchical: select a server, an artist, an album, then a song. I've since changed the interface a little so that the process can be short-circuited: you can select the server in such a way that it immediately shows all songs on the server, or you can select an artist such that it shows all their songs, irrespective of the album the belong to (if any). This was, IMHO, better than the previous mechanism - it let me listen to a variety of songs and also leave the player running for an extended period of time without driving me nuts.

My first thought was to show both artists and playlists after a server was selected, and while this is possible wmamp's penchant for sorting will cause artists and playlists to be intermixed and it's not obvious which is which and I feel that is bad. The other thing I want to keep in mind is that some work has already been done to provide a web interface to wmamp with an eye towards controlling music selection when no television is connected.

Implementing playlist support has been a very valuable experience though. I had suspected that memory was still leaking somewhere, and it was, but not at all where I had expected. Entries are sorted by inserting them into a binary tree. It turns out that the code for sorting made two mistakes - it would not always detect duplicate entries (which should not be added to the tree) and when it did detect a duplicate (rarely) it had no way of communicating to its caller that it had not been added to the tree and needed to be free()'d (as data being added to the tree was generally malloc()'d). Both of these have been corrected, and if there is a memory leak now it is much more subtle than what I was seeing previously. I have another suspicion that I haven't had time to check out yet: when any DAAP request is processed it's added into a tree (as above) which sorts the entries based on some simple key. For display, the contents of this tree are moved into a different tree, also keyed, but the key may be more elaborate. If the keys are identical (as in most cases) then the second tree actually becomes a simple linked-list and and not a tree at all. If the additional sorting is not important then it may be faster and more memory efficient to count the items in the tree and malloc() an array to hold the listview items.

Oh, and a note to other wmamp hackers who want to keep the code small: libhttpd appears to be compiled by default with -g (add debugging information) and -O (some optimisation). Unless you are trying to debug this code, rebuild it without -g and use -Os instead of -O. The resulting code is much smaller, and probably faster too. I think this knocked another 10-12kB off the size of wmamp. As I recall, my current builds of wmamp are now around 64kB.

Labels: