[scan] Fixing time_added field for local files when building library

[whatdoineed2do]

I have owntone running on my RPi 4 with music on an NFS exported filesystem (/export/music)

I recently rebuilt a RPi 1and it's own instance of owntone NFS mounting my music. When I rebuilt the music library I see that all albums are shown as added today - this is a little annoying as whilst the albums were known/discovered by this owntone service, the music was (arguable) not added today. This does mean any smart playlists for recent additions are all wrong since EVERYTHING in the library was added today.

This happens because the file scan uses the current time when setting the time_added field. We can make the server observe the file's mtime field which would be more sensible and I've got a working patch.

diff --git a/src/library/filescanner.c b/src/library/filescanner.c
index 67739aed..bac3de78 100644
--- a/src/library/filescanner.c
+++ b/src/library/filescanner.c
@@ -74,6 +74,7 @@
 #define F_SCAN_FAST    (1 << 2)
 #define F_SCAN_MOVED   (1 << 3)
 #define F_SCAN_METARESCAN  (1 << 4)
+#define F_SCAN_TIMEADDED_FILE (1 << 5)
 
 #define F_SCAN_TYPE_FILE         (1 << 0)
 #define F_SCAN_TYPE_PODCAST      (1 << 1)
@@ -580,6 +581,8 @@ process_regular_file(const char *file, struct stat *sb, int type, int flags, int
   mfi.path = strdup(file);
 
   mfi.time_modified = sb->st_mtime;
+  if (flags & F_SCAN_TIMEADDED_FILE)
+    mfi.time_added = sb->st_mtime;
   mfi.file_size = sb->st_size;
 
   snprintf(virtual_path, PATH_MAX, "/file:%s", file);
@@ -896,7 +899,7 @@ process_directory(char *path, int parent_id, int flags)
       else if (!(flags & F_SCAN_FAST))
        {
          if (S_ISREG(sb.st_mode) || S_ISFIFO(sb.st_mode))
-           process_file(resolved_path, &sb, file_type, scan_type, flags, dir_id);
+           process_file(resolved_path, &sb, file_type, scan_type, flags | F_SCAN_TIMEADDED_FILE, dir_id);
          else
            DPRINTF(E_LOG, L_SCAN, "Skipping %s, not a directory, symlink, pipe nor regular file\n", entry);
        }

This patch ONLY takes care of the file scanned (not noticed by inotify) which has its own code path to calling process_file() and the time_added would remain as current time.

Is this something you'd accept for PR?

[ejurgensen]

Looks hacky to me, and I'm not sure I understand why the issue should even be addressed in the scanner? Saving time added to library as time_added and mtime as time_modified seems reasonable to me. Sounds like it is more the view that is causing the issue?

[whatdoineed2do]

sorry its taken a while to get back (year end release scrambles).

You're probably right this could be solved by a different view; when an album object is retreived from the db it only has one time_added but not a time_modified entry. This could be solved easily for sure but doesn't solve my problem - I'm using the built-in web http://owntone.local:3689/#/music/browse to see what new albums have been added to the library. At this point is where I encounter the issue I described above.

The PageBrowse makes a smart expression query to the backend specifically on time_added so to solve my issue I can modify this here but it might be useful to let users see the recent added albums via time_added or time_modified.. with @chme not so active these days, I may just make a local fix to solve my problem.

[Eleison667]

I have a similar use case. Also using NFS mounted share and sometimes my library gets reset. When this happens 'recently added' makes no sense anymore. It would be very nice if for initial scans this can be either configured on date created or date modified of the actual files.

As a work around I now keep making smart playlists to keep track of recently modified. Unfortunately this doesn't support date created either so after fixing some tags in my collection it suddenly seems as if these were just added so it is still not ideal.