From d86ca1176dea94312d7e0ec72e53e35c2c83309a Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Tue, 11 Feb 2020 14:12:04 +0100 Subject: [PATCH 01/17] [misc] Add an in-place string replacement function --- src/misc.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--- src/misc.h | 2 ++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/misc.c b/src/misc.c index 36427404..3a91b44e 100644 --- a/src/misc.c +++ b/src/misc.c @@ -380,6 +380,49 @@ safe_snprintf_cat(char *dst, size_t n, const char *fmt, ...) return -1; } +int +safe_snreplace(char *s, size_t sz, const char *pattern, const char *replacement) +{ + char *ptr; + char *src; + char *dst; + size_t num; + + if (!s) + return -1; + + if (!pattern || !replacement) + return 0; + + size_t p_len = strlen(pattern); + size_t r_len = strlen(replacement); + size_t s_len = strlen(s) + 1; // Incl terminator + + ptr = s; + while ((ptr = strstr(ptr, pattern))) + { + // We will move the part of the string after the pattern from src to dst + src = ptr + p_len; + dst = ptr + r_len; + + num = s_len - (src - s); // Number of bytes w/terminator we need to move + if (dst + num > s + sz) + return -1; // Not enough room + + // Shift everything after the pattern to the right, use memmove since + // there might be an overlap + memmove(dst, src, num); + + // Write replacement, no null terminater + memcpy(ptr, replacement, r_len); + + // Advance ptr to avoid infinite looping + ptr = dst; + } + + return 0; +} + /* Key/value functions */ struct keyval * @@ -387,7 +430,7 @@ keyval_alloc(void) { struct keyval *kv; - kv = (struct keyval *)malloc(sizeof(struct keyval)); + kv = calloc(1, sizeof(struct keyval)); if (!kv) { DPRINTF(E_LOG, L_MISC, "Out of memory for keyval alloc\n"); @@ -395,8 +438,6 @@ keyval_alloc(void) return NULL; } - memset(kv, 0, sizeof(struct keyval)); - return kv; } diff --git a/src/misc.h b/src/misc.h index 06ee8bd0..ce1af74a 100644 --- a/src/misc.h +++ b/src/misc.h @@ -87,6 +87,8 @@ safe_asprintf(const char *fmt, ...) __attribute__ ((format (printf, 1, 2))); int safe_snprintf_cat(char *dst, size_t n, const char *fmt, ...) __attribute__ ((format (printf, 3, 4))); +int +safe_snreplace(char *s, size_t sz, const char *pattern, const char *replacement); /* Key/value functions */ struct keyval * From 937f4431f8dd137441c7580ec27f4705011a250a Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Tue, 11 Feb 2020 14:13:18 +0100 Subject: [PATCH 02/17] [misc] Add json_drilldown() function Will descend into a json object along the path set by a list of keys --- src/misc_json.c | 23 +++++++++++++++++++++++ src/misc_json.h | 7 +++++++ 2 files changed, 30 insertions(+) diff --git a/src/misc_json.c b/src/misc_json.c index b6eb18d1..9968a215 100644 --- a/src/misc_json.c +++ b/src/misc_json.c @@ -32,6 +32,29 @@ #include "logger.h" +json_object * +jparse_drilldown(json_object *haystack, const char *keys[]) +{ + json_object *needle; + json_bool found; + + while (*keys) + { + found = json_object_object_get_ex(haystack, *keys, &needle); + if (!found) + return NULL; + + if (json_object_get_type(needle) == json_type_array) + haystack = json_object_array_get_idx(needle, 0); + else + haystack = needle; + + keys++; + } + + return needle; +} + void jparse_free(json_object* haystack) { diff --git a/src/misc_json.h b/src/misc_json.h index 2a8410fe..05a5a880 100644 --- a/src/misc_json.h +++ b/src/misc_json.h @@ -33,6 +33,13 @@ #include #include +// Convenience macro so that instead of calling jparse with an array of keys +// to follow, you can call JPARSE_DRILLDOWN(haystack, "key1", "key2"...) +#define JPARSE_DRILLDOWN(haystack, ...) jparse_drilldown(haystack, (const char *[]){__VA_ARGS__, NULL}) + +json_object * +jparse_drilldown(json_object *haystack, const char *keys[]); + void jparse_free(json_object *haystack); From 15b18e26b71c53ca0d7a6be089e59e6f064759f4 Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Tue, 11 Feb 2020 14:15:23 +0100 Subject: [PATCH 03/17] [http] Let the curl https client follow up to 5 redirects Requests for artwork to Cover Art Archive require redirects. Perhaps there are also some playlist requests that will benefit from this. --- src/http.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/http.c b/src/http.c index 1d48c6f3..69da96e5 100644 --- a/src/http.c +++ b/src/http.c @@ -386,10 +386,13 @@ https_client_request_impl(struct http_client_ctx *ctx) curl_easy_setopt(curl, CURLOPT_POSTFIELDS, ctx->output_body); curl_easy_setopt(curl, CURLOPT_TIMEOUT, HTTP_CLIENT_TIMEOUT); - curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, curl_request_cb); curl_easy_setopt(curl, CURLOPT_WRITEDATA, ctx); + // Artwork and playlist requests might require redirects + curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1); + curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 5); + /* Make request */ DPRINTF(E_INFO, L_HTTP, "Making request for %s\n", ctx->url); @@ -416,7 +419,6 @@ http_client_request(struct http_client_ctx *ctx) { if (strncmp(ctx->url, "http:", strlen("http:")) == 0) return http_client_request_impl(ctx); - #ifdef HAVE_LIBCURL if (strncmp(ctx->url, "https:", strlen("https:")) == 0) return https_client_request_impl(ctx); From 70f0ff1f61a5974ee60436867455414ff70b9bd7 Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Tue, 11 Feb 2020 14:17:28 +0100 Subject: [PATCH 04/17] [artwork] Support for online artwork sources - WIP * Discogs * Spotify * Cover Art Archive --- src/artwork.c | 674 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 546 insertions(+), 128 deletions(-) diff --git a/src/artwork.c b/src/artwork.c index 715e05be..81624185 100644 --- a/src/artwork.c +++ b/src/artwork.c @@ -33,6 +33,7 @@ #include "db.h" #include "misc.h" +#include "misc_json.h" #include "logger.h" #include "conffile.h" #include "cache.h" @@ -120,6 +121,37 @@ struct artwork_source { enum artwork_cache cache; }; +/* Since online sources of artwork have similar characteristics there generic + * callers for them. They use the below info to request artwork. + */ +enum parse_result { + ONLINE_SOURCE_PARSE_OK, + ONLINE_SOURCE_PARSE_INVALID, + ONLINE_SOURCE_PARSE_NOT_FOUND, + ONLINE_SOURCE_PARSE_NO_PARSER, +}; + +struct online_source { + // Name of online source + char *name; + + // How to authorize (using the Authorize http header) + char *auth_header; + char *auth_key; + char *auth_secret; + + // How to search for artwork + char *search_endpoint; + char *search_param; + struct query_part { + const char *key; + const char *template; + } query_parts[8]; + + // Function that can extract the artwork url from the parsed json response + enum parse_result (*response_jparse)(char **artwork_url, json_object *response, int max_w, int max_h); +}; + /* File extensions that we look for or accept */ static const char *cover_extension[] = @@ -127,7 +159,6 @@ static const char *cover_extension[] = "jpg", "png", }; - /* ----------------- DECLARE AND CONFIGURE SOURCE HANDLERS ----------------- */ /* Forward - group handlers */ @@ -139,6 +170,9 @@ static int source_item_embedded_get(struct artwork_ctx *ctx); static int source_item_own_get(struct artwork_ctx *ctx); static int source_item_stream_get(struct artwork_ctx *ctx); static int source_item_pipe_get(struct artwork_ctx *ctx); +static int source_item_discogs_get(struct artwork_ctx *ctx); +static int source_item_coverartarchive_get(struct artwork_ctx *ctx); +static int source_item_spotify2_get(struct artwork_ctx *ctx); static int source_item_spotify_get(struct artwork_ctx *ctx); static int source_item_spotifywebapi_get(struct artwork_ctx *ctx); static int source_item_ownpl_get(struct artwork_ctx *ctx); @@ -179,7 +213,27 @@ static struct artwork_source artwork_item_source[] = .data_kinds = (1 << DATA_KIND_FILE) | (1 << DATA_KIND_SPOTIFY), .cache = ON_FAILURE, }, +/* { + // TODO merge with spotifywebapi_get + .name = "Spotify2", + .handler = source_item_spotify2_get, + .data_kinds = (1 << DATA_KIND_FILE), + .cache = NEVER, + }, { + .name = "Discogs", + .handler = source_item_discogs_get, + .data_kinds = (1 << DATA_KIND_FILE), + .cache = NEVER, + }, + { + // The Cover Art Archive seems rather slow, so low priority + .name = "Cover Art Archive", + .handler = source_item_coverartarchive_get, + .data_kinds = (1 << DATA_KIND_FILE), + .cache = NEVER, + }, +*/ { .name = "embedded", .handler = source_item_embedded_get, .data_kinds = (1 << DATA_KIND_FILE), @@ -229,6 +283,57 @@ static struct artwork_source artwork_item_source[] = } }; +/* Forward - parsers of online source responses */ +static enum parse_result response_jparse_spotify(char **artwork_url, json_object *response, int max_w, int max_h); +static enum parse_result response_jparse_discogs(char **artwork_url, json_object *response, int max_w, int max_h); +static enum parse_result response_jparse_musicbrainz(char **artwork_url, json_object *response, int max_w, int max_h); + +/* Definitions of online sources */ +static struct online_source spotify_source = + { + .name = "Spotify", + .auth_header = "Bearer $SECRET$", + .search_endpoint = "https://api.spotify.com/v1/search", + .search_param = "type=album&limit=1&$QUERY$", + .query_parts = + { + { "q", "artist:$ARTIST$ album:$ALBUM$" }, + { NULL, NULL }, + }, + .response_jparse = response_jparse_spotify, + }; + +static struct online_source discogs_source = + { + .name = "Discogs", + .auth_header = "Discogs key=$KEY$, secret=$SECRET$", + .auth_key = "ivzUxlkUiwpptDKpSCHF", + .auth_secret = "CYLZyExtlznKCupoIIhTpHVDReLunhUo", + .search_endpoint = "https://api.discogs.com/database/search", + .search_param = "type=release&per_page=1&$QUERY$", + .query_parts = + { + { "artist", "$ARTIST$" }, + { "release_title", "$ALBUM$" }, + { "track", "$TITLE$" }, + { NULL, NULL }, + }, + .response_jparse = response_jparse_discogs, + }; + +static struct online_source musicbrainz_source = + { + .name = "Musicbrainz", + .search_endpoint = "http://musicbrainz.org/ws/2/release-group/", + .search_param = "limit=1&fmt=json&$QUERY$", + .query_parts = + { + { "query", "artist:$ARTIST$ AND release:$ALBUM$ AND status:Official" }, + { NULL, NULL }, + }, + .response_jparse = response_jparse_musicbrainz, + }; + /* -------------------------------- HELPERS -------------------------------- */ @@ -237,52 +342,65 @@ static struct artwork_source artwork_item_source[] = * * @out evbuf Image data * @in url URL for the image - * @return 0 on success, -1 on error + * @return ART_FMT_* on success, ART_E_ERROR otherwise */ static int -artwork_url_read(struct evbuffer *evbuf, const char *url) +artwork_read_byurl(struct evbuffer *evbuf, const char *url) { struct http_client_ctx client; struct keyval *kv; const char *content_type; - int len; + size_t len; int ret; DPRINTF(E_SPAM, L_ART, "Trying internet artwork in %s\n", url); - ret = ART_E_NONE; + CHECK_NULL(L_ART, kv = keyval_alloc()); len = strlen(url); if ((len < 14) || (len > PATH_MAX)) // Can't be shorter than http://a/1.jpg - goto out_url; - - kv = keyval_alloc(); - if (!kv) - goto out_url; + { + DPRINTF(E_LOG, L_ART, "Artwork request URL is invalid (len=%zu): '%s'\n", len, url); + goto error; + } memset(&client, 0, sizeof(struct http_client_ctx)); client.url = url; client.input_headers = kv; client.input_body = evbuf; - if (http_client_request(&client) < 0) - goto out_kv; + ret = http_client_request(&client); + if (ret < 0) + { + DPRINTF(E_LOG, L_ART, "Request to '%s' failed with return value %d\n", url, ret); + goto error; + } if (client.response_code != HTTP_OK) - goto out_kv; + { + DPRINTF(E_LOG, L_ART, "Request to '%s' failed with code %d\n", url, client.response_code); + goto error; + } content_type = keyval_get(kv, "Content-Type"); if (content_type && (strcmp(content_type, "image/jpeg") == 0)) ret = ART_FMT_JPEG; else if (content_type && (strcmp(content_type, "image/png") == 0)) ret = ART_FMT_PNG; + else + { + DPRINTF(E_LOG, L_ART, "Artwork from '%s' has no known content type\n", url); + goto error; + } - out_kv: keyval_clear(kv); free(kv); - - out_url: return ret; + + error: + keyval_clear(kv); + free(kv); + return ART_E_ERROR; } /* Reads an artwork file from the filesystem straight into an evbuf @@ -293,7 +411,7 @@ artwork_url_read(struct evbuffer *evbuf, const char *url) * @return 0 on success, -1 on error */ static int -artwork_read(struct evbuffer *evbuf, char *path) +artwork_read_bypath(struct evbuffer *evbuf, char *path) { uint8_t buf[4096]; struct stat sb; @@ -390,7 +508,8 @@ rescale_calculate(int *target_w, int *target_h, int width, int height, int max_w } /* - * Either gets the artwork file given in "path" from the file system (rescaled if needed) or rescales the artwork given in "inbuf". + * Either gets the artwork file given in "path" from the file system (rescaled + * if needed) or rescales the artwork given in "inbuf". * * @out evbuf Image data (rescaled if needed) * @in path Path to the artwork file (alternative to inbuf) @@ -455,7 +574,7 @@ artwork_get(struct evbuffer *evbuf, char *path, struct evbuffer *inbuf, int max_ else if (path) { // No rescaling required, just read the raw file into the evbuf - ret = artwork_read(evbuf, path); + ret = artwork_read_bypath(evbuf, path); if (ret < 0) goto fail_free_decode; @@ -507,6 +626,63 @@ artwork_get(struct evbuffer *evbuf, char *path, struct evbuffer *inbuf, int max_ return ART_E_ERROR; } +/* Rescales an image in an evbuf (if required) + * + * @out artwork Rescaled image data (or original, if not rescaled) + * @in raw Original image data + * @in max_w Requested max width + * @in max_h Requested max height + * @return 0 on success, -1 on error + */ +static int +artwork_evbuf_rescale(struct evbuffer *artwork, struct evbuffer *raw, int max_w, int max_h) +{ + struct evbuffer *refbuf; + int ret; + + CHECK_NULL(L_ART, refbuf = evbuffer_new()); + + // Make a refbuf of raw for ffmpeg image size probing and possibly rescaling. + // We keep raw around in case rescaling is not necessary. +#ifdef HAVE_LIBEVENT2_OLD + uint8_t *buf = evbuffer_pullup(raw, -1); + if (!buf) + { + DPRINTF(E_LOG, L_ART, "Could not pullup raw artwork\n"); + goto error; + } + + ret = evbuffer_add_reference(refbuf, buf, evbuffer_get_length(raw), NULL, NULL); +#else + ret = evbuffer_add_buffer_reference(refbuf, raw); +#endif + if (ret < 0) + { + DPRINTF(E_LOG, L_ART, "Could not copy/ref raw image for rescaling (ret=%d)\n", ret); + goto error; + } + + // For non-file input, artwork_get() will also fail if no rescaling is required + ret = artwork_get(artwork, NULL, refbuf, max_w, max_h, false); + if (ret == ART_E_ERROR) + { + DPRINTF(E_DBG, L_ART, "No rescaling required\n"); + ret = evbuffer_add_buffer(artwork, raw); + if (ret < 0) + { + DPRINTF(E_LOG, L_ART, "Could not add or rescale image to output evbuf (ret=%d)\n", ret); + goto error; + } + } + + evbuffer_free(refbuf); + return 0; + + error: + evbuffer_free(refbuf); + return -1; +} + /* Looks for an artwork file in a directory. Will rescale if needed. * * @out evbuf Image data @@ -517,7 +693,7 @@ artwork_get(struct evbuffer *evbuf, char *path, struct evbuffer *inbuf, int max_ * @return ART_FMT_* on success, ART_E_NONE on nothing found, ART_E_ERROR on error */ static int -artwork_get_dir_image(struct evbuffer *evbuf, char *dir, int max_w, int max_h, char *out_path) +artwork_get_bydir(struct evbuffer *evbuf, char *dir, int max_w, int max_h, char *out_path) { char path[PATH_MAX]; char parentdir[PATH_MAX]; @@ -617,6 +793,280 @@ artwork_get_dir_image(struct evbuffer *evbuf, char *dir, int max_w, int max_h, c return artwork_get(evbuf, path, NULL, max_w, max_h, false); } +/* Retrieves artwork from a URL. Will rescale if needed. + * + * @out artwork Image data + * @in url URL of the artwork + * @in max_w Requested max width + * @in max_h Requested max height + * @return ART_FMT_* on success, ART_E_ERROR on error + */ +static int +artwork_get_byurl(struct evbuffer *artwork, const char *url, int max_w, int max_h) +{ + struct evbuffer *raw; + int content_type; + int ret; + + CHECK_NULL(L_ART, raw = evbuffer_new()); + + content_type = artwork_read_byurl(raw, url); + if (content_type < 0) + goto error; + + ret = artwork_evbuf_rescale(artwork, raw, max_w, max_h); + if (ret < 0) + goto error; + + evbuffer_free(raw); + return content_type; + + error: + evbuffer_free(raw); + return ART_E_ERROR; +} + +/* ------------------------- ONLINE SOURCE HANDLING ----------------------- */ + +static enum parse_result +response_jparse_discogs(char **artwork_url, json_object *response, int max_w, int max_h) +{ + json_object *image; + const char *s; + const char *key; + + if ((max_w > 0 && max_w <= 150) || (max_h > 0 && max_h <= 150)) + key = "thumb"; + else + key = "cover_image"; + + image = JPARSE_DRILLDOWN(response, "results", key); + if (!image || json_object_get_type(image) != json_type_string) + return ONLINE_SOURCE_PARSE_NOT_FOUND; + + s = json_object_get_string(image); + if (!s) + return ONLINE_SOURCE_PARSE_INVALID; + + *artwork_url = strdup(s); + + return ONLINE_SOURCE_PARSE_OK; +} + +static enum parse_result +response_jparse_musicbrainz(char **artwork_url, json_object *response, int max_w, int max_h) +{ + json_object *id; + const char *s; + + id = JPARSE_DRILLDOWN(response, "release-groups", "id"); + if (!id || json_object_get_type(id) != json_type_string) + return ONLINE_SOURCE_PARSE_NOT_FOUND; + + s = json_object_get_string(id); + if (!s) + return ONLINE_SOURCE_PARSE_INVALID; + + // We will request 500 as a default. The use of https is not just for privacy + // it is also because the http client only supports redirects for https. + if ((max_w > 0 && max_w <= 250) || (max_h > 0 && max_h <= 250)) + *artwork_url = safe_asprintf("https://coverartarchive.org/release-group/%s/front-250", s); + else if ((max_w == 0 && max_h == 0) || (max_w <= 500 && max_h <= 500)) + *artwork_url = safe_asprintf("https://coverartarchive.org/release-group/%s/front-500", s); + else + *artwork_url = safe_asprintf("https://coverartarchive.org/release-group/%s/front-1200", s); + + return ONLINE_SOURCE_PARSE_OK; +} + +static enum parse_result +response_jparse_spotify(char **artwork_url, json_object *response, int max_w, int max_h) +{ + json_object *images; + json_object *image; + const char *s; + int image_count; + int i; + + images = JPARSE_DRILLDOWN(response, "albums", "items", "images"); + if (!images || json_object_get_type(images) != json_type_array) + return ONLINE_SOURCE_PARSE_INVALID; + + // Find first image that has a smaller width than the given max_w (this should + // avoid the need for resizing and improve performance at the cost of some + // quality loss). Note that Spotify returns the images ordered descending by + // width (widest image first). Special case is if no max width (max_w = 0) is + // given, then the widest images will be used. + s = NULL; + image_count = json_object_array_length(images); + for (i = 0; i < image_count; i++) + { + image = json_object_array_get_idx(images, i); + if (image) + { + s = jparse_str_from_obj(image, "url"); + + if (max_w <= 0 || jparse_int_from_obj(image, "width") <= max_w) + { + // We have the first image that has a smaller width than the given max_w + break; + } + } + } + + if (!s) + return ONLINE_SOURCE_PARSE_NOT_FOUND; + + *artwork_url = strdup(s); + return ONLINE_SOURCE_PARSE_OK; +} + +static enum parse_result +online_source_response_parse(char **artwork_url, struct online_source *src, struct evbuffer *response, int max_w, int max_h) +{ + json_object *jresponse; + char *body; + int ret; + + // 0-terminate for safety + evbuffer_add(response, "", 1); + body = (char *)evbuffer_pullup(response, -1); + + // TODO remove + DPRINTF(E_DBG, L_ART, "Response from '%s': %s\n", src->name, body); + + if (src->response_jparse) + { + jresponse = json_tokener_parse(body); + if (!jresponse) + return ONLINE_SOURCE_PARSE_INVALID; + + ret = src->response_jparse(artwork_url, jresponse, max_w, max_h); + jparse_free(jresponse); + } + else + ret = ONLINE_SOURCE_PARSE_NO_PARSER; + + return ret; +} + +static int +online_source_request_url_make(char *url, size_t url_size, struct online_source *src, const char *artist, const char *album, const char *title) +{ + struct keyval query = { 0 }; + char param[512]; + char *encoded_query; + int ret; + int i; + + for (i = 0; src->query_parts[i].key; i++) + { + snprintf(param, sizeof(param), "%s", src->query_parts[i].template); + if ((safe_snreplace(param, sizeof(param), "$ARTIST$", artist) < 0) || + (safe_snreplace(param, sizeof(param), "$ALBUM$", album) < 0) || + (safe_snreplace(param, sizeof(param), "$TITLE$", title) < 0)) + { + DPRINTF(E_WARN, L_ART, "Cannot make request for online artwork, query string is too long\n"); + goto error; + } + + ret = keyval_add(&query, src->query_parts[i].key, param); + if (ret < 0) + { + DPRINTF(E_LOG, L_ART, "keyval_add() failed in request_url_make()\n"); + goto error; + } + } + + encoded_query = http_form_urlencode(&query); + if (!encoded_query) + goto error; + + snprintf(url, url_size, "%s?%s", src->search_endpoint, src->search_param); + if (safe_snreplace(url, url_size, "$QUERY$", encoded_query) < 0) + { + DPRINTF(E_WARN, L_ART, "Cannot make request for online artwork, url is too long (%zu)\n", strlen(encoded_query)); + goto error; + } + + free(encoded_query); + keyval_clear(&query); + + return 0; + + error: + keyval_clear(&query); + return -1; +} + +static char * +online_source_search(struct online_source *src, struct artwork_ctx *ctx) +{ + char *artwork_url; + struct http_client_ctx client = { 0 }; + struct keyval output_headers = { 0 }; + char url[2048]; + char auth_header[256]; + int ret; + + if (!ctx->dbmfi->artist || !ctx->dbmfi->album || !ctx->dbmfi->title) + { + DPRINTF(E_DBG, L_ART, "Skipping artwork source %s, missing input data (artist=%s, album=%s, title=%s)\n", + src->name, ctx->dbmfi->artist, ctx->dbmfi->album, ctx->dbmfi->title); + return NULL; + } + + ret = online_source_request_url_make(url, sizeof(url), src, ctx->dbmfi->artist, ctx->dbmfi->album, ctx->dbmfi->title); + if (ret < 0) + { + DPRINTF(E_WARN, L_ART, "Skipping artwork source %s, could not construct a request URL\n", src->name); + return NULL; + } + + if (src->auth_header) + { + snprintf(auth_header, sizeof(auth_header), "%s", src->auth_header); + if ((safe_snreplace(auth_header, sizeof(auth_header), "$KEY$", src->auth_key) < 0) || + (safe_snreplace(auth_header, sizeof(auth_header), "$SECRET$", src->auth_secret) < 0)) + { + DPRINTF(E_WARN, L_ART, "Cannot make request for online artwork, auth header is too long\n"); + return NULL; + } + + keyval_add(&output_headers, "Authorization", auth_header); + } + + CHECK_NULL(L_ART, client.input_body = evbuffer_new()); + client.url = url; + client.output_headers = &output_headers; + + ret = http_client_request(&client); + keyval_clear(&output_headers); + if (ret < 0 || client.response_code != HTTP_OK) + { + DPRINTF(E_WARN, L_ART, "Artwork request to '%s' failed, response code %d\n", url, client.response_code); + evbuffer_free(client.input_body); + return NULL; + } + + ret = online_source_response_parse(&artwork_url, src, client.input_body, ctx->max_w, ctx->max_h); + if (ret == ONLINE_SOURCE_PARSE_NOT_FOUND) + DPRINTF(E_DBG, L_ART, "No image tag found in response from source '%s'\n", src->name); + else if (ret == ONLINE_SOURCE_PARSE_INVALID) + DPRINTF(E_WARN, L_ART, "Response from source '%s' was in an unexpected format\n", src->name); + else if (ret == ONLINE_SOURCE_PARSE_NO_PARSER) + DPRINTF(E_LOG, L_ART, "Bug! Cannot parse response from source '%s', parser missing\n", src->name); + else if (ret != ONLINE_SOURCE_PARSE_OK) + DPRINTF(E_LOG, L_ART, "Bug! Cannot parse response from source '%s', unknown error\n", src->name); + + evbuffer_free(client.input_body); + + if (ret != ONLINE_SOURCE_PARSE_OK) + return NULL; + + return artwork_url; +} + /* ---------------------- SOURCE HANDLER IMPLEMENTATION -------------------- */ @@ -673,7 +1123,7 @@ source_group_dir_get(struct artwork_ctx *ctx) if (access(dir, F_OK) < 0) continue; - ret = artwork_get_dir_image(ctx->evbuf, dir, ctx->max_w, ctx->max_h, ctx->path); + ret = artwork_get_bydir(ctx->evbuf, dir, ctx->max_w, ctx->max_h, ctx->path); if (ret > 0) { db_query_end(&qp); @@ -829,7 +1279,7 @@ source_item_stream_get(struct artwork_ctx *ctx) if (ret > 0) goto out_url; - ret = artwork_url_read(ctx->evbuf, url); + ret = artwork_read_byurl(ctx->evbuf, url); if (ret > 0) { @@ -871,94 +1321,100 @@ source_item_pipe_get(struct artwork_ctx *ctx) return artwork_get(ctx->evbuf, path, NULL, ctx->max_w, ctx->max_h, false); } +static int +source_item_discogs_get(struct artwork_ctx *ctx) +{ + char *url; + int ret; + + url = online_source_search(&discogs_source, ctx); + if (!url) + return ART_E_NONE; + + ret = artwork_get_byurl(ctx->evbuf, url, ctx->max_w, ctx->max_h); + + free(url); + return ret; +} + +static int +source_item_coverartarchive_get(struct artwork_ctx *ctx) +{ + char *url; + int ret; + + // We search Musicbrainz to get the Musicbrainz ID, which we need to get the + // artwork from the Cover Art Archive + url = online_source_search(&musicbrainz_source, ctx); + if (!url) + return ART_E_NONE; + + // TODO add support in http client for 307 redirects + ret = artwork_get_byurl(ctx->evbuf, url, ctx->max_w, ctx->max_h); + + free(url); + return ret; +} + #ifdef HAVE_SPOTIFY_H +static int +source_item_spotify2_get(struct artwork_ctx *ctx) +{ + struct spotifywebapi_access_token info; + char *url; + int ret; + + spotifywebapi_access_token_get(&info); + if (!info.token) + return ART_E_ERROR; + + spotify_source.auth_secret = info.token; + + url = online_source_search(&spotify_source, ctx); + free(info.token); + if (!url) + return ART_E_NONE; + + ret = artwork_get_byurl(ctx->evbuf, url, ctx->max_w, ctx->max_h); + + free(url); + return ret; +} + static int source_item_spotify_get(struct artwork_ctx *ctx) { struct evbuffer *raw; - struct evbuffer *evbuf; int ret; - raw = evbuffer_new(); - evbuf = evbuffer_new(); - if (!raw || !evbuf) - { - DPRINTF(E_LOG, L_ART, "Out of memory for Spotify evbuf\n"); - return ART_E_ERROR; - } + CHECK_NULL(L_ART, raw = evbuffer_new()); ret = spotify_artwork_get(raw, ctx->dbmfi->path, ctx->max_w, ctx->max_h); if (ret < 0) { DPRINTF(E_WARN, L_ART, "No artwork from Spotify for %s\n", ctx->dbmfi->path); evbuffer_free(raw); - evbuffer_free(evbuf); return ART_E_NONE; } - // Make a refbuf of raw for ffmpeg image size probing and possibly rescaling. - // We keep raw around in case rescaling is not necessary. -#ifdef HAVE_LIBEVENT2_OLD - uint8_t *buf = evbuffer_pullup(raw, -1); - if (!buf) - { - DPRINTF(E_LOG, L_ART, "Could not pullup raw artwork\n"); - goto out_free_evbuf; - } - - ret = evbuffer_add_reference(evbuf, buf, evbuffer_get_length(raw), NULL, NULL); -#else - ret = evbuffer_add_buffer_reference(evbuf, raw); -#endif + ret = artwork_evbuf_rescale(ctx->evbuf, raw, ctx->max_w, ctx->max_h); if (ret < 0) { - DPRINTF(E_LOG, L_ART, "Could not copy/ref raw image for ffmpeg\n"); - goto out_free_evbuf; + DPRINTF(E_LOG, L_ART, "Could not rescale Spotify artwork for '%s'\n", ctx->dbmfi->path); + evbuffer_free(raw); + return ART_E_ERROR; } - // For non-file input, artwork_get() will also fail if no rescaling is required - ret = artwork_get(ctx->evbuf, NULL, evbuf, ctx->max_w, ctx->max_h, false); - if (ret == ART_E_ERROR) - { - DPRINTF(E_DBG, L_ART, "Not rescaling Spotify image\n"); - ret = evbuffer_add_buffer(ctx->evbuf, raw); - if (ret < 0) - { - DPRINTF(E_LOG, L_ART, "Could not add or rescale image to output evbuf\n"); - goto out_free_evbuf; - } - } - - evbuffer_free(evbuf); evbuffer_free(raw); - return ART_FMT_JPEG; - - out_free_evbuf: - evbuffer_free(evbuf); - evbuffer_free(raw); - - return ART_E_ERROR; } static int source_item_spotifywebapi_get(struct artwork_ctx *ctx) { - struct evbuffer *raw; - struct evbuffer *evbuf; char *artwork_url; - int content_type; int ret; - artwork_url = NULL; - raw = evbuffer_new(); - evbuf = evbuffer_new(); - if (!raw || !evbuf) - { - DPRINTF(E_LOG, L_ART, "Out of memory for Spotify evbuf\n"); - return ART_E_ERROR; - } - artwork_url = spotifywebapi_artwork_url_get(ctx->dbmfi->path, ctx->max_w, ctx->max_h); if (!artwork_url) { @@ -966,59 +1422,21 @@ source_item_spotifywebapi_get(struct artwork_ctx *ctx) return ART_E_NONE; } - ret = artwork_url_read(raw, artwork_url); - if (ret <= 0) - goto out_free_evbuf; + ret = artwork_get_byurl(ctx->evbuf, artwork_url, ctx->max_w, ctx->max_h); - content_type = ret; - - // Make a refbuf of raw for ffmpeg image size probing and possibly rescaling. - // We keep raw around in case rescaling is not necessary. -#ifdef HAVE_LIBEVENT2_OLD - uint8_t *buf = evbuffer_pullup(raw, -1); - if (!buf) - { - DPRINTF(E_LOG, L_ART, "Could not pullup raw artwork\n"); - goto out_free_evbuf; - } - - ret = evbuffer_add_reference(evbuf, buf, evbuffer_get_length(raw), NULL, NULL); + free(artwork_url); + return ret; +} #else - ret = evbuffer_add_buffer_reference(evbuf, raw); -#endif - if (ret < 0) - { - DPRINTF(E_LOG, L_ART, "Could not copy/ref raw image for ffmpeg\n"); - goto out_free_evbuf; - } - - // For non-file input, artwork_get() will also fail if no rescaling is required - ret = artwork_get(ctx->evbuf, NULL, evbuf, ctx->max_w, ctx->max_h, false); - if (ret == ART_E_ERROR) - { - DPRINTF(E_DBG, L_ART, "Not rescaling Spotify image\n"); - ret = evbuffer_add_buffer(ctx->evbuf, raw); - if (ret < 0) - { - DPRINTF(E_LOG, L_ART, "Could not add or rescale image to output evbuf\n"); - goto out_free_evbuf; - } - } - - evbuffer_free(evbuf); - evbuffer_free(raw); - free(artwork_url); - - return content_type; - - out_free_evbuf: - evbuffer_free(evbuf); - evbuffer_free(raw); - free(artwork_url); +static int +source_item_spotify2_get(struct artwork_ctx *ctx) +{ + // Silence compiler warning about spotify_source being unused + (void)spotify_source; return ART_E_ERROR; } -#else + static int source_item_spotify_get(struct artwork_ctx *ctx) { From 18cf2dbbbff8ff504c948f00a13ceca91bec73ab Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Sun, 16 Feb 2020 21:29:43 +0100 Subject: [PATCH 05/17] [misc] Change json_drilldown to _select and fix error case --- src/artwork.c | 6 +++--- src/misc_json.c | 3 ++- src/misc_json.h | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/artwork.c b/src/artwork.c index 81624185..b8ddf6e1 100644 --- a/src/artwork.c +++ b/src/artwork.c @@ -840,7 +840,7 @@ response_jparse_discogs(char **artwork_url, json_object *response, int max_w, in else key = "cover_image"; - image = JPARSE_DRILLDOWN(response, "results", key); + image = JPARSE_SELECT(response, "results", key); if (!image || json_object_get_type(image) != json_type_string) return ONLINE_SOURCE_PARSE_NOT_FOUND; @@ -859,7 +859,7 @@ response_jparse_musicbrainz(char **artwork_url, json_object *response, int max_w json_object *id; const char *s; - id = JPARSE_DRILLDOWN(response, "release-groups", "id"); + id = JPARSE_SELECT(response, "release-groups", "id"); if (!id || json_object_get_type(id) != json_type_string) return ONLINE_SOURCE_PARSE_NOT_FOUND; @@ -888,7 +888,7 @@ response_jparse_spotify(char **artwork_url, json_object *response, int max_w, in int image_count; int i; - images = JPARSE_DRILLDOWN(response, "albums", "items", "images"); + images = JPARSE_SELECT(response, "albums", "items", "images"); if (!images || json_object_get_type(images) != json_type_array) return ONLINE_SOURCE_PARSE_INVALID; diff --git a/src/misc_json.c b/src/misc_json.c index 9968a215..a335e560 100644 --- a/src/misc_json.c +++ b/src/misc_json.c @@ -33,11 +33,12 @@ #include "logger.h" json_object * -jparse_drilldown(json_object *haystack, const char *keys[]) +jparse_select(json_object *haystack, const char *keys[]) { json_object *needle; json_bool found; + needle = NULL; while (*keys) { found = json_object_object_get_ex(haystack, *keys, &needle); diff --git a/src/misc_json.h b/src/misc_json.h index 05a5a880..cd55d80b 100644 --- a/src/misc_json.h +++ b/src/misc_json.h @@ -35,10 +35,10 @@ // Convenience macro so that instead of calling jparse with an array of keys // to follow, you can call JPARSE_DRILLDOWN(haystack, "key1", "key2"...) -#define JPARSE_DRILLDOWN(haystack, ...) jparse_drilldown(haystack, (const char *[]){__VA_ARGS__, NULL}) +#define JPARSE_SELECT(haystack, ...) jparse_select(haystack, (const char *[]){__VA_ARGS__, NULL}) json_object * -jparse_drilldown(json_object *haystack, const char *keys[]); +jparse_select(json_object *haystack, const char *keys[]); void jparse_free(json_object *haystack); From b73f33f8e9e554dffe0dd9f9be2148c06e7dbf92 Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Mon, 17 Feb 2020 22:08:31 +0100 Subject: [PATCH 06/17] [artwork] Use settings to enable online sources --- src/artwork.c | 33 ++++++++++++++++++++++++++++++--- src/settings.c | 8 ++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/artwork.c b/src/artwork.c index b8ddf6e1..77b79204 100644 --- a/src/artwork.c +++ b/src/artwork.c @@ -36,6 +36,7 @@ #include "misc_json.h" #include "logger.h" #include "conffile.h" +#include "settings.h" #include "cache.h" #include "http.h" #include "transcode.h" @@ -213,7 +214,7 @@ static struct artwork_source artwork_item_source[] = .data_kinds = (1 << DATA_KIND_FILE) | (1 << DATA_KIND_SPOTIFY), .cache = ON_FAILURE, }, -/* { + { // TODO merge with spotifywebapi_get .name = "Spotify2", .handler = source_item_spotify2_get, @@ -233,7 +234,7 @@ static struct artwork_source artwork_item_source[] = .data_kinds = (1 << DATA_KIND_FILE), .cache = NEVER, }, -*/ { + { .name = "embedded", .handler = source_item_embedded_get, .data_kinds = (1 << DATA_KIND_FILE), @@ -1067,6 +1068,23 @@ online_source_search(struct online_source *src, struct artwork_ctx *ctx) return artwork_url; } +static bool +online_source_is_enabled(const char *setting_name) +{ + struct settings_category *category; + struct settings_option *option; + + category = settings_category_get("artwork"); + if (!category) + return false; + + option = settings_option_get(category, setting_name); + if (!option) + return false; + + return settings_option_getbool(option); +} + /* ---------------------- SOURCE HANDLER IMPLEMENTATION -------------------- */ @@ -1327,6 +1345,9 @@ source_item_discogs_get(struct artwork_ctx *ctx) char *url; int ret; + if (!online_source_is_enabled("enable_discogs")) + return ART_E_NONE; + url = online_source_search(&discogs_source, ctx); if (!url) return ART_E_NONE; @@ -1343,6 +1364,9 @@ source_item_coverartarchive_get(struct artwork_ctx *ctx) char *url; int ret; + if (!online_source_is_enabled("enable_coverartarchive")) + return ART_E_NONE; + // We search Musicbrainz to get the Musicbrainz ID, which we need to get the // artwork from the Cover Art Archive url = online_source_search(&musicbrainz_source, ctx); @@ -1364,6 +1388,9 @@ source_item_spotify2_get(struct artwork_ctx *ctx) char *url; int ret; + if (!online_source_is_enabled("enable_spotify")) + return ART_E_NONE; + spotifywebapi_access_token_get(&info); if (!info.token) return ART_E_ERROR; @@ -1434,7 +1461,7 @@ source_item_spotify2_get(struct artwork_ctx *ctx) // Silence compiler warning about spotify_source being unused (void)spotify_source; - return ART_E_ERROR; + return ART_E_NONE; } static int diff --git a/src/settings.c b/src/settings.c index 3bdb7244..8e2f89d5 100644 --- a/src/settings.c +++ b/src/settings.c @@ -31,9 +31,17 @@ static struct settings_option webinterface_options[] = { "show_composer_for_genre", SETTINGS_TYPE_STR }, }; +static struct settings_option artwork_options[] = + { + { "enable_spotify", SETTINGS_TYPE_BOOL }, + { "enable_discogs", SETTINGS_TYPE_BOOL }, + { "enable_coverartarchive", SETTINGS_TYPE_BOOL }, + }; + static struct settings_category categories[] = { { "webinterface", webinterface_options, ARRAY_SIZE(webinterface_options) }, + { "artwork", artwork_options, ARRAY_SIZE(artwork_options) }, }; From a289135325bd11cb30f5df588ec4f75059d23f24 Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Tue, 18 Feb 2020 22:12:54 +0100 Subject: [PATCH 07/17] [artwork] Construct online src querys based on data_kind For http streams we don't have an album name to search for. Plus we don't want to cache those images. --- src/artwork.c | 224 +++++++++++++++++++++++++++++++------------------- 1 file changed, 140 insertions(+), 84 deletions(-) diff --git a/src/artwork.c b/src/artwork.c index 77b79204..892937fa 100644 --- a/src/artwork.c +++ b/src/artwork.c @@ -97,6 +97,7 @@ struct artwork_ctx { // Input data for item handlers struct db_media_file_info *dbmfi; int id; + uint32_t data_kind; // Input data for group handlers int64_t persistentid; @@ -171,12 +172,12 @@ static int source_item_embedded_get(struct artwork_ctx *ctx); static int source_item_own_get(struct artwork_ctx *ctx); static int source_item_stream_get(struct artwork_ctx *ctx); static int source_item_pipe_get(struct artwork_ctx *ctx); +static int source_item_libspotify_get(struct artwork_ctx *ctx); +static int source_item_spotifywebapi_track_get(struct artwork_ctx *ctx); +static int source_item_ownpl_get(struct artwork_ctx *ctx); +static int source_item_spotifywebapi_search_get(struct artwork_ctx *ctx); static int source_item_discogs_get(struct artwork_ctx *ctx); static int source_item_coverartarchive_get(struct artwork_ctx *ctx); -static int source_item_spotify2_get(struct artwork_ctx *ctx); -static int source_item_spotify_get(struct artwork_ctx *ctx); -static int source_item_spotifywebapi_get(struct artwork_ctx *ctx); -static int source_item_ownpl_get(struct artwork_ctx *ctx); /* List of sources that can provide artwork for a group (i.e. usually an album * identified by a persistentid). The source handlers will be called in the @@ -214,26 +215,6 @@ static struct artwork_source artwork_item_source[] = .data_kinds = (1 << DATA_KIND_FILE) | (1 << DATA_KIND_SPOTIFY), .cache = ON_FAILURE, }, - { - // TODO merge with spotifywebapi_get - .name = "Spotify2", - .handler = source_item_spotify2_get, - .data_kinds = (1 << DATA_KIND_FILE), - .cache = NEVER, - }, - { - .name = "Discogs", - .handler = source_item_discogs_get, - .data_kinds = (1 << DATA_KIND_FILE), - .cache = NEVER, - }, - { - // The Cover Art Archive seems rather slow, so low priority - .name = "Cover Art Archive", - .handler = source_item_coverartarchive_get, - .data_kinds = (1 << DATA_KIND_FILE), - .cache = NEVER, - }, { .name = "embedded", .handler = source_item_embedded_get, @@ -259,14 +240,14 @@ static struct artwork_source artwork_item_source[] = .cache = NEVER, }, { - .name = "Spotify", - .handler = source_item_spotify_get, + .name = "libspotify", + .handler = source_item_libspotify_get, .data_kinds = (1 << DATA_KIND_SPOTIFY), .cache = ON_SUCCESS, }, { - .name = "Spotify web api", - .handler = source_item_spotifywebapi_get, + .name = "Spotify track web api", + .handler = source_item_spotifywebapi_track_get, .data_kinds = (1 << DATA_KIND_SPOTIFY), .cache = ON_SUCCESS | ON_FAILURE, }, @@ -276,6 +257,44 @@ static struct artwork_source artwork_item_source[] = .data_kinds = (1 << DATA_KIND_HTTP), .cache = ON_SUCCESS | ON_FAILURE, }, + { + .name = "Spotify search web api (files)", + .handler = source_item_spotifywebapi_search_get, + .data_kinds = (1 << DATA_KIND_FILE), + .cache = ON_SUCCESS | ON_FAILURE, + }, + { + .name = "Spotify search web api (streams)", + .handler = source_item_spotifywebapi_search_get, + .data_kinds = (1 << DATA_KIND_HTTP) | (1 << DATA_KIND_PIPE), + .cache = NEVER, + }, + { + .name = "Discogs (files)", + .handler = source_item_discogs_get, + .data_kinds = (1 << DATA_KIND_FILE), + .cache = ON_SUCCESS | ON_FAILURE, + }, + { + .name = "Discogs (streams)", + .handler = source_item_discogs_get, + .data_kinds = (1 << DATA_KIND_HTTP) | (1 << DATA_KIND_PIPE), + .cache = NEVER, + }, + { + // The Cover Art Archive seems rather slow, so low priority + .name = "Cover Art Archive (files)", + .handler = source_item_coverartarchive_get, + .data_kinds = (1 << DATA_KIND_FILE), + .cache = ON_SUCCESS | ON_FAILURE, + }, + { + // The Cover Art Archive seems rather slow, so low priority + .name = "Cover Art Archive (streams)", + .handler = source_item_coverartarchive_get, + .data_kinds = (1 << DATA_KIND_HTTP) | (1 << DATA_KIND_PIPE), + .cache = NEVER, + }, { .name = NULL, .handler = NULL, @@ -299,6 +318,7 @@ static struct online_source spotify_source = .query_parts = { { "q", "artist:$ARTIST$ album:$ALBUM$" }, + { "q", "artist:$ARTIST$ track:$TITLE$" }, { NULL, NULL }, }, .response_jparse = response_jparse_spotify, @@ -330,6 +350,7 @@ static struct online_source musicbrainz_source = .query_parts = { { "query", "artist:$ARTIST$ AND release:$ALBUM$ AND status:Official" }, + { "query", "artist:$ARTIST$ AND title:$TITLE$ AND status:Official" }, { NULL, NULL }, }, .response_jparse = response_jparse_musicbrainz, @@ -952,16 +973,56 @@ online_source_response_parse(char **artwork_url, struct online_source *src, stru } static int -online_source_request_url_make(char *url, size_t url_size, struct online_source *src, const char *artist, const char *album, const char *title) +online_source_request_url_make(char *url, size_t url_size, struct online_source *src, struct artwork_ctx *ctx) { + struct db_queue_item *queue_item; struct keyval query = { 0 }; + const char *artist = NULL; + const char *album = NULL; + const char *title = NULL; char param[512]; - char *encoded_query; + char *encoded_query = NULL; int ret; int i; + // First check if the item is in the queue. When searching for artwork, it is + // better to use queue_item metadata. For stream items the queue metadata will + // for instance be updated with icy metadata. It is also possible we are asked + // for artwork for a non-library item. + queue_item = db_queue_fetch_byfileid(ctx->id); + if (queue_item && ctx->data_kind == DATA_KIND_HTTP) + { + // Normally we prefer searching by artist and album, but for streams we + // take the below approach, since they have no album information, and the + // title is in the album field + artist = queue_item->artist; + title = queue_item->album; + } + else if (queue_item) + { + artist = queue_item->artist; + album = queue_item->album; + } + else + { + // We will just search for artist and album + artist = ctx->dbmfi->artist; + album = ctx->dbmfi->album; + } + + if (!artist || (!album && !title)) + { + DPRINTF(E_DBG, L_ART, "Cannot construct query to %s, missing input data (artist=%s, album=%s, title=%s)\n", src->name, artist, album, title); + goto error; + } + for (i = 0; src->query_parts[i].key; i++) { + if (!album && strstr(src->query_parts[i].template, "$ALBUM$")) + continue; + if (!title && strstr(src->query_parts[i].template, "$TITLE$")) + continue; + snprintf(param, sizeof(param), "%s", src->query_parts[i].template); if ((safe_snreplace(param, sizeof(param), "$ARTIST$", artist) < 0) || (safe_snreplace(param, sizeof(param), "$ALBUM$", album) < 0) || @@ -992,11 +1053,14 @@ online_source_request_url_make(char *url, size_t url_size, struct online_source free(encoded_query); keyval_clear(&query); + free_queue_item(queue_item, 0); return 0; error: + free(encoded_query); keyval_clear(&query); + free_queue_item(queue_item, 0); return -1; } @@ -1010,14 +1074,7 @@ online_source_search(struct online_source *src, struct artwork_ctx *ctx) char auth_header[256]; int ret; - if (!ctx->dbmfi->artist || !ctx->dbmfi->album || !ctx->dbmfi->title) - { - DPRINTF(E_DBG, L_ART, "Skipping artwork source %s, missing input data (artist=%s, album=%s, title=%s)\n", - src->name, ctx->dbmfi->artist, ctx->dbmfi->album, ctx->dbmfi->title); - return NULL; - } - - ret = online_source_request_url_make(url, sizeof(url), src, ctx->dbmfi->artist, ctx->dbmfi->album, ctx->dbmfi->title); + ret = online_source_request_url_make(url, sizeof(url), src, ctx); if (ret < 0) { DPRINTF(E_WARN, L_ART, "Skipping artwork source %s, could not construct a request URL\n", src->name); @@ -1382,34 +1439,7 @@ source_item_coverartarchive_get(struct artwork_ctx *ctx) #ifdef HAVE_SPOTIFY_H static int -source_item_spotify2_get(struct artwork_ctx *ctx) -{ - struct spotifywebapi_access_token info; - char *url; - int ret; - - if (!online_source_is_enabled("enable_spotify")) - return ART_E_NONE; - - spotifywebapi_access_token_get(&info); - if (!info.token) - return ART_E_ERROR; - - spotify_source.auth_secret = info.token; - - url = online_source_search(&spotify_source, ctx); - free(info.token); - if (!url) - return ART_E_NONE; - - ret = artwork_get_byurl(ctx->evbuf, url, ctx->max_w, ctx->max_h); - - free(url); - return ret; -} - -static int -source_item_spotify_get(struct artwork_ctx *ctx) +source_item_libspotify_get(struct artwork_ctx *ctx) { struct evbuffer *raw; int ret; @@ -1437,7 +1467,7 @@ source_item_spotify_get(struct artwork_ctx *ctx) } static int -source_item_spotifywebapi_get(struct artwork_ctx *ctx) +source_item_spotifywebapi_track_get(struct artwork_ctx *ctx) { char *artwork_url; int ret; @@ -1454,27 +1484,54 @@ source_item_spotifywebapi_get(struct artwork_ctx *ctx) free(artwork_url); return ret; } + +static int +source_item_spotifywebapi_search_get(struct artwork_ctx *ctx) +{ + struct spotifywebapi_access_token info; + char *url; + int ret; + + if (!online_source_is_enabled("enable_spotify")) + return ART_E_NONE; + + spotifywebapi_access_token_get(&info); + if (!info.token) + return ART_E_ERROR; + + spotify_source.auth_secret = info.token; + + url = online_source_search(&spotify_source, ctx); + free(info.token); + if (!url) + return ART_E_NONE; + + ret = artwork_get_byurl(ctx->evbuf, url, ctx->max_w, ctx->max_h); + + free(url); + return ret; +} #else static int -source_item_spotify2_get(struct artwork_ctx *ctx) +source_item_libspotify_get(struct artwork_ctx *ctx) +{ + return ART_E_ERROR; +} + +static int +source_item_spotifywebapi_track_get(struct artwork_ctx *ctx) +{ + return ART_E_ERROR; +} + +static int +source_item_spotifywebapi_search_get(struct artwork_ctx *ctx) { // Silence compiler warning about spotify_source being unused (void)spotify_source; return ART_E_NONE; } - -static int -source_item_spotify_get(struct artwork_ctx *ctx) -{ - return ART_E_ERROR; -} - -static int -source_item_spotifywebapi_get(struct artwork_ctx *ctx) -{ - return ART_E_ERROR; -} #endif /* First looks of the mfi->path is in any playlist, and if so looks in the dir @@ -1538,7 +1595,6 @@ static int process_items(struct artwork_ctx *ctx, int item_mode) { struct db_media_file_info dbmfi; - uint32_t data_kind; int i; int ret; @@ -1560,8 +1616,8 @@ process_items(struct artwork_ctx *ctx, int item_mode) goto no_artwork; ret = (safe_atoi32(dbmfi.id, &ctx->id) < 0) || - (safe_atou32(dbmfi.data_kind, &data_kind) < 0) || - (data_kind > 30); + (safe_atou32(dbmfi.data_kind, &ctx->data_kind) < 0) || + (ctx->data_kind > 30); if (ret) { DPRINTF(E_LOG, L_ART, "Error converting dbmfi id or data_kind to number\n"); @@ -1570,7 +1626,7 @@ process_items(struct artwork_ctx *ctx, int item_mode) for (i = 0; artwork_item_source[i].handler; i++) { - if ((artwork_item_source[i].data_kinds & (1 << data_kind)) == 0) + if ((artwork_item_source[i].data_kinds & (1 << ctx->data_kind)) == 0) continue; // If just one handler says we should not cache a negative result then we obey that From b43e174baff49e3a3d40685527941448e2889c19 Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Wed, 19 Feb 2020 22:57:24 +0100 Subject: [PATCH 08/17] [logger] Show long log messages truncated --- src/logger.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/logger.c b/src/logger.c index b7b43568..1b36cfcb 100644 --- a/src/logger.c +++ b/src/logger.c @@ -121,8 +121,10 @@ vlogger_writer(int severity, int domain, const char *fmt, va_list args) va_copy(ap, args); ret = vsnprintf(content, sizeof(content), fmt, ap); - if (ret < 0 || ret >= sizeof(content)) - strcpy(content, "(LOGGING SKIPPED - invalid content)\n"); + if (ret < 0) + strcpy(content, "(LOGGING SKIPPED - error printing log message)\n"); + else if (ret >= sizeof(content)) + strcpy(content + sizeof(content) - 8, "...\n"); va_end(ap); ret = repeat_count(content); From 2651a979fe7412bc304850842228bbff3e1c6edd Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Wed, 19 Feb 2020 22:58:33 +0100 Subject: [PATCH 09/17] [inputs] Fix issue where input metadata would not be erased When playing a stream, the input metadata is transferred to the queue_item. However, that was not done if there was no input metadata, which meant that old metadata was not getting erased. --- src/inputs/file_http.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/inputs/file_http.c b/src/inputs/file_http.c index 01ff0e20..5206fee3 100644 --- a/src/inputs/file_http.c +++ b/src/inputs/file_http.c @@ -144,13 +144,10 @@ metadata_get_http(struct input_metadata *metadata, struct input_source *source) return -1; // TODO Perhaps a problem since this prohibits the player updating metadata } - if (m->artist) - swap_pointers(&metadata->artist, &m->artist); + swap_pointers(&metadata->artist, &m->artist); // Note we map title to album, because clients should show stream name as titel - if (m->title) - swap_pointers(&metadata->album, &m->title); - if (m->artwork_url) - swap_pointers(&metadata->artwork_url, &m->artwork_url); + swap_pointers(&metadata->album, &m->title); + swap_pointers(&metadata->artwork_url, &m->artwork_url); http_icy_metadata_free(m, 0); return 0; From 9068c66dcdd4c030f09ec93afc95191281edaa9f Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Wed, 19 Feb 2020 23:01:43 +0100 Subject: [PATCH 10/17] [cache] Minor changes so "const char *" path arguments are accepted --- src/cache.c | 21 +++++++++++---------- src/cache.h | 6 +++--- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/cache.c b/src/cache.c index d8c277f8..05b75eb0 100644 --- a/src/cache.c +++ b/src/cache.c @@ -57,7 +57,8 @@ struct cache_arg int is_remote; int msec; - char *path; // artwork path + const char *path; // artwork path + char *pathcopy; // copy of artwork path (for async operations) int type; // individual or group artwork int64_t persistentid; int max_w; @@ -903,7 +904,7 @@ cache_daap_listener_cb(short event_mask) * after the cached timestamp. All cache entries for the given path are deleted, if the file was * modified after the cached timestamp. * - * @param cmdarg->path the full path to the artwork file (could be an jpg/png image or a media file with embedded artwork) + * @param cmdarg->pathcopy the full path to the artwork file (could be an jpg/png image or a media file with embedded artwork) * @param cmdarg->mtime modified timestamp of the artwork file * @return 0 if successful, -1 if an error occurred */ @@ -919,7 +920,7 @@ cache_artwork_ping_impl(void *arg, int *retval) int ret; cmdarg = arg; - query = sqlite3_mprintf(Q_TMPL_PING, (int64_t)time(NULL), cmdarg->path, (int64_t)cmdarg->mtime); + query = sqlite3_mprintf(Q_TMPL_PING, (int64_t)time(NULL), cmdarg->pathcopy, (int64_t)cmdarg->mtime); DPRINTF(E_DBG, L_CACHE, "Running query '%s'\n", query); @@ -934,7 +935,7 @@ cache_artwork_ping_impl(void *arg, int *retval) if (cmdarg->del > 0) { - query = sqlite3_mprintf(Q_TMPL_DEL, cmdarg->path, (int64_t)cmdarg->mtime); + query = sqlite3_mprintf(Q_TMPL_DEL, cmdarg->pathcopy, (int64_t)cmdarg->mtime); DPRINTF(E_DBG, L_CACHE, "Running query '%s'\n", query); @@ -948,14 +949,14 @@ cache_artwork_ping_impl(void *arg, int *retval) } } - free(cmdarg->path); + free(cmdarg->pathcopy); *retval = 0; return COMMAND_END; error_ping: sqlite3_free(errmsg); - free(cmdarg->path); + free(cmdarg->pathcopy); *retval = -1; return COMMAND_END; @@ -1415,7 +1416,7 @@ cache_artwork_ping(const char *path, time_t mtime, int del) return; } - cmdarg->path = strdup(path); + cmdarg->pathcopy = strdup(path); cmdarg->mtime = mtime; cmdarg->del = del; @@ -1429,7 +1430,7 @@ cache_artwork_ping(const char *path, time_t mtime, int del) * @return 0 if successful, -1 if an error occurred */ int -cache_artwork_delete_by_path(char *path) +cache_artwork_delete_by_path(const char *path) { struct cache_arg cmdarg; @@ -1541,7 +1542,7 @@ cache_artwork_get(int type, int64_t persistentid, int max_w, int max_h, int *cac * @return 0 if successful, -1 if an error occurred */ int -cache_artwork_stash(struct evbuffer *evbuf, char *path, int format) +cache_artwork_stash(struct evbuffer *evbuf, const char *path, int format) { struct cache_arg cmdarg; @@ -1564,7 +1565,7 @@ cache_artwork_stash(struct evbuffer *evbuf, char *path, int format) * @return 0 if successful, -1 if an error occurred */ int -cache_artwork_read(struct evbuffer *evbuf, char *path, int *format) +cache_artwork_read(struct evbuffer *evbuf, const char *path, int *format) { struct cache_arg cmdarg; int ret; diff --git a/src/cache.h b/src/cache.h index 3e7aa8ae..acf8bfbb 100644 --- a/src/cache.h +++ b/src/cache.h @@ -31,7 +31,7 @@ void cache_artwork_ping(const char *path, time_t mtime, int del); int -cache_artwork_delete_by_path(char *path); +cache_artwork_delete_by_path(const char *path); int cache_artwork_purge_cruft(time_t ref); @@ -43,10 +43,10 @@ int cache_artwork_get(int type, int64_t persistentid, int max_w, int max_h, int *cached, int *format, struct evbuffer *evbuf); int -cache_artwork_stash(struct evbuffer *evbuf, char *path, int format); +cache_artwork_stash(struct evbuffer *evbuf, const char *path, int format); int -cache_artwork_read(struct evbuffer *evbuf, char *path, int *format); +cache_artwork_read(struct evbuffer *evbuf, const char *path, int *format); /* ---------------------------- Cache API --------------------------- */ From 8261fb8e595a7abcf6eecc8a136e3aa21077609a Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Wed, 19 Feb 2020 23:03:50 +0100 Subject: [PATCH 11/17] [http] Don't return zero-length ICY title metadata --- src/http.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/http.c b/src/http.c index 69da96e5..09caddb2 100644 --- a/src/http.c +++ b/src/http.c @@ -660,6 +660,8 @@ metadata_packet_get(struct http_icy_metadata *metadata, AVFormatContext *fmtctx) metadata->title = strdup(ptr + 3); } + else if (strlen(metadata->title) == 0) + metadata->title = NULL; else metadata->title = strdup(metadata->title); } From ad9ebb75c6fe6c3c7f2dbb6ecb5ffae72facf11d Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Wed, 19 Feb 2020 23:06:16 +0100 Subject: [PATCH 12/17] [artwork] Refinement of online artwork search * Add system to avoid making too many futile requests + repeated requests * Fixup Spotify artwork search (use type=track, type=album gave empty results) * Include stash caching in artwork_get_byurl() --- src/artwork.c | 135 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 106 insertions(+), 29 deletions(-) diff --git a/src/artwork.c b/src/artwork.c index 892937fa..4fbf6781 100644 --- a/src/artwork.c +++ b/src/artwork.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2015-2017 Espen Jürgensen + * Copyright (C) 2015-2020 Espen Jürgensen * Copyright (C) 2010-2011 Julien BLACHE * * This program is free software; you can redistribute it and/or modify @@ -69,6 +69,10 @@ #define ART_E_ERROR -1 #define ART_E_ABORT -2 +// See online_source_is_failing() +#define ONLINE_SEARCH_COOLDOWN_TIME 3600 +#define ONLINE_SEARCH_FAILURES_MAX 3 + enum artwork_cache { NEVER = 0, // No caching of any results @@ -145,11 +149,21 @@ struct online_source { // How to search for artwork char *search_endpoint; char *search_param; - struct query_part { + struct { const char *key; const char *template; } query_parts[8]; + // Cache previous artwork searches, so we can avoid futile requests + struct { + int last_id; + uint32_t last_hash; + int last_response_code; + char *last_artwork_url; + time_t last_timestamp; + int count_failures; + } search_history; + // Function that can extract the artwork url from the parsed json response enum parse_result (*response_jparse)(char **artwork_url, json_object *response, int max_w, int max_h); }; @@ -314,10 +328,10 @@ static struct online_source spotify_source = .name = "Spotify", .auth_header = "Bearer $SECRET$", .search_endpoint = "https://api.spotify.com/v1/search", - .search_param = "type=album&limit=1&$QUERY$", + .search_param = "type=track&limit=1&$QUERY$", .query_parts = { - { "q", "artist:$ARTIST$ album:$ALBUM$" }, + { "q", "artist:$ARTIST$ album:$ALBUM$" }, // TODO test if album search works with type=track { "q", "artist:$ARTIST$ track:$TITLE$" }, { NULL, NULL }, }, @@ -815,7 +829,8 @@ artwork_get_bydir(struct evbuffer *evbuf, char *dir, int max_w, int max_h, char return artwork_get(evbuf, path, NULL, max_w, max_h, false); } -/* Retrieves artwork from a URL. Will rescale if needed. +/* Retrieves artwork from an URL. Will use cache as appropriate, and will + * rescale if needed. * * @out artwork Image data * @in url URL of the artwork @@ -827,21 +842,34 @@ static int artwork_get_byurl(struct evbuffer *artwork, const char *url, int max_w, int max_h) { struct evbuffer *raw; - int content_type; + int format; int ret; CHECK_NULL(L_ART, raw = evbuffer_new()); - content_type = artwork_read_byurl(raw, url); - if (content_type < 0) + ret = cache_artwork_read(raw, url, &format); + if (ret == 0 && format > 0) + { + ret = artwork_evbuf_rescale(artwork, raw, max_w, max_h); + if (ret < 0) + goto error; + + evbuffer_free(raw); + return format; + } + + format = artwork_read_byurl(raw, url); + if (format < 0) goto error; + cache_artwork_stash(raw, url, format); + ret = artwork_evbuf_rescale(artwork, raw, max_w, max_h); if (ret < 0) goto error; evbuffer_free(raw); - return content_type; + return format; error: evbuffer_free(raw); @@ -910,9 +938,9 @@ response_jparse_spotify(char **artwork_url, json_object *response, int max_w, in int image_count; int i; - images = JPARSE_SELECT(response, "albums", "items", "images"); + images = JPARSE_SELECT(response, "tracks", "items", "album", "images"); if (!images || json_object_get_type(images) != json_type_array) - return ONLINE_SOURCE_PARSE_INVALID; + return ONLINE_SOURCE_PARSE_NOT_FOUND; // Find first image that has a smaller width than the given max_w (this should // avoid the need for resizing and improve performance at the cost of some @@ -1064,12 +1092,53 @@ online_source_request_url_make(char *url, size_t url_size, struct online_source return -1; } +static bool +online_source_is_failing(struct online_source *src, int id) +{ + // If the last request was more than ONLINE_SEARCH_COOLDOWN_TIME ago we will always try again + if (time(NULL) > src->search_history.last_timestamp + ONLINE_SEARCH_COOLDOWN_TIME) + return false; + + // We won't try again if the source was not replying as expected + if (src->search_history.last_response_code != HTTP_OK) + return true; + + // The playback source has changed since the last search, let's give it a chance + // (internet streams can feed us with garbage search metadata, but will not change id) + if (id != src->search_history.last_id) + return false; + + // We allow up to ONLINE_SEARCH_FAILURES_MAX for the same track id before declaring failure + if (src->search_history.count_failures < ONLINE_SEARCH_FAILURES_MAX) + return false; + + return true; +} + +static void +online_source_history_update(struct online_source *src, int id, uint32_t request_hash, int response_code, const char *artwork_url) +{ + src->search_history.last_id = id; + src->search_history.last_hash = request_hash; + src->search_history.last_response_code = response_code; + src->search_history.last_timestamp = time(NULL); + + free(src->search_history.last_artwork_url); + src->search_history.last_artwork_url = safe_strdup(artwork_url); // FIXME should free this on exit + + if (artwork_url) + src->search_history.count_failures = 0; + else + src->search_history.count_failures++; +} + static char * online_source_search(struct online_source *src, struct artwork_ctx *ctx) { char *artwork_url; struct http_client_ctx client = { 0 }; struct keyval output_headers = { 0 }; + uint32_t hash; char url[2048]; char auth_header[256]; int ret; @@ -1081,6 +1150,21 @@ online_source_search(struct online_source *src, struct artwork_ctx *ctx) return NULL; } + // Be nice to our peer + improve response times by not repeating search requests + hash = djb_hash(url, strlen(url)); + if (hash == src->search_history.last_hash) + { + artwork_url = safe_strdup(src->search_history.last_artwork_url); + return artwork_url; + } + + // If our recent searches have been futile we may give the source a break + if (online_source_is_failing(src, ctx->id)) + { + DPRINTF(E_DBG, L_ART, "Skipping artwork source %s, too many failed requests\n", src->name); + return NULL; + } + if (src->auth_header) { snprintf(auth_header, sizeof(auth_header), "%s", src->auth_header); @@ -1103,8 +1187,7 @@ online_source_search(struct online_source *src, struct artwork_ctx *ctx) if (ret < 0 || client.response_code != HTTP_OK) { DPRINTF(E_WARN, L_ART, "Artwork request to '%s' failed, response code %d\n", url, client.response_code); - evbuffer_free(client.input_body); - return NULL; + goto error; } ret = online_source_response_parse(&artwork_url, src, client.input_body, ctx->max_w, ctx->max_h); @@ -1117,12 +1200,17 @@ online_source_search(struct online_source *src, struct artwork_ctx *ctx) else if (ret != ONLINE_SOURCE_PARSE_OK) DPRINTF(E_LOG, L_ART, "Bug! Cannot parse response from source '%s', unknown error\n", src->name); - evbuffer_free(client.input_body); - if (ret != ONLINE_SOURCE_PARSE_OK) - return NULL; + goto error; + online_source_history_update(src, ctx->id, hash, client.response_code, artwork_url); + evbuffer_free(client.input_body); return artwork_url; + + error: + online_source_history_update(src, ctx->id, hash, client.response_code, NULL); + evbuffer_free(client.input_body); + return NULL; } static bool @@ -1314,8 +1402,7 @@ source_item_own_get(struct artwork_ctx *ctx) * stream (the StreamUrl tag). The path will be converted back to the id, which * is given to the player. If the id is currently being played, and there is a * valid ICY metadata artwork URL available, it will be returned to this - * function, which will then use the http client to get the artwork. Notice: No - * rescaling is done. + * function, which will then use the http client to get the artwork. */ static int source_item_stream_get(struct artwork_ctx *ctx) @@ -1350,17 +1437,7 @@ source_item_stream_get(struct artwork_ctx *ctx) if ((strcmp(ext, ".jpg") != 0) && (strcmp(ext, ".png") != 0)) goto out_url; - cache_artwork_read(ctx->evbuf, url, &ret); - if (ret > 0) - goto out_url; - - ret = artwork_read_byurl(ctx->evbuf, url); - - if (ret > 0) - { - DPRINTF(E_SPAM, L_ART, "Found internet stream artwork in %s (%d)\n", url, ret); - cache_artwork_stash(ctx->evbuf, url, ret); - } + ret = artwork_get_byurl(ctx->evbuf, url, ctx->max_w, ctx->max_h); out_url: free(url); From c674f844979f68337e3c3dda7a0a2812712a0234 Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Wed, 19 Feb 2020 23:25:59 +0100 Subject: [PATCH 13/17] [json] Make clients reload artwork when we have new http metadata --- src/httpd_jsonapi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/httpd_jsonapi.c b/src/httpd_jsonapi.c index 43301196..4a263a51 100644 --- a/src/httpd_jsonapi.c +++ b/src/httpd_jsonapi.c @@ -2037,7 +2037,7 @@ queue_item_to_json(struct db_queue_item *queue_item, char shuffle) } else if (queue_item->file_id > 0 && queue_item->file_id != DB_MEDIA_FILE_NON_PERSISTENT_ID) { - if (queue_item->data_kind != DATA_KIND_PIPE) + if (queue_item->data_kind == DATA_KIND_FILE) { // Queue item does not have a valid artwork url, construct artwork url to // get the image through the httpd_artworkapi (uses the artwork handlers). @@ -2047,7 +2047,7 @@ queue_item_to_json(struct db_queue_item *queue_item, char shuffle) } else { - // Pipe metadata can change if the queue version changes. Construct artwork url + // Pipe and stream metadata can change if the queue version changes. Construct artwork url // similar to non-pipe items, but append the queue version to the url to force // clients to reload image if the queue version changes (additional metadata was found). ret = snprintf(artwork_url, sizeof(artwork_url), "/artwork/item/%d?v=%d", queue_item->file_id, queue_item->queue_version); From afa1a07a424d4d33ef382d869bec105954a83b55 Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Fri, 21 Feb 2020 19:51:35 +0100 Subject: [PATCH 14/17] [artwork] Fix handling of cache + enable online srcs via config * Don't save artwork for permanent items (file + Spotify) to the stash. The stash is only for short term artwork. * If a request comes with a different max_w/max then search the online source again. * Make artwork requests thread-safe by mutex protecting the search history. * Add config option --- src/artwork.c | 186 ++++++++++++++++++++++++++++++++++--------------- src/conffile.c | 1 + src/settings.c | 8 +-- src/settings.h | 2 +- 4 files changed, 135 insertions(+), 62 deletions(-) diff --git a/src/artwork.c b/src/artwork.c index 4fbf6781..cd5fbdde 100644 --- a/src/artwork.c +++ b/src/artwork.c @@ -30,6 +30,7 @@ #include #include #include +#include #include "db.h" #include "misc.h" @@ -78,6 +79,7 @@ enum artwork_cache NEVER = 0, // No caching of any results ON_SUCCESS = 1, // Cache if artwork found ON_FAILURE = 2, // Cache if artwork not found (so we don't keep asking) + STASH = 4, // Only cache "short term": Cache holds just one image, useful for streams }; /* This struct contains the data available to the handler, as well as a char @@ -87,7 +89,7 @@ enum artwork_cache * changes. */ struct artwork_ctx { - // Handler should output path here if artwork is local + // Handler should output path or URL to artwork here char path[PATH_MAX]; // Handler should output artwork data to this evbuffer struct evbuffer *evbuf; @@ -139,25 +141,31 @@ enum parse_result { struct online_source { // Name of online source - char *name; + const char *name; + const bool is_default; + + const char *setting_name; // How to authorize (using the Authorize http header) - char *auth_header; - char *auth_key; - char *auth_secret; + const char *auth_header; + const char *auth_key; + const char *auth_secret; // How to search for artwork - char *search_endpoint; - char *search_param; + const char *search_endpoint; + const char *search_param; struct { const char *key; const char *template; } query_parts[8]; - // Cache previous artwork searches, so we can avoid futile requests + // Remember previous artwork searches, used to avoid futile requests struct { + pthread_mutex_t mutex; int last_id; uint32_t last_hash; + int last_max_w; + int last_max_h; int last_response_code; char *last_artwork_url; time_t last_timestamp; @@ -245,7 +253,7 @@ static struct artwork_source artwork_item_source[] = .name = "stream", .handler = source_item_stream_get, .data_kinds = (1 << DATA_KIND_HTTP), - .cache = NEVER, + .cache = STASH, }, { .name = "pipe", @@ -281,7 +289,7 @@ static struct artwork_source artwork_item_source[] = .name = "Spotify search web api (streams)", .handler = source_item_spotifywebapi_search_get, .data_kinds = (1 << DATA_KIND_HTTP) | (1 << DATA_KIND_PIPE), - .cache = NEVER, + .cache = STASH, }, { .name = "Discogs (files)", @@ -293,7 +301,7 @@ static struct artwork_source artwork_item_source[] = .name = "Discogs (streams)", .handler = source_item_discogs_get, .data_kinds = (1 << DATA_KIND_HTTP) | (1 << DATA_KIND_PIPE), - .cache = NEVER, + .cache = STASH, }, { // The Cover Art Archive seems rather slow, so low priority @@ -307,7 +315,7 @@ static struct artwork_source artwork_item_source[] = .name = "Cover Art Archive (streams)", .handler = source_item_coverartarchive_get, .data_kinds = (1 << DATA_KIND_HTTP) | (1 << DATA_KIND_PIPE), - .cache = NEVER, + .cache = STASH, }, { .name = NULL, @@ -326,6 +334,8 @@ static enum parse_result response_jparse_musicbrainz(char **artwork_url, json_ob static struct online_source spotify_source = { .name = "Spotify", + .is_default = true, + .setting_name = "use_artwork_source_spotify", .auth_header = "Bearer $SECRET$", .search_endpoint = "https://api.spotify.com/v1/search", .search_param = "type=track&limit=1&$QUERY$", @@ -336,11 +346,14 @@ static struct online_source spotify_source = { NULL, NULL }, }, .response_jparse = response_jparse_spotify, + .search_history = { .mutex = PTHREAD_MUTEX_INITIALIZER }, }; static struct online_source discogs_source = { .name = "Discogs", + .is_default = false, + .setting_name = "use_artwork_source_discogs", .auth_header = "Discogs key=$KEY$, secret=$SECRET$", .auth_key = "ivzUxlkUiwpptDKpSCHF", .auth_secret = "CYLZyExtlznKCupoIIhTpHVDReLunhUo", @@ -354,11 +367,14 @@ static struct online_source discogs_source = { NULL, NULL }, }, .response_jparse = response_jparse_discogs, + .search_history = { .mutex = PTHREAD_MUTEX_INITIALIZER }, }; static struct online_source musicbrainz_source = { .name = "Musicbrainz", + .is_default = false, + .setting_name = "use_artwork_source_coverartarchive", .search_endpoint = "http://musicbrainz.org/ws/2/release-group/", .search_param = "limit=1&fmt=json&$QUERY$", .query_parts = @@ -368,6 +384,7 @@ static struct online_source musicbrainz_source = { NULL, NULL }, }, .response_jparse = response_jparse_musicbrainz, + .search_history = { .mutex = PTHREAD_MUTEX_INITIALIZER }, }; @@ -829,8 +846,8 @@ artwork_get_bydir(struct evbuffer *evbuf, char *dir, int max_w, int max_h, char return artwork_get(evbuf, path, NULL, max_w, max_h, false); } -/* Retrieves artwork from an URL. Will use cache as appropriate, and will - * rescale if needed. +/* Retrieves artwork from an URL, will rescale if needed. Checks the cache stash + * before making a request. * * @out artwork Image data * @in url URL of the artwork @@ -852,7 +869,7 @@ artwork_get_byurl(struct evbuffer *artwork, const char *url, int max_w, int max_ { ret = artwork_evbuf_rescale(artwork, raw, max_w, max_h); if (ret < 0) - goto error; + goto error; evbuffer_free(raw); return format; @@ -862,8 +879,6 @@ artwork_get_byurl(struct evbuffer *artwork, const char *url, int max_w, int max_ if (format < 0) goto error; - cache_artwork_stash(raw, url, format); - ret = artwork_evbuf_rescale(artwork, raw, max_w, max_h); if (ret < 0) goto error; @@ -1092,32 +1107,59 @@ online_source_request_url_make(char *url, size_t url_size, struct online_source return -1; } +static char * +online_source_search_check_last(struct online_source *src, uint32_t hash, int max_w, int max_h) +{ + char *last_artwork_url = NULL; + bool is_same; + + pthread_mutex_lock(&src->search_history.mutex); + + is_same = (hash == src->search_history.last_hash) && + (max_w == src->search_history.last_max_w) && + (max_h == src->search_history.last_max_h); + + if (is_same) + last_artwork_url = safe_strdup(src->search_history.last_artwork_url); + + pthread_mutex_unlock(&src->search_history.mutex); + + return last_artwork_url; +} + static bool online_source_is_failing(struct online_source *src, int id) { + bool is_failing; + + pthread_mutex_lock(&src->search_history.mutex); + // If the last request was more than ONLINE_SEARCH_COOLDOWN_TIME ago we will always try again if (time(NULL) > src->search_history.last_timestamp + ONLINE_SEARCH_COOLDOWN_TIME) - return false; - + is_failing = false; // We won't try again if the source was not replying as expected - if (src->search_history.last_response_code != HTTP_OK) - return true; - + else if (src->search_history.last_response_code != HTTP_OK) + is_failing = true; // The playback source has changed since the last search, let's give it a chance // (internet streams can feed us with garbage search metadata, but will not change id) - if (id != src->search_history.last_id) - return false; - + else if (id != src->search_history.last_id) + is_failing = false; // We allow up to ONLINE_SEARCH_FAILURES_MAX for the same track id before declaring failure - if (src->search_history.count_failures < ONLINE_SEARCH_FAILURES_MAX) - return false; + else if (src->search_history.count_failures < ONLINE_SEARCH_FAILURES_MAX) + is_failing = false; + else + is_failing = true; - return true; + pthread_mutex_unlock(&src->search_history.mutex); + + return is_failing; } static void online_source_history_update(struct online_source *src, int id, uint32_t request_hash, int response_code, const char *artwork_url) { + pthread_mutex_lock(&src->search_history.mutex); + src->search_history.last_id = id; src->search_history.last_hash = request_hash; src->search_history.last_response_code = response_code; @@ -1130,6 +1172,8 @@ online_source_history_update(struct online_source *src, int id, uint32_t request src->search_history.count_failures = 0; else src->search_history.count_failures++; + + pthread_mutex_unlock(&src->search_history.mutex); } static char * @@ -1143,6 +1187,8 @@ online_source_search(struct online_source *src, struct artwork_ctx *ctx) char auth_header[256]; int ret; + DPRINTF(E_SPAM, L_ART, "Trying %s for %s\n", src->name, ctx->dbmfi->path); + ret = online_source_request_url_make(url, sizeof(url), src, ctx); if (ret < 0) { @@ -1152,11 +1198,9 @@ online_source_search(struct online_source *src, struct artwork_ctx *ctx) // Be nice to our peer + improve response times by not repeating search requests hash = djb_hash(url, strlen(url)); - if (hash == src->search_history.last_hash) - { - artwork_url = safe_strdup(src->search_history.last_artwork_url); - return artwork_url; - } + artwork_url = online_source_search_check_last(src, hash, ctx->max_w, ctx->max_h); + if (artwork_url) + return artwork_url; // If our recent searches have been futile we may give the source a break if (online_source_is_failing(src, ctx->id)) @@ -1214,20 +1258,37 @@ online_source_search(struct online_source *src, struct artwork_ctx *ctx) } static bool -online_source_is_enabled(const char *setting_name) +online_source_is_enabled(struct online_source *src) { struct settings_category *category; - struct settings_option *option; + cfg_t *lib; + const char *name; + int n_cfg; + int i; - category = settings_category_get("artwork"); - if (!category) - return false; + lib = cfg_getsec(cfg, "library"); - option = settings_option_get(category, setting_name); - if (!option) - return false; + n_cfg = cfg_size(lib, "artwork_online_sources"); + if (n_cfg == 0) // User didn't set anything in the config file, use settings + { + category = settings_category_get("artwork"); + if (!category) + return src->is_default; - return settings_option_getbool(option); +// if (!settings_option_is_set(category, src->setting_name)) +// return src->is_default; + + return settings_option_getbool(settings_option_get(category, src->setting_name)); + } + + for (i = 0; i < n_cfg; i++) + { + name = cfg_getnstr(lib, "artwork_online_sources", i); + if (strcasecmp(name, src->name) == 0) + return true; + } + + return src->is_default; } @@ -1479,13 +1540,15 @@ source_item_discogs_get(struct artwork_ctx *ctx) char *url; int ret; - if (!online_source_is_enabled("enable_discogs")) + if (!online_source_is_enabled(&discogs_source)) return ART_E_NONE; url = online_source_search(&discogs_source, ctx); if (!url) return ART_E_NONE; + snprintf(ctx->path, sizeof(ctx->path), "%s", url); + ret = artwork_get_byurl(ctx->evbuf, url, ctx->max_w, ctx->max_h); free(url); @@ -1498,7 +1561,7 @@ source_item_coverartarchive_get(struct artwork_ctx *ctx) char *url; int ret; - if (!online_source_is_enabled("enable_coverartarchive")) + if (!online_source_is_enabled(&musicbrainz_source)) return ART_E_NONE; // We search Musicbrainz to get the Musicbrainz ID, which we need to get the @@ -1507,7 +1570,8 @@ source_item_coverartarchive_get(struct artwork_ctx *ctx) if (!url) return ART_E_NONE; - // TODO add support in http client for 307 redirects + snprintf(ctx->path, sizeof(ctx->path), "%s", url); + ret = artwork_get_byurl(ctx->evbuf, url, ctx->max_w, ctx->max_h); free(url); @@ -1526,7 +1590,7 @@ source_item_libspotify_get(struct artwork_ctx *ctx) ret = spotify_artwork_get(raw, ctx->dbmfi->path, ctx->max_w, ctx->max_h); if (ret < 0) { - DPRINTF(E_WARN, L_ART, "No artwork from Spotify for %s\n", ctx->dbmfi->path); + DPRINTF(E_WARN, L_ART, "No artwork from libspotify for %s\n", ctx->dbmfi->path); evbuffer_free(raw); return ART_E_NONE; } @@ -1534,7 +1598,7 @@ source_item_libspotify_get(struct artwork_ctx *ctx) ret = artwork_evbuf_rescale(ctx->evbuf, raw, ctx->max_w, ctx->max_h); if (ret < 0) { - DPRINTF(E_LOG, L_ART, "Could not rescale Spotify artwork for '%s'\n", ctx->dbmfi->path); + DPRINTF(E_LOG, L_ART, "Could not rescale libspotify artwork for '%s'\n", ctx->dbmfi->path); evbuffer_free(raw); return ART_E_ERROR; } @@ -1569,7 +1633,7 @@ source_item_spotifywebapi_search_get(struct artwork_ctx *ctx) char *url; int ret; - if (!online_source_is_enabled("enable_spotify")) + if (!online_source_is_enabled(&spotify_source)) return ART_E_NONE; spotifywebapi_access_token_get(&info); @@ -1583,6 +1647,8 @@ source_item_spotifywebapi_search_get(struct artwork_ctx *ctx) if (!url) return ART_E_NONE; + snprintf(ctx->path, sizeof(ctx->path), "%s", url); + ret = artwork_get_byurl(ctx->evbuf, url, ctx->max_w, ctx->max_h); free(url); @@ -1719,7 +1785,7 @@ process_items(struct artwork_ctx *ctx, int item_mode) if (ret > 0) { DPRINTF(E_DBG, L_ART, "Artwork for '%s' found in source '%s'\n", dbmfi.title, artwork_item_source[i].name); - ctx->cache = (artwork_item_source[i].cache & ON_SUCCESS); + ctx->cache = artwork_item_source[i].cache; db_query_end(&ctx->qp); return ret; } @@ -1774,7 +1840,7 @@ process_group(struct artwork_ctx *ctx) if (ret > 0) { DPRINTF(E_DBG, L_ART, "Artwork for group %" PRIi64 " found in source '%s'\n", ctx->persistentid, artwork_group_source[i].name); - ctx->cache = (artwork_group_source[i].cache & ON_SUCCESS); + ctx->cache = artwork_group_source[i].cache; return ret; } else if (ret == ART_E_ABORT) @@ -1805,7 +1871,7 @@ artwork_get_item(struct evbuffer *evbuf, int id, int max_w, int max_h) char filter[32]; int ret; - DPRINTF(E_DBG, L_ART, "Artwork request for item %d\n", id); + DPRINTF(E_DBG, L_ART, "Artwork request for item %d (max_w=%d, max_h=%d)\n", id, max_w, max_h); if (id == DB_MEDIA_FILE_NON_PERSISTENT_ID) return -1; @@ -1832,8 +1898,10 @@ artwork_get_item(struct evbuffer *evbuf, int id, int max_w, int max_h) ret = process_items(&ctx, 1); if (ret > 0) { - if (ctx.cache == ON_SUCCESS) + if (ctx.cache & ON_SUCCESS) cache_artwork_add(CACHE_ARTWORK_INDIVIDUAL, id, max_w, max_h, ret, ctx.path, evbuf); + if (ctx.cache & STASH) + cache_artwork_stash(evbuf, ctx.path, ret); return ret; } @@ -1844,15 +1912,17 @@ artwork_get_item(struct evbuffer *evbuf, int id, int max_w, int max_h) ret = process_group(&ctx); if (ret > 0) { - if (ctx.cache == ON_SUCCESS) + if (ctx.cache & ON_SUCCESS) cache_artwork_add(CACHE_ARTWORK_GROUP, ctx.persistentid, max_w, max_h, ret, ctx.path, evbuf); + if (ctx.cache & STASH) + cache_artwork_stash(evbuf, ctx.path, ret); return ret; } DPRINTF(E_DBG, L_ART, "No artwork found for item %d\n", id); - if (ctx.cache == ON_FAILURE) + if (ctx.cache & ON_FAILURE) cache_artwork_add(CACHE_ARTWORK_GROUP, ctx.persistentid, max_w, max_h, 0, "", evbuf); return -1; @@ -1864,7 +1934,7 @@ artwork_get_group(struct evbuffer *evbuf, int id, int max_w, int max_h) struct artwork_ctx ctx; int ret; - DPRINTF(E_DBG, L_ART, "Artwork request for group %d\n", id); + DPRINTF(E_DBG, L_ART, "Artwork request for group %d (max_w=%d, max_h=%d)\n", id, max_w, max_h); memset(&ctx, 0, sizeof(struct artwork_ctx)); @@ -1887,15 +1957,17 @@ artwork_get_group(struct evbuffer *evbuf, int id, int max_w, int max_h) ret = process_group(&ctx); if (ret > 0) { - if (ctx.cache == ON_SUCCESS) + if (ctx.cache & ON_SUCCESS) cache_artwork_add(CACHE_ARTWORK_GROUP, ctx.persistentid, max_w, max_h, ret, ctx.path, evbuf); + if (ctx.cache & STASH) + cache_artwork_stash(evbuf, ctx.path, ret); return ret; } DPRINTF(E_DBG, L_ART, "No artwork found for group %d\n", id); - if (ctx.cache == ON_FAILURE) + if (ctx.cache & ON_FAILURE) cache_artwork_add(CACHE_ARTWORK_GROUP, ctx.persistentid, max_w, max_h, 0, "", evbuf); return -1; diff --git a/src/conffile.c b/src/conffile.c index 4e40405c..650380f1 100644 --- a/src/conffile.c +++ b/src/conffile.c @@ -92,6 +92,7 @@ static cfg_opt_t sec_library[] = CFG_STR("name_radio", "Radio", CFGF_NONE), CFG_STR_LIST("artwork_basenames", "{artwork,cover,Folder}", CFGF_NONE), CFG_BOOL("artwork_individual", cfg_false, CFGF_NONE), + CFG_STR_LIST("artwork_online_sources", NULL, CFGF_NONE), CFG_STR_LIST("filetypes_ignore", "{.db,.ini,.db-journal,.pdf,.metadata}", CFGF_NONE), CFG_STR_LIST("filepath_ignore", NULL, CFGF_NONE), CFG_BOOL("filescan_disable", cfg_false, CFGF_NONE), diff --git a/src/settings.c b/src/settings.c index 8e2f89d5..699b8431 100644 --- a/src/settings.c +++ b/src/settings.c @@ -33,9 +33,9 @@ static struct settings_option webinterface_options[] = static struct settings_option artwork_options[] = { - { "enable_spotify", SETTINGS_TYPE_BOOL }, - { "enable_discogs", SETTINGS_TYPE_BOOL }, - { "enable_coverartarchive", SETTINGS_TYPE_BOOL }, + { "use_artwork_source_spotify", SETTINGS_TYPE_BOOL }, + { "use_artwork_source_discogs", SETTINGS_TYPE_BOOL }, + { "use_artwork_source_coverartarchive", SETTINGS_TYPE_BOOL }, }; static struct settings_category categories[] = @@ -105,6 +105,7 @@ settings_option_get(struct settings_category *category, const char *name) return NULL; } + int settings_option_getint(struct settings_option *option) { @@ -114,7 +115,6 @@ settings_option_getint(struct settings_option *option) return db_admin_getint(option->name); } - bool settings_option_getbool(struct settings_option *option) { diff --git a/src/settings.h b/src/settings.h index bc4f7b24..0d972608 100644 --- a/src/settings.h +++ b/src/settings.h @@ -42,10 +42,10 @@ settings_option_get_byindex(struct settings_category *category, int index); struct settings_option * settings_option_get(struct settings_category *category, const char *name); + int settings_option_getint(struct settings_option *option); - bool settings_option_getbool(struct settings_option *option); From 5736217315c517c0b3e23826fdae19c94284ec3d Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Sat, 22 Feb 2020 10:03:13 +0100 Subject: [PATCH 15/17] [db] Change prototype of db_admin_getxxx() functions Makes it possible for caller to distinguish between "not set" and "set to 0". --- src/db.c | 70 +++++++++------------------------- src/db.h | 10 ++--- src/httpd.c | 6 +-- src/httpd.h | 2 +- src/httpd_jsonapi.c | 91 ++++++++++++++++++-------------------------- src/lastfm.c | 6 ++- src/mpd.c | 14 +++---- src/settings.c | 18 +++++++-- src/spotify_webapi.c | 4 +- 9 files changed, 91 insertions(+), 130 deletions(-) diff --git a/src/db.c b/src/db.c index 3743a360..9fd0fe60 100644 --- a/src/db.c +++ b/src/db.c @@ -4353,7 +4353,7 @@ db_admin_setint64(const char *key, int64_t value) } static int -admin_get(const char *key, short type, void *value) +admin_get(void *value, const char *key, short type) { #define Q_TMPL "SELECT value FROM admin a WHERE a.key = '%q';" char *query; @@ -4364,13 +4364,7 @@ admin_get(const char *key, short type, void *value) char **strval; int ret; - query = sqlite3_mprintf(Q_TMPL, key); - if (!query) - { - DPRINTF(E_LOG, L_DB, "Out of memory for query string\n"); - - return -1; - } + CHECK_NULL(L_DB, query = sqlite3_mprintf(Q_TMPL, key)); DPRINTF(E_DBG, L_DB, "Running query '%s'\n", query); @@ -4437,52 +4431,22 @@ admin_get(const char *key, short type, void *value) #undef Q_TMPL } -char * -db_admin_get(const char *key) +int +db_admin_get(char **value, const char *key) { - char *value = NULL; - int ret; - - ret = admin_get(key, DB_TYPE_STRING, &value); - if (ret < 0) - { - DPRINTF(E_DBG, L_DB, "Could not find key '%s' in admin table\n", key); - return NULL; - } - - return value; + return admin_get(value, key, DB_TYPE_STRING); } int -db_admin_getint(const char *key) +db_admin_getint(int *intval, const char *key) { - int value = 0; - int ret; - - ret = admin_get(key, DB_TYPE_INT, &value); - if (ret < 0) - { - DPRINTF(E_DBG, L_DB, "Could not find key '%s' in admin table\n", key); - return 0; - } - - return value; + return admin_get(intval, key, DB_TYPE_INT); } -int64_t -db_admin_getint64(const char *key) +int +db_admin_getint64(int64_t *int64val, const char *key) { - int64_t value = 0; - int ret; - - ret = admin_get(key, DB_TYPE_INT64, &value); - if (ret < 0) - { - DPRINTF(E_DBG, L_DB, "Could not find key '%s' in admin table\n", key); - return 0; - } - - return value; + return admin_get(int64val, key, DB_TYPE_INT64); } int @@ -4586,11 +4550,11 @@ db_speaker_clear_all(void) static int queue_transaction_begin() { - int queue_version; + int queue_version = 0; db_transaction_begin(); - queue_version = db_admin_getint(DB_ADMIN_QUEUE_VERSION); + db_admin_getint(&queue_version, DB_ADMIN_QUEUE_VERSION); queue_version++; return queue_version; @@ -6938,22 +6902,22 @@ db_check_version(void) { #define Q_VACUUM "VACUUM;" char *errmsg; - int db_ver_major; - int db_ver_minor; + int db_ver_major = 0; + int db_ver_minor = 0; int db_ver; int vacuum; int ret; vacuum = cfg_getbool(cfg_getsec(cfg, "sqlite"), "vacuum"); - db_ver_major = db_admin_getint(DB_ADMIN_SCHEMA_VERSION_MAJOR); + db_admin_getint(&db_ver_major, DB_ADMIN_SCHEMA_VERSION_MAJOR); if (!db_ver_major) - db_ver_major = db_admin_getint(DB_ADMIN_SCHEMA_VERSION); // Pre schema v15.1 + db_admin_getint(&db_ver_major, DB_ADMIN_SCHEMA_VERSION); // Pre schema v15.1 if (!db_ver_major) return 1; // Will create new database - db_ver_minor = db_admin_getint(DB_ADMIN_SCHEMA_VERSION_MINOR); + db_admin_getint(&db_ver_minor, DB_ADMIN_SCHEMA_VERSION_MINOR); db_ver = db_ver_major * 100 + db_ver_minor; diff --git a/src/db.h b/src/db.h index e1c52604..c900057e 100644 --- a/src/db.h +++ b/src/db.h @@ -771,14 +771,14 @@ db_admin_setint(const char *key, int value); int db_admin_setint64(const char *key, int64_t value); -char * -db_admin_get(const char *key); +int +db_admin_get(char **value, const char *key); int -db_admin_getint(const char *key); +db_admin_getint(int *intval, const char *key); -int64_t -db_admin_getint64(const char *key); +int +db_admin_getint64(int64_t *int64val, const char *key); int db_admin_delete(const char *key); diff --git a/src/httpd.c b/src/httpd.c index 4a3ce341..817bfe52 100644 --- a/src/httpd.c +++ b/src/httpd.c @@ -319,7 +319,7 @@ httpd_request_etag_matches(struct evhttp_request *req, const char *etag) * @return True if the given timestamp matches the request-header-value "If-Modified-Since", otherwise false */ bool -httpd_request_not_modified_since(struct evhttp_request *req, const time_t *mtime) +httpd_request_not_modified_since(struct evhttp_request *req, time_t mtime) { struct evkeyvalq *input_headers; struct evkeyvalq *output_headers; @@ -329,7 +329,7 @@ httpd_request_not_modified_since(struct evhttp_request *req, const time_t *mtime input_headers = evhttp_request_get_input_headers(req); modified_since = evhttp_find_header(input_headers, "If-Modified-Since"); - strftime(last_modified, sizeof(last_modified), "%a, %d %b %Y %H:%M:%S %Z", gmtime(mtime)); + strftime(last_modified, sizeof(last_modified), "%a, %d %b %Y %H:%M:%S %Z", gmtime(&mtime)); // Return not modified, if given timestamp matches "If-Modified-Since" request header if (modified_since && (strcasecmp(last_modified, modified_since) == 0)) @@ -455,7 +455,7 @@ serve_file(struct evhttp_request *req, const char *uri) return; } - if (httpd_request_not_modified_since(req, &sb.st_mtime)) + if (httpd_request_not_modified_since(req, sb.st_mtime)) { httpd_send_reply(req, HTTP_NOTMODIFIED, NULL, NULL, HTTPD_SEND_NO_GZIP); return; diff --git a/src/httpd.h b/src/httpd.h index 16253027..de295442 100644 --- a/src/httpd.h +++ b/src/httpd.h @@ -102,7 +102,7 @@ void httpd_stream_file(struct evhttp_request *req, int id); bool -httpd_request_not_modified_since(struct evhttp_request *req, const time_t *mtime); +httpd_request_not_modified_since(struct evhttp_request *req, time_t mtime); bool httpd_request_etag_matches(struct evhttp_request *req, const char *etag); diff --git a/src/httpd_jsonapi.c b/src/httpd_jsonapi.c index 4a263a51..0787d777 100644 --- a/src/httpd_jsonapi.c +++ b/src/httpd_jsonapi.c @@ -67,6 +67,16 @@ static char *default_playlist_directory; /* -------------------------------- HELPERS --------------------------------- */ +static bool +is_modified(struct evhttp_request *req, const char *key) +{ + int64_t db_update = 0; + + db_admin_getint64(&db_update, key); + + return (!db_update || !httpd_request_not_modified_since(req, (time_t)db_update)); +} + static inline void safe_json_add_string(json_object *obj, const char *key, const char *value) { @@ -1023,10 +1033,19 @@ jsonapi_reply_library(struct httpd_request *hreq) DPRINTF(E_LOG, L_WEB, "library: failed to get file count info\n"); } - safe_json_add_time_from_string(jreply, "started_at", (s = db_admin_get(DB_ADMIN_START_TIME)), true); - free(s); - safe_json_add_time_from_string(jreply, "updated_at", (s = db_admin_get(DB_ADMIN_DB_UPDATE)), true); - free(s); + ret = db_admin_get(&s, DB_ADMIN_START_TIME); + if (ret == 0) + { + safe_json_add_time_from_string(jreply, "started_at", s, true); + free(s); + } + + ret = db_admin_get(&s, DB_ADMIN_DB_UPDATE); + if (ret == 0) + { + safe_json_add_time_from_string(jreply, "updated_at", s, true); + free(s); + } json_object_object_add(jreply, "updating", json_object_new_boolean(library_is_scanning())); @@ -2458,7 +2477,7 @@ jsonapi_reply_queue(struct httpd_request *hreq) uint32_t item_id; uint32_t count; int start_pos, end_pos; - int version; + int version = 0; char etag[21]; struct player_status status; struct db_queue_item queue_item; @@ -2467,7 +2486,7 @@ jsonapi_reply_queue(struct httpd_request *hreq) json_object *item; int ret = 0; - version = db_admin_getint(DB_ADMIN_QUEUE_VERSION); + db_admin_getint(&version, DB_ADMIN_QUEUE_VERSION); db_queue_get_count(&count); snprintf(etag, sizeof(etag), "%d", version); @@ -2715,7 +2734,6 @@ jsonapi_reply_player_volume(struct httpd_request *hreq) static int jsonapi_reply_library_artists(struct httpd_request *hreq) { - time_t db_update; struct query_params query_params; const char *param; enum media_kind media_kind; @@ -2724,11 +2742,9 @@ jsonapi_reply_library_artists(struct httpd_request *hreq) int total; int ret = 0; - db_update = (time_t) db_admin_getint64(DB_ADMIN_DB_UPDATE); - if (db_update && httpd_request_not_modified_since(hreq->req, &db_update)) + if (!is_modified(hreq->req, DB_ADMIN_DB_UPDATE)) return HTTP_NOTMODIFIED; - media_kind = 0; param = evhttp_find_header(hreq->query, "media_kind"); if (param) @@ -2782,16 +2798,13 @@ jsonapi_reply_library_artists(struct httpd_request *hreq) static int jsonapi_reply_library_artist(struct httpd_request *hreq) { - time_t db_update; const char *artist_id; json_object *reply; int ret = 0; - db_update = (time_t) db_admin_getint64(DB_ADMIN_DB_UPDATE); - if (db_update && httpd_request_not_modified_since(hreq->req, &db_update)) + if (!is_modified(hreq->req, DB_ADMIN_DB_UPDATE)) return HTTP_NOTMODIFIED; - artist_id = hreq->uri_parsed->path_parts[3]; reply = fetch_artist(artist_id); @@ -2817,7 +2830,6 @@ jsonapi_reply_library_artist(struct httpd_request *hreq) static int jsonapi_reply_library_artist_albums(struct httpd_request *hreq) { - time_t db_update; struct query_params query_params; const char *artist_id; json_object *reply; @@ -2825,11 +2837,9 @@ jsonapi_reply_library_artist_albums(struct httpd_request *hreq) int total; int ret = 0; - db_update = (time_t) db_admin_getint64(DB_ADMIN_DB_UPDATE); - if (db_update && httpd_request_not_modified_since(hreq->req, &db_update)) + if (!is_modified(hreq->req, DB_ADMIN_DB_UPDATE)) return HTTP_NOTMODIFIED; - artist_id = hreq->uri_parsed->path_parts[3]; reply = json_object_new_object(); @@ -2872,7 +2882,6 @@ jsonapi_reply_library_artist_albums(struct httpd_request *hreq) static int jsonapi_reply_library_albums(struct httpd_request *hreq) { - time_t db_update; struct query_params query_params; const char *param; enum media_kind media_kind; @@ -2881,11 +2890,9 @@ jsonapi_reply_library_albums(struct httpd_request *hreq) int total; int ret = 0; - db_update = (time_t) db_admin_getint64(DB_ADMIN_DB_UPDATE); - if (db_update && httpd_request_not_modified_since(hreq->req, &db_update)) + if (!is_modified(hreq->req, DB_ADMIN_DB_UPDATE)) return HTTP_NOTMODIFIED; - media_kind = 0; param = evhttp_find_header(hreq->query, "media_kind"); if (param) @@ -2939,16 +2946,13 @@ jsonapi_reply_library_albums(struct httpd_request *hreq) static int jsonapi_reply_library_album(struct httpd_request *hreq) { - time_t db_update; const char *album_id; json_object *reply; int ret = 0; - db_update = (time_t) db_admin_getint64(DB_ADMIN_DB_UPDATE); - if (db_update && httpd_request_not_modified_since(hreq->req, &db_update)) + if (!is_modified(hreq->req, DB_ADMIN_DB_UPDATE)) return HTTP_NOTMODIFIED; - album_id = hreq->uri_parsed->path_parts[3]; reply = fetch_album(album_id); @@ -2974,7 +2978,6 @@ jsonapi_reply_library_album(struct httpd_request *hreq) static int jsonapi_reply_library_album_tracks(struct httpd_request *hreq) { - time_t db_update; struct query_params query_params; const char *album_id; json_object *reply; @@ -2982,11 +2985,9 @@ jsonapi_reply_library_album_tracks(struct httpd_request *hreq) int total; int ret = 0; - db_update = (time_t) db_admin_getint64(DB_ADMIN_DB_MODIFIED); - if (db_update && httpd_request_not_modified_since(hreq->req, &db_update)) + if (!is_modified(hreq->req, DB_ADMIN_DB_MODIFIED)) return HTTP_NOTMODIFIED; - album_id = hreq->uri_parsed->path_parts[3]; reply = json_object_new_object(); @@ -3029,18 +3030,15 @@ jsonapi_reply_library_album_tracks(struct httpd_request *hreq) static int jsonapi_reply_library_tracks_get_byid(struct httpd_request *hreq) { - time_t db_update; struct query_params query_params; const char *track_id; struct db_media_file_info dbmfi; json_object *reply = NULL; int ret = 0; - db_update = (time_t) db_admin_getint64(DB_ADMIN_DB_MODIFIED); - if (db_update && httpd_request_not_modified_since(hreq->req, &db_update)) + if (!is_modified(hreq->req, DB_ADMIN_DB_MODIFIED)) return HTTP_NOTMODIFIED; - track_id = hreq->uri_parsed->path_parts[3]; memset(&query_params, 0, sizeof(struct query_params)); @@ -3133,18 +3131,15 @@ jsonapi_reply_library_tracks_put_byid(struct httpd_request *hreq) static int jsonapi_reply_library_playlists(struct httpd_request *hreq) { - time_t db_update; struct query_params query_params; json_object *reply; json_object *items; int total; int ret = 0; - db_update = (time_t) db_admin_getint64(DB_ADMIN_DB_UPDATE); - if (db_update && httpd_request_not_modified_since(hreq->req, &db_update)) + if (!is_modified(hreq->req, DB_ADMIN_DB_UPDATE)) return HTTP_NOTMODIFIED; - reply = json_object_new_object(); items = json_object_new_array(); json_object_object_add(reply, "items", items); @@ -3185,16 +3180,13 @@ jsonapi_reply_library_playlists(struct httpd_request *hreq) static int jsonapi_reply_library_playlist(struct httpd_request *hreq) { - time_t db_update; const char *playlist_id; json_object *reply; int ret = 0; - db_update = (time_t) db_admin_getint64(DB_ADMIN_DB_UPDATE); - if (db_update && httpd_request_not_modified_since(hreq->req, &db_update)) + if (!is_modified(hreq->req, DB_ADMIN_DB_UPDATE)) return HTTP_NOTMODIFIED; - playlist_id = hreq->uri_parsed->path_parts[3]; reply = fetch_playlist(playlist_id); @@ -3220,7 +3212,6 @@ jsonapi_reply_library_playlist(struct httpd_request *hreq) static int jsonapi_reply_library_playlist_tracks(struct httpd_request *hreq) { - time_t db_update; struct query_params query_params; json_object *reply; json_object *items; @@ -3228,11 +3219,9 @@ jsonapi_reply_library_playlist_tracks(struct httpd_request *hreq) int total; int ret = 0; - db_update = (time_t) db_admin_getint64(DB_ADMIN_DB_MODIFIED); - if (db_update && httpd_request_not_modified_since(hreq->req, &db_update)) + if (!is_modified(hreq->req, DB_ADMIN_DB_MODIFIED)) return HTTP_NOTMODIFIED; - ret = safe_atoi32(hreq->uri_parsed->path_parts[3], &playlist_id); if (ret < 0) { @@ -3325,7 +3314,6 @@ jsonapi_reply_queue_save(struct httpd_request *hreq) static int jsonapi_reply_library_genres(struct httpd_request *hreq) { - time_t db_update; struct query_params query_params; const char *param; enum media_kind media_kind; @@ -3334,9 +3322,7 @@ jsonapi_reply_library_genres(struct httpd_request *hreq) int total; int ret; - - db_update = (time_t) db_admin_getint64(DB_ADMIN_DB_UPDATE); - if (db_update && httpd_request_not_modified_since(hreq->req, &db_update)) + if (!is_modified(hreq->req, DB_ADMIN_DB_UPDATE)) return HTTP_NOTMODIFIED; media_kind = 0; @@ -3394,7 +3380,6 @@ jsonapi_reply_library_genres(struct httpd_request *hreq) static int jsonapi_reply_library_count(struct httpd_request *hreq) { - time_t db_update; const char *param_expression; char *expression; struct smartpl smartpl_expression; @@ -3403,9 +3388,7 @@ jsonapi_reply_library_count(struct httpd_request *hreq) json_object *jreply; int ret; - - db_update = (time_t) db_admin_getint64(DB_ADMIN_DB_UPDATE); - if (db_update && httpd_request_not_modified_since(hreq->req, &db_update)) + if (!is_modified(hreq->req, DB_ADMIN_DB_UPDATE)) return HTTP_NOTMODIFIED; memset(&qp, 0, sizeof(struct query_params)); diff --git a/src/lastfm.c b/src/lastfm.c index 6e18620b..52895ede 100644 --- a/src/lastfm.c +++ b/src/lastfm.c @@ -422,8 +422,10 @@ lastfm_is_enabled(void) int lastfm_init(void) { - lastfm_session_key = db_admin_get(DB_ADMIN_LASTFM_SESSION_KEY); - if (!lastfm_session_key) + int ret; + + ret = db_admin_get(&lastfm_session_key, DB_ADMIN_LASTFM_SESSION_KEY); + if (ret < 0) { DPRINTF(E_DBG, L_LASTFM, "No valid LastFM session key\n"); lastfm_disabled = true; diff --git a/src/mpd.c b/src/mpd.c index cfbf5aad..5a411d62 100644 --- a/src/mpd.c +++ b/src/mpd.c @@ -959,7 +959,7 @@ mpd_command_status(struct evbuffer *evbuf, int argc, char **argv, char **errmsg, { struct player_status status; uint32_t queue_length = 0; - int queue_version; + int queue_version = 0; char *state; uint32_t itemid = 0; struct db_queue_item *queue_item; @@ -981,7 +981,7 @@ mpd_command_status(struct evbuffer *evbuf, int argc, char **argv, char **errmsg, break; } - queue_version = db_admin_getint(DB_ADMIN_QUEUE_VERSION); + db_admin_getint(&queue_version, DB_ADMIN_QUEUE_VERSION); db_queue_get_count(&queue_length); evbuffer_add_printf(evbuf, @@ -1062,9 +1062,9 @@ mpd_command_stats(struct evbuffer *evbuf, int argc, char **argv, char **errmsg, { struct query_params qp; struct filecount_info fci; - time_t start_time; double uptime; - int64_t db_update; + int64_t db_start = 0; + int64_t db_update = 0; int ret; memset(&qp, 0, sizeof(struct query_params)); @@ -1077,9 +1077,9 @@ mpd_command_stats(struct evbuffer *evbuf, int argc, char **argv, char **errmsg, return ACK_ERROR_UNKNOWN; } - start_time = (time_t) db_admin_getint64(DB_ADMIN_START_TIME); - uptime = difftime(time(NULL), start_time); - db_update = db_admin_getint64(DB_ADMIN_DB_UPDATE); + db_admin_getint64(&db_start, DB_ADMIN_START_TIME); + uptime = difftime(time(NULL), (time_t) db_start); + db_admin_getint64(&db_update, DB_ADMIN_DB_UPDATE); //TODO [mpd] Implement missing stats attributes (playtime) evbuffer_add_printf(evbuf, diff --git a/src/settings.c b/src/settings.c index 699b8431..a3989dcb 100644 --- a/src/settings.c +++ b/src/settings.c @@ -109,28 +109,40 @@ settings_option_get(struct settings_category *category, const char *name) int settings_option_getint(struct settings_option *option) { + int intval = 0; + if (!option || option->type != SETTINGS_TYPE_INT) return 0; - return db_admin_getint(option->name); + db_admin_getint(&intval, option->name); + + return intval; } bool settings_option_getbool(struct settings_option *option) { + int intval = 0; + if (!option || option->type != SETTINGS_TYPE_BOOL) return false; - return db_admin_getint(option->name) > 0; + db_admin_getint(&intval, option->name); + + return (intval != 0); } char * settings_option_getstr(struct settings_option *option) { + char *s = NULL; + if (!option || option->type != SETTINGS_TYPE_STR) return NULL; - return db_admin_get(option->name); + db_admin_get(&s, option->name); + + return s; } int diff --git a/src/spotify_webapi.c b/src/spotify_webapi.c index 338f9293..5cc76875 100644 --- a/src/spotify_webapi.c +++ b/src/spotify_webapi.c @@ -419,8 +419,8 @@ token_refresh(void) return 0; } - refresh_token = db_admin_get(DB_ADMIN_SPOTIFY_REFRESH_TOKEN); - if (!refresh_token) + ret = db_admin_get(&refresh_token, DB_ADMIN_SPOTIFY_REFRESH_TOKEN); + if (ret < 0) { DPRINTF(E_LOG, L_SPOTIFY, "No spotify refresh token found\n"); goto error; From 9b9d2d0fb71a6f00d769bd0836b3eb97b80cd4ed Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Sat, 22 Feb 2020 16:46:07 +0100 Subject: [PATCH 16/17] [settings] Add handlers to set default settings, e.g. from the cfg file --- src/settings.c | 90 ++++++++++++++++++++++++++++++++++++++++++++------ src/settings.h | 4 ++- 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/src/settings.c b/src/settings.c index a3989dcb..d3f9bae6 100644 --- a/src/settings.c +++ b/src/settings.c @@ -22,8 +22,12 @@ #include #include "db.h" +#include "conffile.h" - +// Forward - setting initializers +static bool artwork_spotify_default_getbool(struct settings_option *option); +static bool artwork_discogs_default_getbool(struct settings_option *option); +static bool artwork_coverartarchive_default_getbool(struct settings_option *option); static struct settings_option webinterface_options[] = { @@ -33,9 +37,9 @@ static struct settings_option webinterface_options[] = static struct settings_option artwork_options[] = { - { "use_artwork_source_spotify", SETTINGS_TYPE_BOOL }, - { "use_artwork_source_discogs", SETTINGS_TYPE_BOOL }, - { "use_artwork_source_coverartarchive", SETTINGS_TYPE_BOOL }, + { "use_artwork_source_spotify", SETTINGS_TYPE_BOOL, NULL, artwork_spotify_default_getbool, NULL }, + { "use_artwork_source_discogs", SETTINGS_TYPE_BOOL, NULL, artwork_discogs_default_getbool, NULL }, + { "use_artwork_source_coverartarchive", SETTINGS_TYPE_BOOL, NULL, artwork_coverartarchive_default_getbool, NULL }, }; static struct settings_category categories[] = @@ -45,6 +49,54 @@ static struct settings_category categories[] = }; +/* ---------------------------- DEFAULT SETTERS ------------------------------*/ + +static bool +artwork_default_getbool(bool no_cfg_default, const char *cfg_name) +{ + cfg_t *lib = cfg_getsec(cfg, "library"); + const char *name; + int n_cfg; + int i; + + n_cfg = cfg_size(lib, "artwork_online_sources"); + if (n_cfg == 0) + return no_cfg_default; + + for (i = 0; i < n_cfg; i++) + { + name = cfg_getnstr(lib, "artwork_online_sources", i); + if (strcasecmp(name, cfg_name) == 0) + return true; + } + + return false; +} + +static bool +artwork_spotify_default_getbool(struct settings_option *option) +{ + // Enabled by default, it will only work for premium users anyway. So Spotify + // probably won't mind, and the user probably also won't mind that we share + // data with Spotify, since he is already doing it. + return artwork_default_getbool(true, "spotify"); +} + +static bool +artwork_discogs_default_getbool(struct settings_option *option) +{ + return artwork_default_getbool(false, "discogs"); +} + +static bool +artwork_coverartarchive_default_getbool(struct settings_option *option) +{ + return artwork_default_getbool(false, "coverartarchive"); +} + + +/* ------------------------------ IMPLEMENTATION -----------------------------*/ + int settings_categories_count() { @@ -110,39 +162,57 @@ int settings_option_getint(struct settings_option *option) { int intval = 0; + int ret; if (!option || option->type != SETTINGS_TYPE_INT) return 0; - db_admin_getint(&intval, option->name); + ret = db_admin_getint(&intval, option->name); + if (ret == 0) + return intval; - return intval; + if (option->default_getint) + return option->default_getint(option); + + return 0; } bool settings_option_getbool(struct settings_option *option) { int intval = 0; + int ret; if (!option || option->type != SETTINGS_TYPE_BOOL) return false; - db_admin_getint(&intval, option->name); + ret = db_admin_getint(&intval, option->name); + if (ret == 0) + return (intval != 0); - return (intval != 0); + if (option->default_getbool) + return option->default_getbool(option); + + return false; } char * settings_option_getstr(struct settings_option *option) { char *s = NULL; + int ret; if (!option || option->type != SETTINGS_TYPE_STR) return NULL; - db_admin_get(&s, option->name); + ret = db_admin_get(&s, option->name); + if (ret == 0) + return s; - return s; + if (option->default_getstr) + return option->default_getstr(option); + + return NULL; } int diff --git a/src/settings.h b/src/settings.h index 0d972608..80d1a18f 100644 --- a/src/settings.h +++ b/src/settings.h @@ -15,6 +15,9 @@ enum settings_type { struct settings_option { const char *name; enum settings_type type; + int (*default_getint)(struct settings_option *option); + bool (*default_getbool)(struct settings_option *option); + char *(*default_getstr)(struct settings_option *option); }; struct settings_category { @@ -61,5 +64,4 @@ settings_option_setbool(struct settings_option *option, bool value); int settings_option_setstr(struct settings_option *option, const char *value); - #endif /* __SETTINGS_H__ */ From 1ed10771f174a0313216dd25b67da9b0c9e5eda2 Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Sat, 22 Feb 2020 16:49:00 +0100 Subject: [PATCH 17/17] [artwork] Get enablement of online srcs from settings Settings will take care of consulting the cfg file now --- src/artwork.c | 41 ++++++++--------------------------------- 1 file changed, 8 insertions(+), 33 deletions(-) diff --git a/src/artwork.c b/src/artwork.c index cd5fbdde..7e703139 100644 --- a/src/artwork.c +++ b/src/artwork.c @@ -142,8 +142,6 @@ enum parse_result { struct online_source { // Name of online source const char *name; - const bool is_default; - const char *setting_name; // How to authorize (using the Authorize http header) @@ -334,14 +332,13 @@ static enum parse_result response_jparse_musicbrainz(char **artwork_url, json_ob static struct online_source spotify_source = { .name = "Spotify", - .is_default = true, .setting_name = "use_artwork_source_spotify", .auth_header = "Bearer $SECRET$", .search_endpoint = "https://api.spotify.com/v1/search", .search_param = "type=track&limit=1&$QUERY$", .query_parts = { - { "q", "artist:$ARTIST$ album:$ALBUM$" }, // TODO test if album search works with type=track + { "q", "artist:$ARTIST$ album:$ALBUM$" }, { "q", "artist:$ARTIST$ track:$TITLE$" }, { NULL, NULL }, }, @@ -352,7 +349,6 @@ static struct online_source spotify_source = static struct online_source discogs_source = { .name = "Discogs", - .is_default = false, .setting_name = "use_artwork_source_discogs", .auth_header = "Discogs key=$KEY$, secret=$SECRET$", .auth_key = "ivzUxlkUiwpptDKpSCHF", @@ -373,7 +369,6 @@ static struct online_source discogs_source = static struct online_source musicbrainz_source = { .name = "Musicbrainz", - .is_default = false, .setting_name = "use_artwork_source_coverartarchive", .search_endpoint = "http://musicbrainz.org/ws/2/release-group/", .search_param = "limit=1&fmt=json&$QUERY$", @@ -997,8 +992,7 @@ online_source_response_parse(char **artwork_url, struct online_source *src, stru evbuffer_add(response, "", 1); body = (char *)evbuffer_pullup(response, -1); - // TODO remove - DPRINTF(E_DBG, L_ART, "Response from '%s': %s\n", src->name, body); + DPRINTF(E_SPAM, L_ART, "Response from '%s': %s\n", src->name, body); if (src->response_jparse) { @@ -1261,34 +1255,15 @@ static bool online_source_is_enabled(struct online_source *src) { struct settings_category *category; - cfg_t *lib; - const char *name; - int n_cfg; - int i; + bool enabled; - lib = cfg_getsec(cfg, "library"); + CHECK_NULL(L_ART, category = settings_category_get("artwork")); + enabled = settings_option_getbool(settings_option_get(category, src->setting_name)); - n_cfg = cfg_size(lib, "artwork_online_sources"); - if (n_cfg == 0) // User didn't set anything in the config file, use settings - { - category = settings_category_get("artwork"); - if (!category) - return src->is_default; + if (!enabled) + DPRINTF(E_DBG, L_ART, "Source %s is disabled\n", src->name); -// if (!settings_option_is_set(category, src->setting_name)) -// return src->is_default; - - return settings_option_getbool(settings_option_get(category, src->setting_name)); - } - - for (i = 0; i < n_cfg; i++) - { - name = cfg_getnstr(lib, "artwork_online_sources", i); - if (strcasecmp(name, src->name) == 0) - return true; - } - - return src->is_default; + return enabled; }