Skip to content

fix(ytm): Improve history handling #248

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Jan 15, 2025
Merged

fix(ytm): Improve history handling #248

merged 7 commits into from
Jan 15, 2025

Conversation

FoxxMD
Copy link
Owner

@FoxxMD FoxxMD commented Jan 2, 2025 •
edited
Loading

Checklist before requesting a review

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Describe your changes

  • Use diff change type to restrict type of diff results allowed for YTM history changes -- only allow when add/bump diffs are prepend ("new" tracks at top of history list)
  • Store some (4) recent history responses where order changed. Then, on add-prepend diffs check that a previous response does not exactly match. If it does this is probably an indicator a "newer" response actually has outdated data (bug: (YTM) With each scrobble, another song gets scrobbled #227) and we should ignore new tracks
  • Add tests for YTM history processing to test all of the above
  • Use shelf-playlist response instead of flat list when parsing plays so we preserve playlist name
    • Print playlist name to diff debug for more context when troubleshooting logs
  • Detect skipped plays based on reasonable time elapsed since last discovered track (bug: (YTM) Songs skipped 0-9 seconds after their scrobble cause following song to be discarded #226)
    • Newest track is always added as it could be playing now, n+1 tracks are added based on cumulative elapsed - duration
  • Interim, discovered tracks are given play date based on now - cumulative duration of all prior interim tracks

Issue number and link, if applicable

#227
#228
#226

FoxxMD added 4 commits January 2, 2025 18:25
@FoxxMD
feat: Include comment in play diff when present
f320e6c
@FoxxMD
fix(ytm): Use diff type to restrict valid history changes
484fb16 
* Diff type should always be prepend since we are checking for new tracks
  * Warn and log if another type is detected
* Warn if more than one track diffed for discovery
* Refactor and and make YTM history parsing testable

#227
@FoxxMD
feat(source): Ensure delay on close-interval for scrobbling is less t…
d30ee0c 
…han interval
@FoxxMD
fix(ytm): Compare recent changed history to detect outdated responses
dd4b2eb 
Store responses when history changes and then check if YTM returns an outdated response after previously returning new tracks.

#227
@FoxxMD FoxxMD added bug Something isn't working safe to test trusted to build image labels Jan 2, 2025
@FoxxMD FoxxMD self-assigned this Jan 2, 2025
@FoxxMD FoxxMD changed the title feat: Include comment in play diff when present fix(ytm): Improve handling of temporally inconsistent history Jan 2, 2025
@FoxxMD FoxxMD mentioned this pull request Jan 2, 2025
Closed
2 tasks
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 3, 2025

📦 A new release has been made for this pull request.

To play around with this PR, pull an image:

  • foxxmd/multi-scrobbler:pr-248

Images are available for x86_64 and ARM64.

Latest commit: 8b8f32d

@FoxxMD FoxxMD changed the title fix(ytm): Improve handling of temporally inconsistent history fix(ytm): Improve history handling Jan 3, 2025
@FoxxMD FoxxMD added this to the 0.9.0 milestone Jan 14, 2025
@FoxxMD FoxxMD merged commit 1ea1069 into master Jan 15, 2025
4 checks passed
@FoxxMD FoxxMD deleted the GH-227/ytmMusicalChairs branch January 16, 2025 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@FoxxMD FoxxMD

Labels
bug Something isn't working safe to test trusted to build image
Projects
None yet
Milestone
0.9.0
Development

Successfully merging this pull request may close these issues.
1 participant
@FoxxMD