After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 594568 - An unplayed song can never have a Score of 100
An unplayed song can never have a Score of 100
Status: VERIFIED FIXED
Product: banshee
Classification: Other
Component: general
git master
Other Linux
: Normal normal
: 1.x
Assigned To: Alexander Kojevnikov
Banshee Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-09-08 21:44 UTC by Michael Martin-Smucker
Modified: 2009-09-13 15:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
picture, showing a song with several plays, no skips, and a score < 100 (56.97 KB, image/png)
2009-09-08 21:44 UTC, Michael Martin-Smucker
  Details
Fix (740 bytes, patch)
2009-09-10 05:58 UTC, Alexander Kojevnikov
committed Details | Review

Description Michael Martin-Smucker 2009-09-08 21:44:03 UTC
Created attachment 142743 [details]
picture, showing a song with several plays, no skips, and a score < 100

I'm not entirely sure why, but some songs (that were all imported around the
same time) have an incorrect score.  These songs all calculate the score as if
they have one skip, but the Skip Count displays 0 skips.  I'll attach a
picture.

Any ideas what could cause this?  Or even better, any ideas what could fix
this?
Comment 1 David Stone 2009-09-09 23:35:34 UTC
The computation for score is as follows:

Score = (int) Math.Floor ((((double)Score * total_plays) + (percentCompleted * 100)) / (total_plays + 1));

