Bug 433509

Yeah. I'd even say 90% or more.

Play count increment seems to be triggered when ElisaApplication::initializePlayer() which calls &DatabaseInterface::trackHasStartedPlaying

In fact, this seems to simply be a bug: the play count *only* increases when a track is skipped, but not when it goes to completion. I suspect a conditional statement in the code is simply inverted somewhere.

(In reply to Nate Graham from comment #3)
> In fact, this seems to simply be a bug: the play count *only* increases when
> a track is skipped, but not when it goes to completion. I suspect a
> conditional statement in the code is simply inverted somewhere.

In my testing it increments the counter when it starts playing a song, like david says. You don't have to skip it.

For testing I put a few song in the queue. I play the first song and put it on pause. The second song has 6 plays. Then I skip over to the second song. It still has 6 plays. Skipping to the third song doesn't increment the play count. But when I press play when the second song is selected, the play count immediately increases to 7.

The better behaviour would be to only increment the counter when finishing a song. Then you can be pretty sure the user has listened to the entire song. More than 90% played is also possible, but then you have to run some code every time the timestamp changes, so I suggest going for the first one (incrementing when finishing the song).

Indeed, all the action takes place in updateTrackStatistics(), which is only ever called in DatabaseInterface::trackHasStartedPlaying(). Let me see if I can fix it.

(In reply to Nate Graham from comment #5)
> Indeed, all the action takes place in updateTrackStatistics(), which is only
> ever called in DatabaseInterface::trackHasStartedPlaying(). Let me see if I
> can fix it.

Actually, I was trying to fix it. I just need a little bit of guidance. What function should the update be called from instead?

Oh nice, I'll let you take over.

In manageaudioplayer.h, we'll want a new signal finishedPlayingTrack(). That should get emitted in somewhere in ManageAudioPlayer::setPlayerPlaybackState() and possible also in ManageAudioPlayer::setCurrentTrack() if mSkippingCurrentTrack is false.

Then in elisaapplication.cpp, we'll want to connect ManageAudioPlayer::finishedPlayingTrack to a new function in databaseinterface.cpp called DatabaseInterface::trackHasFinishedPlaying(). That new function should be responsible for the logic that has "`PlayCounter` = `PlayCounter` + 1 " in it.

Or something like that. :)

(In reply to Nate Graham from comment #7)
> Oh nice, I'll let you take over.
> 
> In manageaudioplayer.h, we'll want a new signal finishedPlayingTrack(). That
> should get emitted in somewhere in
> ManageAudioPlayer::setPlayerPlaybackState() and possible also in
> ManageAudioPlayer::setCurrentTrack() if mSkippingCurrentTrack is false.
> 
> Then in elisaapplication.cpp, we'll want to connect
> ManageAudioPlayer::finishedPlayingTrack to a new function in
> databaseinterface.cpp called DatabaseInterface::trackHasFinishedPlaying().
> That new function should be responsible for the logic that has
> "`PlayCounter` = `PlayCounter` + 1 " in it.
> 
> Or something like that. :)

Thanks for the guidance, that was very helpful! I have posted a MR: https://invent.kde.org/multimedia/elisa/-/merge_requests/404 (🤖 ⚠️merge request not found)

Git commit cd1c2f1afa96b05766cbea6fb5c9c49ef147f898 by Nate Graham, on behalf of Friso Smit.
Committed on 31/01/2023 at 20:53.
Pushed by ngraham into branch 'master'.

Change track stats only when finishing song
FIXED-IN: 23.04

M  +26   -15   autotests/databaseinterfacetest.cpp
M  +77   -22   src/databaseinterface.cpp
M  +5    -1    src/databaseinterface.h
M  +2    -0    src/elisaapplication.cpp
M  +3    -0    src/manageaudioplayer.cpp
M  +2    -0    src/manageaudioplayer.h

https://invent.kde.org/multimedia/elisa/commit/cd1c2f1afa96b05766cbea6fb5c9c49ef147f898