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

Add "added" timestamp to song database #1875

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

jcorporation
Copy link
Member

addedtime is only set to mtime, if a new song is added to the database.

Code for proxy plugin is missing, this needs a patch for libmpdclient.

closes #378

@jcorporation jcorporation force-pushed the added_time branch 2 times, most recently from 9ae5129 to c800010 Compare October 5, 2023 17:44
@jcorporation
Copy link
Member Author

jcorporation commented Oct 8, 2023

Should addedtime fallback to modifiedsince for things like sorting? Implemented

@jcorporation jcorporation force-pushed the added_time branch 3 times, most recently from 0c7bc17 to 6162fc0 Compare October 9, 2023 20:20
@MaxKellermann
Copy link
Member

I don't quite get it. Why is it initialized with mtime? If it's the mtime, what's the point of having a separate field, because we already have mtime?

@jcorporation
Copy link
Member Author

i initiailize it with mtime and leave it untouched in further database updates. Idea is to have a timestamp that does not change after a song was added to the database.

mtime changes on each change if the file, even only a tag is corrected. With addtime there is a timestamp a mpd client can use to sort by date added and not only last modified.

@MaxKellermann
Copy link
Member

Do people really modify the song files, and is that important enough to add global unavoidable overhead to the protocol, MPD's memory consumption and to the database?
At best, this feature is misnamed/misdocumented, because it's not when this song was added to MPD, but the last time is was modified before it was added.

@jcorporation
Copy link
Member Author

Yes, I change tags regularly, if I see that a file was tagged falsely. Typo in a tag etc.

Without the added attribute you can not get a list of newly added songs, only a list of last modified songs.

We can initialize the added with the current timestamp, then the behavior is as documented.

@MaxKellermann
Copy link
Member

We shouldn't design a feature to match documentation, we should rather decide which value is more useful and then fix both code and documentation.

@jcorporation
Copy link
Member Author

jcorporation commented Oct 15, 2023

I am unsure what of both options is the better one. I can perfectly life with both.

I think the real added date to the mpd database is the most useful one. Would you principally merge this pull request or do you mean this is not a useful feature?

I will use this feature in my mpdclient myMPD and issue #378 requests this feature also.

Edit: added is now initialized with the current timestamp.

- added is set to current time, if a new song is added to the database.
- GetAdded falls back to mtime.

Code for proxy plugin is missing, this needs a patch for libmpdclient.

closes MusicPlayerDaemon#378
@MaxKellermann MaxKellermann merged commit ac25f34 into MusicPlayerDaemon:master Nov 2, 2023
3 checks passed
@naglis naglis mentioned this pull request Mar 2, 2024
@jcorporation jcorporation deleted the added_time branch May 12, 2024 20:12
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.

[database] add "date added" attribute for songs
3 participants