On Sun, Oct 11, 2009 at 2:10 AM, Stephen Boyd <[email protected]> wrote: > I had to read this a few times but now I understand. > > Basically, scrobbler.iterate() is called every 0.5 seconds and > scrobbler.handle_change_status() is called every 1 or 2 seconds. The > math in scrobbler.handle_change_status() is entirely wrong, and is > really calculating the time between scrobbler.iterate() and > scrobbler.handle_change_status() which happens to come out to 0.5 > seconds or less usually. Therefore in the end we're adding 0.5 seconds > per every status change totaling about half the time of the song when > it's done playing. > > Is that any clearer? Well, I thought the original was plenty clear, but I suppose that's just me. Yours is fine too. > Might be better to have > > prevstate = self.prevstatus['state'] if self.prevstatus else 'stop' > > as long as it's just one assignment in the if-else. I originally considered that, but I wasn't sure if it was acceptable or if Sonata's trying to keep compatibility with older versions of Python like some projects. Grepping around I now see there's already a few usages of the if-else expression, so I guess I was being overly cautious. > Does this mean we update the time even when the song is paused? Consider > the case where I change the volume while the song is paused. My > scrobbler time goes up? Is this a bug currently when the song is stopped > and I change the volume? No, look down to where it updates scrob_playing_duration. It's only updated if the previous state isn't 'pause'. It's there to add whatever bit of playtime was left over when it's first paused. Though, looking over it again, it should've been "if state != 'pause' or prevstate != 'pause'" instead, as just checking prevstate will make it not count the first call after it resumes playing. > The rest looks fine, but I'm starting to wonder if we should take a > different approach. > > Maybe it would be easier if we passed main's client (the mpd connection) > to scrobbler's iterate(), and then did everything we needed in > scrobbler.iterate() at half second intervals? We'd basically detect song > changes, state changes, and update our elapsed time counter every call. It might be easier to pass in the client instead of having a bunch of arguments for passing in stuff from it, but there's no need to do it at every iterate() call. It really only needs to run when there's a state transition, so calling it more often doesn't really offer any benefits. > Or we could split up scrobbler.handle_change_status() into functions for > play, stop, etc. and then call those in main.py when we notice those > state changes (lines 1430-1463, etc). This is a good idea. You'd also need to move the code that detects when the same song is played twice in a row out into main.py and then have it call the stop function for the old song and start for the new one, as there'd be no way for it to be detected from within scrobbler.py with that sort of setup. Though with my patch I was really just going for the simplest thing that would work. Most of my library is in FLAC and with the most recent mpd update, mpd only seems to update the status for FLAC files at about ~1.5 second intervals, so it kinda pretty much completely broke last.fm support for me. > Either way, we need to do a few things: > > * Constantly update some clock source (at 0.5 seconds or 1 second or > whatever) while the song is playing. In fact, you might not need to > update the time more than when the song starts and when it's > paused/changed. Less "might" and more "definitely". With the sort of edge-triggered setup like above, this comes naturally anyway. > * When the song stops, we should post the song if it hasn't been posted > already and meets time requirements. Also reset the clock to zero, etc. > > * If the song changes, we should reset our clock and add the new song to > audioscrobbler's cache. Yeah, that's about all.
I might hack on the code a bit, try out the edge-triggered idea, it shouldn't take too much effort to code. Also, apologies to anyone who tries to directly apply the patch in the original post; Gmail saw fit to murder it with word wrap. I'll make sure not to use Gmail's web client for sending patches in the future. _______________________________________________ Sonata-users mailing list [email protected] https://lists.berlios.de/mailman/listinfo/sonata-users
