From: Michael T. <ti...@ti...> - 2007-06-25 19:46:54
|
>> manpage says "If encountered at the beginning of a file, this means >> the file contains something other than an ID3v2 tag before the MPEG >> audio data." > > Ha, I had found that by grepping the source. Who would think to look > in the man page for such informative information? ;) Well, I found it by grepping the source too, but found it in the manpage file in the source :) > I'm not sure why I have files like this, though I seem to have more > than a few of them. I tried recreating this condition, thinking it > was the version of lame I used at the time (3.96.1 for the few I > noticed last night), or maybe the tagger (eyeD3), but I didn't manage > to reproduce it. It might have been an older version of eyeD3 that I > didn't check. > > Anyway, thanks for whipping up a fix so quickly. I had to move the > get_first_header() call up a bit, as it was gtkpod would just hang and > use up 100% cpu when I tried to update. I don't quite know what the > problem was, I guessed it had to do with seeking in the file prior to > that (but that was just a guess). > > Attached is version 4 of the patch, against revision 1589. While I > was editing I noticed some places where the indentation was odd, > probably due to differences in our editor settings for tab widths ans > such. I hope you don't mind that I ran indent on it? Not at all. I knew it would probably be a problem at some point. I was using actual tabs which render in my vim as 4 spaces, and I noticed the existing code uses actual spaces. > As I said in a reply to Jorg's message, I think a branch would be good > to work on this further. If no one objects, I'll create one soon and > check this stuff in. Seems good to me. It'll give us some time to see if there's anything besides lame we can reasonably support and keep us from breaking the trunk with any gui updates. > Do you have any particular ideas for integrating it into the UI? One > idea I have is to add a checkbox in the details window to control the > track->gapless_track_flag setting (and don't automatically set it in > mp3_read_gapless()). > > With any newly added or updated tracks the gapless data will be read > (if possible) and should be available. If it's not, we can then > disable the checkbox in the details window. > > It might even be good to state in the tooltip or somewhere that the > checkbox is only available when gapless data can be read from the file > so that people know why it's disabled for some files. We could also > let them know they need to update current tracks or even try to update > them from the file if the gapless checkbox is toggled (and prompting > with a dialog if reading the gapless data failed?). > > It might take less characters to code this than it has for me to > describe it. :) Is it worth putting the actual gapless values in the details file? It seems as though you wouldn't want to change them by hand, but stuff like the soundcheck value is in there and I'd be scared to death to touch that one :). I like disabling the checkbox if the values don't exist, and I agree that a tooltip explaining it is probably needed. Michael -- Michael Tiffany ti...@ti... http://www.tiffman.com |