Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed some more problems with song statistics #5251

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

twistios
Copy link

@twistios twistios commented Jan 14, 2025

  • Problems:
    • on API 31 and up, Exoplayer already adds MediaMetricsListener (which also triggers our code as callback) (this is the duplicate playback-event problem)
    • the statistics for songs of a duration was using the total playtime instead of the time for the duration
  • disabled adding of PlaybackStatsListener (one of them)
  • now uses new method that can calculate the playtime for the duration

Related reports: #5127, #5172

- Problem: on API 31 and up, Exoplayer already adds `MediaMetricsListener` (which also triggers our code code as callback)
- disabled adding of `PlaybackStatsListener` for Android 12 and up
- Problem: duplicate playback-events (fixed for the future with a previous commit), main problem: was using listening time of the song for all time (instead of selected period)
- fix (here): new method that can collect playtime for the duration.
- added method `songsPlaytimeMostPlayedByPeriod` to database
- added data class `SongPlaytime` for Song with playtime (used by `songsPlaytimeMostPlayedByPeriod`)
@twistios twistios changed the title fixed problem of duplicate playback events for Android 12 and up Fixed some more problems with song statistics Jan 14, 2025
- Root problem seems to be found: `PlaybackStatsListener` was added twice
- disabled the second `PlaybackStatsListener`
- also disabled the second `PlayerServiceModern` listener
- re-enabled the first `PlaybackStatsListener` for all Android versions
@twistios
Copy link
Author

twistios commented Jan 14, 2025

It seems that the main problem was that PlaybackStatsListener was added twice (for the duplicate playback-event problem). So no special care needed for different Android versions.

- added methods `decrementTotalPlayTimeMs`, `deleteDuplicateEvents`, `delete(Event)` to database
- added button in QuickPicsSettings for removing of duplicated events (created by "duplicate playback-events" bug)
- `deleteDuplicateEvents`:
    - finds duplicated entries (they are located next to each)
    - removes them
    - reduces playtime of a song by that of the duplicated event (was also added twice)
    - 100 ms as threshold (more would probably also be possible), but should be enough
@twistios twistios marked this pull request as ready for review January 16, 2025 23:44
@twistios twistios marked this pull request as draft January 16, 2025 23:57
@twistios
Copy link
Author

twistios commented Jan 17, 2025

This code is not ready yet. Maybe the "duplicate event" removing is not yet working correctly. Because I found in a test that there where songs with negative relative playtime (because somehow the totalPlayTimeMs, so how much the song was played, was negative).

Edit: I could confirm that the problem is created by deleteDuplicateEvents.
Edit2: It seems I found the problem. The totalPlayTimeMs was decreased by the wrong value.
Edit3: It seems the problem is harder to solve than expected. There is a song where it still produces a negative playtime. So there is still some issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant