From 495ad417412a23cc972f8adc9ef2e36370589de8 Mon Sep 17 00:00:00 2001 From: jdgleaver Date: Fri, 10 Jan 2020 11:08:36 +0000 Subject: [PATCH] Thumbnail downloader clean-ups --- intl/msg_hash_lbl.h | 2 + menu/cbs/menu_cbs_ok.c | 14 --- menu/menu_displaylist.c | 2 +- menu/menu_thumbnail_path.c | 3 + msg_hash.h | 1 + tasks/task_pl_thumbnail_download.c | 159 ++++++++++++++++++++++++----- 6 files changed, 141 insertions(+), 40 deletions(-) diff --git a/intl/msg_hash_lbl.h b/intl/msg_hash_lbl.h index f74217ce02..6928a9ea92 100644 --- a/intl/msg_hash_lbl.h +++ b/intl/msg_hash_lbl.h @@ -1307,6 +1307,8 @@ MSG_HASH(MENU_ENUM_LABEL_THUMBNAILS_UPDATER_LIST, "thumbnails_updater_list") MSG_HASH(MENU_ENUM_LABEL_PL_THUMBNAILS_UPDATER_LIST, "pl_thumbnails_updater_list") +MSG_HASH(MENU_ENUM_LABEL_PL_THUMBNAILS_UPDATER_ENTRY, + "pl_thumbnails_updater_entry") MSG_HASH(MENU_ENUM_LABEL_TIMEDATE_ENABLE, "menu_timedate_enable") MSG_HASH(MENU_ENUM_LABEL_TIMEDATE_STYLE, diff --git a/menu/cbs/menu_cbs_ok.c b/menu/cbs/menu_cbs_ok.c index 0a95e1dc34..0afa074df3 100644 --- a/menu/cbs/menu_cbs_ok.c +++ b/menu/cbs/menu_cbs_ok.c @@ -4094,8 +4094,6 @@ void cb_generic_download(retro_task_t *task, dir_path = buf; break; } - case MENU_ENUM_LABEL_CB_SINGLE_THUMBNAIL: - break; default: RARCH_WARN("Unknown transfer type '%s' bailing out.\n", msg_hash_to_str(transf->enum_idx)); @@ -4105,12 +4103,6 @@ void cb_generic_download(retro_task_t *task, if (!string_is_empty(dir_path)) fill_pathname_join(output_path, dir_path, transf->path, sizeof(output_path)); - else if (transf->enum_idx == MENU_ENUM_LABEL_CB_SINGLE_THUMBNAIL) - { - /* In this particular case we have the whole path - * already built from the task */ - strlcpy(output_path, transf->path, sizeof(output_path)); - } /* Make sure the directory exists * This function is horrible. It mutates the original path @@ -4128,12 +4120,6 @@ void cb_generic_download(retro_task_t *task, if (!string_is_empty(dir_path)) fill_pathname_join(output_path, dir_path, transf->path, sizeof(output_path)); - else if (transf->enum_idx == MENU_ENUM_LABEL_CB_SINGLE_THUMBNAIL) - { - /* In this particular case we have the whole path - * already built from the task */ - strlcpy(output_path, transf->path, sizeof(output_path)); - } #ifdef HAVE_COMPRESSION if (path_is_compressed_file(output_path)) diff --git a/menu/menu_displaylist.c b/menu/menu_displaylist.c index 466582de6a..9f072eeea8 100644 --- a/menu/menu_displaylist.c +++ b/menu/menu_displaylist.c @@ -2974,7 +2974,7 @@ static unsigned menu_displaylist_parse_pl_thumbnail_download_list( menu_entries_append_enum(info->list, path_base, path, - MENU_ENUM_LABEL_PLAYLIST_ENTRY, + MENU_ENUM_LABEL_PL_THUMBNAILS_UPDATER_ENTRY, FILE_TYPE_DOWNLOAD_PL_THUMBNAIL_CONTENT, 0, 0); count++; diff --git a/menu/menu_thumbnail_path.c b/menu/menu_thumbnail_path.c index a6c137f04b..346a777999 100644 --- a/menu/menu_thumbnail_path.c +++ b/menu/menu_thumbnail_path.c @@ -470,6 +470,9 @@ bool menu_thumbnail_set_content_playlist(menu_thumbnail_path_data_t *path_data, /* Read playlist values */ playlist_get_index(playlist, idx, &entry); + if (!entry) + return false; + content_path = entry->path; content_label = entry->label; core_name = entry->core_name; diff --git a/msg_hash.h b/msg_hash.h index d0166bb93f..6ac4413e89 100644 --- a/msg_hash.h +++ b/msg_hash.h @@ -1177,6 +1177,7 @@ enum msg_hash_enums MENU_LABEL(MENU_SETTINGS), MENU_LABEL(THUMBNAILS_UPDATER_LIST), MENU_LABEL(PL_THUMBNAILS_UPDATER_LIST), + MENU_LABEL(PL_THUMBNAILS_UPDATER_ENTRY), MENU_LABEL(USER_INTERFACE_SETTINGS), MENU_LABEL(POWER_MANAGEMENT_SETTINGS), MENU_LABEL(RETRO_ACHIEVEMENTS_SETTINGS), diff --git a/tasks/task_pl_thumbnail_download.c b/tasks/task_pl_thumbnail_download.c index faaa2fa9f6..b8f3d571b8 100644 --- a/tasks/task_pl_thumbnail_download.c +++ b/tasks/task_pl_thumbnail_download.c @@ -23,6 +23,7 @@ #include #include #include +#include #include "tasks_internal.h" #include "task_file_transfer.h" @@ -30,6 +31,7 @@ #include "../configuration.h" #include "../file_path_special.h" #include "../playlist.h" +#include "../verbosity.h" #ifdef RARCH_INTERNAL #ifdef HAVE_MENU @@ -55,6 +57,7 @@ typedef struct pl_thumb_handle playlist_t *playlist; menu_thumbnail_path_data_t *thumbnail_path_data; retro_task_t *http_task; + bool http_task_complete; size_t list_size; size_t list_index; unsigned type_idx; @@ -162,16 +165,90 @@ static bool get_thumbnail_paths( return true; } +/* Thumbnail download http task callback function + * > Writes thumbnail file to disk */ +void cb_http_task_download_pl_thumbnail( + retro_task_t *task, void *task_data, + void *user_data, const char *err) +{ + http_transfer_data_t *data = (http_transfer_data_t*)task_data; + file_transfer_t *transf = (file_transfer_t*)user_data; + pl_thumb_handle_t *pl_thumb = NULL; + char output_dir[PATH_MAX_LENGTH]; + + output_dir[0] = '\0'; + + /* Update pl_thumb task status + * > Do this first, to minimise the risk of hanging + * the parent task in the event of an http error */ + if (!transf) + goto finish; + + pl_thumb = (pl_thumb_handle_t*)transf->user_data; + + if (!pl_thumb) + goto finish; + + pl_thumb->http_task_complete = true; + + /* Remaining sanity checks... */ + if (!data) + goto finish; + + if (!data->data || string_is_empty(transf->path)) + goto finish; + + /* Create output directory, if required */ + strlcpy(output_dir, transf->path, sizeof(output_dir)); + path_basedir_wrapper(output_dir); + + if (!path_mkdir(output_dir)) + { + err = msg_hash_to_str(MSG_FAILED_TO_CREATE_THE_DIRECTORY); + goto finish; + } + + /* Write thumbnail file to disk */ + if (!filestream_write_file(transf->path, data->data, data->len)) + { + err = "Write failed."; + goto finish; + } + +finish: + + /* Log any error messages */ + if (!string_is_empty(err)) + { + RARCH_ERR("Download of '%s' failed: %s\n", + (transf ? transf->path: "unknown"), err); + } + + if (data) + { + if (data->data) + free(data->data); + free(data); + } + + if (transf) + free(transf); +} + /* Download thumbnail of the current type for the current * playlist entry */ static void download_pl_thumbnail(pl_thumb_handle_t *pl_thumb) { char path[PATH_MAX_LENGTH]; char url[2048]; - + path[0] = '\0'; - url[0] = '\0'; - + url[0] = '\0'; + + /* Sanity check */ + if (!pl_thumb) + return; + /* Check if paths are valid */ if (get_thumbnail_paths(pl_thumb, path, sizeof(path), url, sizeof(url))) { @@ -181,15 +258,24 @@ static void download_pl_thumbnail(pl_thumb_handle_t *pl_thumb) file_transfer_t *transf = (file_transfer_t*)calloc(1, sizeof(file_transfer_t)); if (!transf) return; /* If this happens then everything is broken anyway... */ - + + /* Initialise http task status */ + pl_thumb->http_task_complete = false; + /* Initialise file transfer */ - transf->enum_idx = MENU_ENUM_LABEL_CB_SINGLE_THUMBNAIL; + transf->user_data = (void*)pl_thumb; strlcpy(transf->path, path, sizeof(transf->path)); - + /* Note: We don't actually care if this fails since that * just means the file is missing from the server, so it's * not something we can handle here... */ - pl_thumb->http_task = (retro_task_t*)task_push_http_transfer(url, true, NULL, cb_generic_download, transf); + pl_thumb->http_task = (retro_task_t*)task_push_http_transfer( + url, true, NULL, cb_http_task_download_pl_thumbnail, transf); + + /* ...if it does fail, however, we can immediately + * signal that the task is 'complete' */ + if (!pl_thumb->http_task) + pl_thumb->http_task_complete = true; } } } @@ -318,13 +404,19 @@ static void task_pl_thumbnail_download_handler(retro_task_t *task) { /* Ensure that we only enqueue one transfer * at a time... */ - if (pl_thumb->http_task) - { - if (task_get_finished(pl_thumb->http_task)) - pl_thumb->http_task = NULL; - else - break; - } + + /* > If HTTP task is NULL, then it either finished + * or an error occurred - in either case, + * current task is 'complete' */ + if (!pl_thumb->http_task) + pl_thumb->http_task_complete = true; + + /* > Wait for task_push_http_transfer() + * callback to trigger */ + if (pl_thumb->http_task_complete) + pl_thumb->http_task = NULL; + else + break; /* Check whether all thumbnail types have been processed */ if (pl_thumb->type_idx > 3) @@ -339,8 +431,7 @@ static void task_pl_thumbnail_download_handler(retro_task_t *task) } /* Download current thumbnail */ - if (pl_thumb) - download_pl_thumbnail(pl_thumb); + download_pl_thumbnail(pl_thumb); /* Increment thumbnail type */ pl_thumb->type_idx++; @@ -431,6 +522,7 @@ bool task_push_pl_thumbnail_download( pl_thumb->playlist = NULL; pl_thumb->thumbnail_path_data = NULL; pl_thumb->http_task = NULL; + pl_thumb->http_task_complete = false; pl_thumb->list_size = 0; pl_thumb->list_index = 0; pl_thumb->type_idx = 1; @@ -574,6 +666,17 @@ static void task_pl_entry_thumbnail_download_handler(retro_task_t *task) const char *right_thumbnail_path = NULL; const char *left_thumbnail_path = NULL; + /* Redundant safety check - ensure that playlist is + * non-NULL, and that the playlist path is the same + * as that recorded when task_push_pl_entry_thumbnail_download() + * was called... */ + if (!pl_thumb->playlist) + goto task_finished; + + if (!string_is_equal(pl_thumb->playlist_path, + playlist_get_conf_path(pl_thumb->playlist))) + goto task_finished; + /* Initialise thumbnail path data */ pl_thumb->thumbnail_path_data = menu_thumbnail_path_init(); @@ -616,13 +719,19 @@ static void task_pl_entry_thumbnail_download_handler(retro_task_t *task) { /* Ensure that we only enqueue one transfer * at a time... */ - if (pl_thumb->http_task) - { - if (task_get_finished(pl_thumb->http_task)) - pl_thumb->http_task = NULL; - else - break; - } + + /* > If HTTP task is NULL, then it either finished + * or an error occurred - in either case, + * current task is 'complete' */ + if (!pl_thumb->http_task) + pl_thumb->http_task_complete = true; + + /* > Wait for task_push_http_transfer() + * callback to trigger */ + if (pl_thumb->http_task_complete) + pl_thumb->http_task = NULL; + else + break; /* Check whether all thumbnail types have been processed */ if (pl_thumb->type_idx > 3) @@ -635,8 +744,7 @@ static void task_pl_entry_thumbnail_download_handler(retro_task_t *task) task_set_progress(task, ((pl_thumb->type_idx - 1) * 100) / 3); /* Download current thumbnail */ - if (pl_thumb) - download_pl_thumbnail(pl_thumb); + download_pl_thumbnail(pl_thumb); /* Increment thumbnail type */ pl_thumb->type_idx++; @@ -746,6 +854,7 @@ bool task_push_pl_entry_thumbnail_download( pl_thumb->playlist = playlist; pl_thumb->thumbnail_path_data = NULL; pl_thumb->http_task = NULL; + pl_thumb->http_task_complete = false; pl_thumb->list_size = playlist_size(playlist); pl_thumb->list_index = idx; pl_thumb->type_idx = 1;