My guess is that one of the 13 times you've played the song, it didn't play through the entire way. Hence the percent completed wasn't 100 and it's introduced a lower score. Or maybe Banshee just doesn't like your taste in music. :-p
Comment 2 Michael Martin-Smucker 2009-09-09 23:48:03 UTC
(In reply to comment #1)
> The computation for score is as follows:
> 
> Score = (int) Math.Floor ((((double)Score * total_plays) + (percentCompleted *
> 100)) / (total_plays + 1));

Meh, I'll try to wrap my brain around that one later.  Too many parens for me to think about right now.
 
> My guess is that one of the 13 times you've played the song, it didn't play
> through the entire way. Hence the percent completed wasn't 100 and it's
> introduced a lower score.

Could be.  What exactly does percentCompleted measure? and is it possible that percentCompleted could be less than 100 without a skip?  In what scenarios would that happen?

> Or maybe Banshee just doesn't like your taste in music. :-p

This makes the most sense; almost all of the songs are by the band Stars!  Not very subtle, Banshee... 

But more seriously, it does seems strange that this happened to a whole pack of songs that I added around the same time.
Comment 3 Alexander Kojevnikov 2009-09-10 00:01:26 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > My guess is that one of the 13 times you've played the song, it didn't play
> > through the entire way. Hence the percent completed wasn't 100 and it's
> > introduced a lower score.
> 
> Could be.  What exactly does percentCompleted measure? and is it possible that
> percentCompleted could be less than 100 without a skip?  In what scenarios
> would that happen?
> 
May be this happens when you quit Banshee while playing a track? Just a guess...
Comment 4 Michael Martin-Smucker 2009-09-10 00:19:04 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > My guess is that one of the 13 times you've played the song, it didn't play
> > > through the entire way. Hence the percent completed wasn't 100 and it's
> > > introduced a lower score.
> > 
> > Could be.  What exactly does percentCompleted measure? and is it possible that
> > percentCompleted could be less than 100 without a skip?  In what scenarios
> > would that happen?
> > 
> May be this happens when you quit Banshee while playing a track? Just a
> guess...

That was what I thought too, but it seems really unlikely that I would have done that for so many songs that I added recently.  Also, I just tried to test this, and it didn't seem to work that way.

...In my continued efforts to figure out this bug, I just made a smart playlist with the criteria score < 100, skip count == 0.  74 songs show up, and I think every one of them was at one point part of a smart playlist (called "Remember!") of songs I added recently that had fewer than 5 plays.  Several weeks ago, I used the Auto DJ patch from Bug 565767 to randomly add songs from "Remember!" to the play queue until all of the songs had at least 5 plays.

That doesn't necessarily explain anything, but perhaps this is somehow related to that patch or the Play Queue in general.
Comment 5 Alexander Kojevnikov 2009-09-10 00:37:00 UTC
(In reply to comment #4)
> Several
> weeks ago, I used the Auto DJ patch from Bug 565767 to randomly add songs from
> "Remember!" to the play queue until all of the songs had at least 5 plays.
> 
> That doesn't necessarily explain anything, but perhaps this is somehow related
> to that patch or the Play Queue in general.

It seems to be related to the issue you noticed in bug 565767, see comments 33-34. Could you try to reproduce it to make sure the score miscalculation is caused by it?
Comment 6 Michael Martin-Smucker 2009-09-10 00:59:17 UTC
Hmm, I can't seem to figure it out.  The fact that Auto DJ updates the Last Played timestamp before the song is actually played doesn't seem to change the score at all.

Any suggestions for possible ways I could reproduce this?  I've tried:

-auto-dj-omatically (fun new word!) adding songs with score==100 to the queu
-removing automatically added songs from the queue
-dragging the seek slider around a bunch - either to restart or finish the track
-randomly hitting the "Refresh" button during all of the above activities

None of this has worked, so if you have any other ideas...
Comment 7 Michael Martin-Smucker 2009-09-10 01:03:56 UTC
Ooh ooh!  Just as soon as I got done posting that useless comment, I think I figured it out!

If an unplayed song is auto-dj-omatically added to the play queue, that song can never reach a score of 100.  I don't know if this is because Last Played info is added to a song that hasn't been played, or if it's something completely different, but I think I can reproduce this 100% of the time.
Comment 8 Alexander Kojevnikov 2009-09-10 05:58:28 UTC
Created attachment 142853 [details] [review]
Fix

Apparently I can reproduce the problem regardless of whether the song was in the play queue or not. All newly imported songs with zero play count, skip count and score, always get score 50 after the first full play.

The patch should fix it, please check.
Comment 9 Michael Martin-Smucker 2009-09-10 12:08:38 UTC
> Apparently I can reproduce the problem regardless of whether the song was in
> the play queue or not. All newly imported songs with zero play count, skip
> count and score, always get score 50 after the first full play.

Yeah, you're right.  I had noticed this, but thought it was intentional (maybe to avoid giving such a high score to a song that you had only heard once?), and I figured it got sorted out after a song's second or third play.  It definitely does not.

> The patch should fix it, please check.

Oh you know I will.
Comment 10 Michael Martin-Smucker 2009-09-10 13:21:42 UTC
So far things are definitely looking better with this patch; unplayed songs don't get cheated in the Score category anymore.  Would it ever be possible for the tracks that were formerly affected by this bug to have correct scores?

Or, stated more specifically... why does the "else" calculation for Score have to take current Score into consideration?  Why wouldn't something like this do the same thing:

   double total_plays = PlayCount + SkipCount;
   if (total_plays <= 0) {
      Score = (int) Math.Floor (percentCompleted * 100);
   } else {
       Score = (int) Math.Floor (PlayCount / (PlayCount + SkipCount));
   }

There's a pretty good chance that I'm not understanding something important, though, like what exactly is percentCompleted?  Also, I'm just curious about the reasoning for the Math.Floor() method.  Why is it so important that the Score never be rounded up?

Maybe this would all make more sense if I weren't such a C# noob :) but I really would appreciate some explanation on this one.
Comment 11 Alexander Kojevnikov 2009-09-10 15:07:17 UTC
(In reply to comment #10)
> So far things are definitely looking better with this patch; unplayed songs
> don't get cheated in the Score category anymore.  Would it ever be possible for
> the tracks that were formerly affected by this bug to have correct scores?

You can fix them with some SQL queries (make the database backup if you do!) but I don't think it's feasible to code a generic solution. All tracks that were first played after the score feature had been added are affected. But we don't have the first played date for the tracks. Also, different users got the score feature on different date.

If you decide to update your library, use this formula to compensate the initial loss of 50 points (but only for the affected tracks):

  new_score = (score * plays + 50) / plays

Alternatively, you can run the query from migration #29 to re-calculate the score based on the play and skip counts for all songs. See BansheeDbFormatMigrator.cs for details.

> Or, stated more specifically... why does the "else" calculation for Score have
> to take current Score into consideration?  Why wouldn't something like this do
> the same thing:
> 
>    double total_plays = PlayCount + SkipCount;
>    if (total_plays <= 0) {
>       Score = (int) Math.Floor (percentCompleted * 100);
>    } else {
>        Score = (int) Math.Floor (PlayCount / (PlayCount + SkipCount));
>    }
> 
> There's a pretty good chance that I'm not understanding something important,
> though, like what exactly is percentCompleted?  

Example: a song has been played twice, once it has been skipped after 80%, second time - after 90%. The total score will be 85.

AFAIR, the song is considered skipped if it has been played for less than 50% of its duration. Thus taking percentCompleted into account is more accurate than using just play and skip counts.

> Also, I'm just curious about
> the reasoning for the Math.Floor() method.  Why is it so important that the
> Score never be rounded up?

Good catch, it looks like Floor() should be replaced with Round()
Comment 12 Michael Martin-Smucker 2009-09-10 15:16:35 UTC
(In reply to comment #11)
> (In reply to comment #10)

> Good catch, it looks like Floor() should be replaced with Round()

Ooh ooh!  If this is indeed the case, can I open a separate bug for it and submit a patch?  I think I just figured out how to create patches with git, and this would just be really exciting for me. :)
Comment 13 Michael Martin-Smucker 2009-09-10 16:29:47 UTC
(In reply to comment #11)
> (In reply to comment #10)
> 
> You can fix them with some SQL queries (make the database backup if you do!)

Oh, I learned my lesson in Bug 593817.  No more screwing with my database without exercising at least a little caution.

> but I don't think it's feasible to code a generic solution. All tracks that
> were first played after the score feature had been added are affected. But we
> don't have the first played date for the tracks. Also, different users got the
> score feature on different date.

If it were possible to calculate the Score  without using the current Score in the equation, a broken score would fix itself as soon as the song is either played or skipped.  I might mess around with this just for fun... stop me if it's impossible and I'm only going to cause myself frustration.

> Example: a song has been played twice, once it has been skipped after 80%,
> second time - after 90%. The total score will be 85.

I had no idea!  Score is a bit more intelligent than I gave it credit for.
Comment 14 Alexander Kojevnikov 2009-09-11 00:56:37 UTC
(In reply to comment #12)
> Ooh ooh!  If this is indeed the case, can I open a separate bug for it and
> submit a patch?  I think I just figured out how to create patches with git, and
> this would just be really exciting for me. :)

I already pushed the fix, congratulations on the first commit ;)
http://git.gnome.org/cgit/banshee/commit/?id=da73b70f8eb21f1036705705cd93c2250ff5260c
Comment 15 Alexander Kojevnikov 2009-09-11 00:58:46 UTC
I committed the patch, the bug should be fixed now.

If you want to update your scores based on the playcounts and skipcounts, run this query on your database:

UPDATE CoreTracks
SET Score = CAST(ROUND(100.00 * PlayCount / (PlayCount + SkipCount)) AS INTEGER)
WHERE PlayCount + SkipCount > 0
Comment 16 Michael Martin-Smucker 2009-09-13 15:52:23 UTC
(In reply to comment #14)

> I already pushed the fix, congratulations on the first commit ;)

Thanks!  You have no idea how special I feel. Also, thanks for:

> UPDATE CoreTracks
> SET Score = CAST(ROUND(100.00 * PlayCount / (PlayCount + SkipCount)) AS
> INTEGER)
> WHERE PlayCount + SkipCount > 0

Worked like a charm.