From 05ac3a065198378653d0b94dc25946c71d4d23a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emysl=20Eric=20Janouch?= Date: Fri, 14 Apr 2023 03:52:01 +0200 Subject: [PATCH] Check filesize when retrieving thumbnails In particular, this handles screenshots from Rigol oscilloscopes, which reuse the same name series with the same file modification time. --- fiv-browser.c | 38 +++++++++++++------- fiv-io.c | 5 ++- fiv-io.h | 1 + fiv-thumbnail.c | 93 +++++++++++++++++++++++++++++++------------------ fiv-thumbnail.h | 4 +-- 5 files changed, 92 insertions(+), 49 deletions(-) diff --git a/fiv-browser.c b/fiv-browser.c index 87488f7..ccc070a 100644 --- a/fiv-browser.c +++ b/fiv-browser.c @@ -105,12 +105,15 @@ struct _FivBrowser { /// The "last modified" timestamp of source images for thumbnails. static cairo_user_data_key_t fiv_browser_key_mtime_msec; +/// The original file size of source images for thumbnails. +static cairo_user_data_key_t fiv_browser_key_filesize; // TODO(p): Include FivIoModelEntry data by reference. struct entry { gchar *uri; ///< GIO URI gchar *target_uri; ///< GIO URI for any target gchar *display_name; ///< Label for the file + guint64 filesize; ///< Filesize in bytes gint64 mtime_msec; ///< Modification time in milliseconds cairo_surface_t *thumbnail; ///< Prescaled thumbnail @@ -513,6 +516,19 @@ entry_system_wide_uri(const Entry *self) return self->uri; } +static void +entry_set_surface_user_data(const Entry *self) +{ + // This choice of mtime favours unnecessary thumbnail reloading + // over retaining stale data (consider both calling functions). + cairo_surface_set_user_data(self->thumbnail, + &fiv_browser_key_mtime_msec, (void *) (intptr_t) self->mtime_msec, + NULL); + cairo_surface_set_user_data(self->thumbnail, + &fiv_browser_key_filesize, (void *) (uintptr_t) self->filesize, + NULL); +} + static void entry_add_thumbnail(gpointer data, gpointer user_data) { @@ -524,23 +540,24 @@ entry_add_thumbnail(gpointer data, gpointer user_data) cairo_surface_t *cached = g_hash_table_lookup(browser->thumbnail_cache, self->uri); if (cached && - (intptr_t) cairo_surface_get_user_data( - cached, &fiv_browser_key_mtime_msec) == self->mtime_msec) { + (intptr_t) cairo_surface_get_user_data(cached, + &fiv_browser_key_mtime_msec) == (intptr_t) self->mtime_msec && + (uintptr_t) cairo_surface_get_user_data(cached, + &fiv_browser_key_filesize) == (uintptr_t) self->filesize) { self->thumbnail = cairo_surface_reference(cached); // TODO(p): If this hit is low-quality, see if a high-quality thumbnail // hasn't been produced without our knowledge (avoid launching a minion // unnecessarily; we might also shift the concern there). } else { cairo_surface_t *found = fiv_thumbnail_lookup( - entry_system_wide_uri(self), self->mtime_msec, browser->item_size); + entry_system_wide_uri(self), self->mtime_msec, self->filesize, + browser->item_size); self->thumbnail = rescale_thumbnail(found, browser->item_height); } if (self->thumbnail) { - // This choice of mtime favours unnecessary thumbnail reloading. - cairo_surface_set_user_data(self->thumbnail, - &fiv_browser_key_mtime_msec, (void *) (intptr_t) self->mtime_msec, - NULL); + // Yes, this is a pointless action in case it's been found in the cache. + entry_set_surface_user_data(self); return; } @@ -662,11 +679,7 @@ thumbnailer_reprocess_entry(FivBrowser *self, GBytes *output, Entry *entry) g_queue_push_tail(&self->thumbnailers_queue, entry); } - // This choice of mtime favours unnecessary thumbnail reloading - // over retaining stale data. - cairo_surface_set_user_data(entry->thumbnail, - &fiv_browser_key_mtime_msec, (void *) (intptr_t) entry->mtime_msec, - NULL); + entry_set_surface_user_data(entry); g_hash_table_insert(self->thumbnail_cache, g_strdup(entry->uri), cairo_surface_reference(entry->thumbnail)); } @@ -1823,6 +1836,7 @@ on_model_files_changed(FivIoModel *model, FivBrowser *self) .uri = g_strdup(files[i].uri), .target_uri = g_strdup(files[i].target_uri), .display_name = g_strdup(files[i].display_name), + .filesize = files[i].filesize, .mtime_msec = files[i].mtime_msec}; g_array_append_val(self->entries, e); } diff --git a/fiv-io.c b/fiv-io.c index f43998e..87a4e01 100644 --- a/fiv-io.c +++ b/fiv-io.c @@ -3172,6 +3172,7 @@ model_reload_to(FivIoModel *self, GFile *directory, GFileEnumerator *enumerator = g_file_enumerate_children(directory, G_FILE_ATTRIBUTE_STANDARD_TYPE "," G_FILE_ATTRIBUTE_STANDARD_NAME "," + G_FILE_ATTRIBUTE_STANDARD_SIZE "," G_FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME "," G_FILE_ATTRIBUTE_STANDARD_TARGET_URI "," G_FILE_ATTRIBUTE_STANDARD_IS_HIDDEN "," @@ -3208,7 +3209,9 @@ model_reload_to(FivIoModel *self, GFile *directory, FivIoModelEntry entry = {.uri = g_file_get_uri(child), .target_uri = g_strdup(g_file_info_get_attribute_string( info, G_FILE_ATTRIBUTE_STANDARD_TARGET_URI)), - .display_name = g_strdup(g_file_info_get_display_name(info))}; + .display_name = g_strdup(g_file_info_get_display_name(info)), + .filesize = (guint64) g_file_info_get_size(info)}; + GDateTime *mtime = g_file_info_get_modification_date_time(info); if (mtime) { entry.mtime_msec = g_date_time_to_unix(mtime) * 1000 + diff --git a/fiv-io.h b/fiv-io.h index 1c3500e..71917da 100644 --- a/fiv-io.h +++ b/fiv-io.h @@ -144,6 +144,7 @@ typedef struct { gchar *target_uri; ///< GIO URI for any target gchar *display_name; ///< Label for the file gchar *collate_key; ///< Collate key for the filename + guint64 filesize; ///< Filesize in bytes gint64 mtime_msec; ///< Modification time in milliseconds } FivIoModelEntry; diff --git a/fiv-thumbnail.c b/fiv-thumbnail.c index 1f6897f..dacbf22 100644 --- a/fiv-thumbnail.c +++ b/fiv-thumbnail.c @@ -524,7 +524,7 @@ fiv_thumbnail_produce(GFile *target, FivThumbnailSize max_size, GError **error) g_string_append_printf( thum, "%s%c%ld%c", THUMB_MTIME, 0, (long) st.st_mtime, 0); g_string_append_printf( - thum, "%s%c%ld%c", THUMB_SIZE, 0, (long) filesize, 0); + thum, "%s%c%llu%c", THUMB_SIZE, 0, (unsigned long long) filesize, 0); if (cairo_surface_get_type(surface) == CAIRO_SURFACE_TYPE_IMAGE) { g_string_append_printf(thum, "%s%c%d%c", THUMB_IMAGE_WIDTH, 0, @@ -563,9 +563,16 @@ fiv_thumbnail_produce(GFile *target, FivThumbnailSize max_size, GError **error) return max_size_surface; } +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + +typedef struct { + const char *uri; ///< Target URI + time_t mtime; ///< File modification time + guint64 size; ///< File size +} Stat; + static bool -check_wide_thumbnail_texts(GBytes *thum, const char *target, time_t mtime, - bool *sRGB) +check_wide_thumbnail_texts(GBytes *thum, const Stat *st, bool *sRGB) { gsize len = 0; const gchar *s = g_bytes_get_data(thum, &len), *end = s + len; @@ -579,11 +586,14 @@ check_wide_thumbnail_texts(GBytes *thum, const char *target, time_t mtime, continue; } else if (!strcmp(key, THUMB_URI)) { have_uri = true; - if (strcmp(target, s)) + if (strcmp(st->uri, s)) return false; } else if (!strcmp(key, THUMB_MTIME)) { have_mtime = true; - if (atol(s) != mtime) + if (atol(s) != st->mtime) + return false; + } else if (!strcmp(key, THUMB_SIZE)) { + if (strtoull(s, NULL, 10) != st->size) return false; } else if (!strcmp(key, THUMB_COLORSPACE)) *sRGB = !strcmp(s, THUMB_COLORSPACE_SRGB); @@ -594,8 +604,7 @@ check_wide_thumbnail_texts(GBytes *thum, const char *target, time_t mtime, } static cairo_surface_t * -read_wide_thumbnail( - const char *path, const char *uri, time_t mtime, GError **error) +read_wide_thumbnail(const char *path, const Stat *st, GError **error) { gchar *thumbnail_uri = g_filename_to_uri(path, NULL, error); if (!thumbnail_uri) @@ -612,7 +621,7 @@ read_wide_thumbnail( if (!thum) { g_clear_error(error); set_error(error, "not a thumbnail"); - } else if (!check_wide_thumbnail_texts(thum, uri, mtime, &sRGB)) { + } else if (!check_wide_thumbnail_texts(thum, st, &sRGB)) { g_clear_error(error); set_error(error, "mismatch"); } else { @@ -629,11 +638,8 @@ read_wide_thumbnail( return NULL; } -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - static cairo_surface_t * -read_png_thumbnail( - const char *path, const char *uri, time_t mtime, GError **error) +read_png_thumbnail(const char *path, const Stat *st, GError **error) { cairo_surface_t *surface = fiv_io_open_png_thumbnail(path, error); if (!surface) @@ -650,18 +656,27 @@ read_png_thumbnail( // but those aren't interesting currently (would be for fast previews). const char *text_uri = g_hash_table_lookup(texts, THUMB_URI); const char *text_mtime = g_hash_table_lookup(texts, THUMB_MTIME); - if (!text_uri || strcmp(text_uri, uri) || - !text_mtime || atol(text_mtime) != mtime) { + const char *text_size = g_hash_table_lookup(texts, THUMB_SIZE); + if (!text_uri || strcmp(text_uri, st->uri) || + !text_mtime || atol(text_mtime) != st->mtime) { set_error(error, "mismatch or not a thumbnail"); cairo_surface_destroy(surface); return NULL; } + if (text_size && strtoull(text_size, NULL, 10) != st->size) { + set_error(error, "file size mismatch"); + cairo_surface_destroy(surface); + return NULL; + } return surface; } +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + cairo_surface_t * -fiv_thumbnail_lookup(const char *uri, gint64 mtime_msec, FivThumbnailSize size) +fiv_thumbnail_lookup(const char *uri, gint64 mtime_msec, guint64 filesize, + FivThumbnailSize size) { g_return_val_if_fail(size >= FIV_THUMBNAIL_SIZE_MIN && size <= FIV_THUMBNAIL_SIZE_MAX, NULL); @@ -673,6 +688,7 @@ fiv_thumbnail_lookup(const char *uri, gint64 mtime_msec, FivThumbnailSize size) gchar *sum = g_compute_checksum_for_string(G_CHECKSUM_MD5, uri, -1); gchar *thumbnails_dir = fiv_thumbnail_get_root(); + const Stat st = {.uri = uri, .mtime = mtime_msec / 1000, .size = filesize}; // The lookup sequence is: nominal..max, then mirroring back to ..min. cairo_surface_t *result = NULL; @@ -685,7 +701,7 @@ fiv_thumbnail_lookup(const char *uri, gint64 mtime_msec, FivThumbnailSize size) const char *name = fiv_thumbnail_sizes[use].thumbnail_spec_name; gchar *wide = g_strconcat(thumbnails_dir, G_DIR_SEPARATOR_S "wide-", name, G_DIR_SEPARATOR_S, sum, ".webp", NULL); - result = read_wide_thumbnail(wide, uri, mtime_msec / 1000, &error); + result = read_wide_thumbnail(wide, &st, &error); if (error) { g_debug("%s: %s", wide, error->message); g_clear_error(&error); @@ -701,7 +717,7 @@ fiv_thumbnail_lookup(const char *uri, gint64 mtime_msec, FivThumbnailSize size) gchar *path = g_strconcat(thumbnails_dir, G_DIR_SEPARATOR_S, name, G_DIR_SEPARATOR_S, sum, ".png", NULL); - result = read_png_thumbnail(path, uri, mtime_msec / 1000, &error); + result = read_png_thumbnail(path, &st, &error); if (error) { g_debug("%s: %s", path, error->message); g_clear_error(&error); @@ -734,7 +750,7 @@ print_error(GFile *file, GError *error) } static gchar * -identify_wide_thumbnail(GMappedFile *mf, time_t *mtime, GError **error) +identify_wide_thumbnail(GMappedFile *mf, Stat *st, GError **error) { WebPDemuxer *demux = WebPDemux(&(WebPData) { .bytes = (const uint8_t *) g_mapped_file_get_contents(mf), @@ -760,7 +776,9 @@ identify_wide_thumbnail(GMappedFile *mf, time_t *mtime, GError **error) if (!strcmp(key, THUMB_URI) && !uri) uri = g_strdup(p); if (!strcmp(key, THUMB_MTIME)) - *mtime = atol(p); + st->mtime = atol(p); + if (!strcmp(key, THUMB_SIZE)) + st->size = strtoull(p, NULL, 10); key = NULL; } else { key = p; @@ -778,16 +796,17 @@ static void check_wide_thumbnail(GFile *thumbnail, GError **error) { // Not all errors are enough of a reason for us to delete something. - GError *tolerable = NULL; + GError *tolerable_error = NULL; const char *path = g_file_peek_path(thumbnail); - GMappedFile *mf = g_mapped_file_new(path, FALSE, &tolerable); + GMappedFile *mf = g_mapped_file_new(path, FALSE, &tolerable_error); if (!mf) { - print_error(thumbnail, tolerable); + print_error(thumbnail, tolerable_error); return; } - time_t target_mtime = 0; - gchar *target_uri = identify_wide_thumbnail(mf, &target_mtime, error); + // Note that we could enforce the presence of the size field in our spec. + Stat target_st = {.uri = NULL, .mtime = 0, .size = G_MAXUINT64}; + gchar *target_uri = identify_wide_thumbnail(mf, &target_st, error); g_mapped_file_unref(mf); if (!target_uri) return; @@ -809,26 +828,32 @@ check_wide_thumbnail(GFile *thumbnail, GError **error) GFile *target = g_file_new_for_uri(target_uri); g_free(target_uri); GFileInfo *info = g_file_query_info(target, - G_FILE_ATTRIBUTE_STANDARD_NAME "," G_FILE_ATTRIBUTE_TIME_MODIFIED, - G_FILE_QUERY_INFO_NONE, NULL, &tolerable); + G_FILE_ATTRIBUTE_STANDARD_NAME "," + G_FILE_ATTRIBUTE_STANDARD_SIZE "," + G_FILE_ATTRIBUTE_TIME_MODIFIED, + G_FILE_QUERY_INFO_NONE, NULL, &tolerable_error); g_object_unref(target); - if (g_error_matches(tolerable, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) { - g_propagate_error(error, tolerable); + if (g_error_matches(tolerable_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) { + g_propagate_error(error, tolerable_error); return; - } else if (tolerable) { - print_error(thumbnail, tolerable); + } else if (tolerable_error) { + print_error(thumbnail, tolerable_error); return; } + guint64 filesize = g_file_info_get_size(info); GDateTime *mdatetime = g_file_info_get_modification_date_time(info); g_object_unref(info); if (!mdatetime) { - set_error(&tolerable, "cannot retrieve file modification time"); - print_error(thumbnail, tolerable); + set_error(&tolerable_error, "cannot retrieve file modification time"); + print_error(thumbnail, tolerable_error); return; } - if (g_date_time_to_unix(mdatetime) != target_mtime) - set_error(error, "mtime mismatch"); + if (g_date_time_to_unix(mdatetime) != target_st.mtime) + set_error(error, "modification time mismatch"); + else if (target_st.size != G_MAXUINT64 && filesize != target_st.size) + set_error(error, "file size mismatch"); + g_date_time_unref(mdatetime); } diff --git a/fiv-thumbnail.h b/fiv-thumbnail.h index de5be51..05c3dc1 100644 --- a/fiv-thumbnail.h +++ b/fiv-thumbnail.h @@ -68,8 +68,8 @@ cairo_surface_t *fiv_thumbnail_produce_for_search( /// Retrieves a thumbnail of the most appropriate quality and resolution /// for the target file. -cairo_surface_t *fiv_thumbnail_lookup( - const char *uri, gint64 mtime_msec, FivThumbnailSize size); +cairo_surface_t *fiv_thumbnail_lookup(const char *uri, + gint64 mtime_msec, guint64 filesize, FivThumbnailSize size); /// Invalidate the wide thumbnail cache. May write to standard streams. void fiv_thumbnail_invalidate(void);