From e9c10189e93fe53cff74ae8fa15d19b1c522d5e4 Mon Sep 17 00:00:00 2001 From: Thomas Martitz Date: Fri, 16 Oct 2009 19:14:41 +0000 Subject: Rework albumart buffering internally to allow for mutliple albumart sizes. Playback now has a few albumart slots. Anything (most importantly: skins) can obtain such a slot. The slot has fields for the size which is passed to bufopen then to image_load to buffer the albumart with the proper size. Currently there's 1 slot. We can increase it for remotes if we want. Custom statusbar will increase it. git-svn-id: svn://svn.rockbox.org/rockbox/trunk@23209 a1c6a512-1295-4272-9138-f99709370657 --- apps/buffering.c | 20 +++-- apps/buffering.h | 3 +- apps/gui/skin_engine/skin_display.c | 6 +- apps/gui/skin_engine/skin_parser.c | 21 +++++ apps/gui/skin_engine/skin_tokens.c | 5 +- apps/gui/skin_engine/wps_internals.h | 1 + apps/gui/wps.c | 22 +---- apps/gui/wps.h | 10 --- apps/playback.c | 156 ++++++++++++++++++++++++++--------- apps/playback.h | 29 ++++++- apps/plugin.h | 3 +- apps/recorder/albumart.c | 23 +----- apps/recorder/albumart.h | 4 +- 13 files changed, 199 insertions(+), 104 deletions(-) diff --git a/apps/buffering.c b/apps/buffering.c index 08590c9fdf..e66e95d66d 100644 --- a/apps/buffering.c +++ b/apps/buffering.c @@ -54,6 +54,7 @@ #ifdef HAVE_ALBUMART #include "albumart.h" #include "jpeg_load.h" +#include "bmp.h" #endif #define GUARD_BUFSIZE (32*1024) @@ -843,10 +844,13 @@ static bool fill_buffer(void) /* Given a file descriptor to a bitmap file, write the bitmap data to the buffer, with a struct bitmap and the actual data immediately following. Return value is the total size (struct + data). */ -static int load_image(int fd, const char *path) +static int load_image(int fd, const char *path, struct dim *dim) { int rc; struct bitmap *bmp = (struct bitmap *)&buffer[buf_widx]; + + /* get the desired image size */ + bmp->width = dim->width, bmp->height = dim->height; /* FIXME: alignment may be needed for the data buffer. */ bmp->data = &buffer[buf_widx + sizeof(struct bitmap)]; #ifndef HAVE_JPEG @@ -859,8 +863,6 @@ static int load_image(int fd, const char *path) int free = (int)MIN(buffer_len - BUF_USED, buffer_len - buf_widx) - sizeof(struct bitmap); - get_albumart_size(bmp); - #ifdef HAVE_JPEG int pathlen = strlen(path); if (strcmp(path + pathlen - 4, ".bmp")) @@ -897,11 +899,19 @@ management functions for all the actual handle management work. filename: name of the file to open offset: offset at which to start buffering the file, useful when the first (offset-1) bytes of the file aren't needed. + type: one of the data types supported (audio, image, cuesheet, others + user_data: user data passed possibly passed in subcalls specific to a + data_type (only used for image (albumart) buffering so far ) return value: <0 if the file cannot be opened, or one file already queued to be opened, otherwise the handle for the file in the buffer */ -int bufopen(const char *file, size_t offset, enum data_type type) +int bufopen(const char *file, size_t offset, enum data_type type, + void *user_data) { +#ifndef HAVE_ALBUMART + /* currently only used for aa loading */ + (void)user_data; +#endif if (type == TYPE_ID3) { /* ID3 case: allocate space, init the handle and return. */ @@ -967,7 +977,7 @@ int bufopen(const char *file, size_t offset, enum data_type type) /* Bitmap file: we load the data instead of the file */ int rc; mutex_lock(&llist_mod_mutex); /* Lock because load_bitmap yields */ - rc = load_image(fd, file); + rc = load_image(fd, file, (struct dim*)user_data); mutex_unlock(&llist_mod_mutex); if (rc <= 0) { diff --git a/apps/buffering.h b/apps/buffering.h index d0e2dd797f..6e17b65d8c 100644 --- a/apps/buffering.h +++ b/apps/buffering.h @@ -74,7 +74,8 @@ bool buffering_reset(char *buf, size_t buflen); #define BUF_MAX_HANDLES 256 -int bufopen(const char *file, size_t offset, enum data_type type); +int bufopen(const char *file, size_t offset, enum data_type type, + void *user_data); int bufalloc(const void *src, size_t size, enum data_type type); bool bufclose(int handle_id); int bufseek(int handle_id, size_t newpos); diff --git a/apps/gui/skin_engine/skin_display.c b/apps/gui/skin_engine/skin_display.c index 67984cd2bb..a5ea28619a 100644 --- a/apps/gui/skin_engine/skin_display.c +++ b/apps/gui/skin_engine/skin_display.c @@ -259,7 +259,8 @@ static void wps_display_images(struct gui_wps *gwps, struct viewport* vp) if (data->albumart && data->albumart->vp == vp && data->albumart->draw) { - draw_album_art(gwps, audio_current_aa_hid(), false); + draw_album_art(gwps, playback_current_aa_hid(data->playback_aa_slot), + false); data->albumart->draw = false; } #endif @@ -486,7 +487,8 @@ static bool evaluate_conditional(struct gui_wps *gwps, int *token_index) #ifdef HAVE_ALBUMART if (data->albumart && data->tokens[i].type == WPS_TOKEN_ALBUMART_DISPLAY) { - draw_album_art(gwps, audio_current_aa_hid(), true); + draw_album_art(gwps, + playback_current_aa_hid(data->playback_aa_slot), true); data->albumart->draw = false; } #endif diff --git a/apps/gui/skin_engine/skin_parser.c b/apps/gui/skin_engine/skin_parser.c index 016126bffb..fa35ed994f 100644 --- a/apps/gui/skin_engine/skin_parser.c +++ b/apps/gui/skin_engine/skin_parser.c @@ -56,6 +56,10 @@ #include "bmp.h" #endif +#ifdef HAVE_ALBUMART +#include "playback.h" +#endif + #include "backdrop.h" #define WPS_ERROR_INVALID_PARAM -1 @@ -985,6 +989,8 @@ static int parse_albumart_load(const char *wps_bufptr, { const char *_pos, *newline; bool parsing; + struct dim dimensions; + int albumart_slot; struct skin_albumart *aa = skin_buffer_alloc(sizeof(struct skin_albumart)); (void)token; /* silence warning */ if (!aa) @@ -1125,6 +1131,16 @@ static int parse_albumart_load(const char *wps_bufptr, aa->draw = false; wps_data->albumart = aa; + dimensions.width = aa->width; + dimensions.height = aa->height; + + albumart_slot = playback_claim_aa_slot(&dimensions); + + if (albumart_slot < 0) /* didn't get a slot ? */ + return skip_end_of_line(wps_bufptr); + else + wps_data->playback_aa_slot = albumart_slot; + /* Skip the rest of the line */ return skip_end_of_line(wps_bufptr); } @@ -1601,6 +1617,11 @@ void skin_data_reset(struct wps_data *wps_data) wps_data->strings = NULL; #ifdef HAVE_ALBUMART wps_data->albumart = NULL; + if (wps_data->playback_aa_slot >= 0) + { + playback_release_aa_slot(wps_data->playback_aa_slot); + wps_data->playback_aa_slot = -1; + } #endif wps_data->tokens = NULL; wps_data->num_tokens = 0; diff --git a/apps/gui/skin_engine/skin_tokens.c b/apps/gui/skin_engine/skin_tokens.c index d607538f0f..6b29091fe6 100644 --- a/apps/gui/skin_engine/skin_tokens.c +++ b/apps/gui/skin_engine/skin_tokens.c @@ -368,8 +368,9 @@ const char *get_token_value(struct gui_wps *gwps, #ifdef HAVE_ALBUMART case WPS_TOKEN_ALBUMART_FOUND: - if (data->albumart && audio_current_aa_hid() >= 0) { - return "C"; + if (data->albumart) { + if (playback_current_aa_hid(data->playback_aa_slot) >= 0) + return "C"; } return NULL; diff --git a/apps/gui/skin_engine/wps_internals.h b/apps/gui/skin_engine/wps_internals.h index 7a4fdddc7c..638fb0a081 100644 --- a/apps/gui/skin_engine/wps_internals.h +++ b/apps/gui/skin_engine/wps_internals.h @@ -255,6 +255,7 @@ struct wps_data struct skin_token_list *strings; #ifdef HAVE_ALBUMART struct skin_albumart *albumart; + int playback_aa_slot; #endif struct wps_token *tokens; /* Total number of tokens in the WPS. During WPS parsing, this is diff --git a/apps/gui/wps.c b/apps/gui/wps.c index d4a2893ff2..342ebdea2b 100644 --- a/apps/gui/wps.c +++ b/apps/gui/wps.c @@ -1278,6 +1278,7 @@ static void statusbar_toggle_handler(void *data) } #endif + void gui_sync_wps_init(void) { int i; @@ -1285,6 +1286,7 @@ void gui_sync_wps_init(void) { #ifdef HAVE_ALBUMART wps_datas[i].albumart = NULL; + wps_datas[i].playback_aa_slot = -1; #endif #ifdef HAVE_REMOTE_LCD wps_datas[i].remote_wps = (i == SCREEN_REMOTE); @@ -1304,26 +1306,6 @@ void gui_sync_wps_init(void) #endif } -#ifdef HAVE_ALBUMART -bool wps_uses_albumart(int *width, int *height) -{ - int i; - FOR_NB_SCREENS(i) { - struct gui_wps *gwps = &gui_wps[i]; - struct skin_albumart *aa = gwps->data->albumart; - if (aa && (aa->state != WPS_ALBUMART_NONE)) - { - if (width) - *width = aa->width; - if (height) - *height = aa->height; - return true; - } - } - return false; -} -#endif - #ifdef IPOD_ACCESSORY_PROTOCOL int wps_get_ff_rewind_count(void) diff --git a/apps/gui/wps.h b/apps/gui/wps.h index 6affcee698..8c6de9e2fc 100644 --- a/apps/gui/wps.h +++ b/apps/gui/wps.h @@ -38,16 +38,6 @@ void display_keylock_text(bool locked); bool is_wps_fading(void); - -#ifdef HAVE_ALBUMART -/* - * Returns true if at least one of the gui_wps screens has an album art - * tag in its wps structure and writes the width and height into the passed - * pointers - */ -bool wps_uses_albumart(int*, int*); -#endif - #ifdef IPOD_ACCESSORY_PROTOCOL /* return length of the current ff or rewin action, IAP needs this */ int wps_get_ff_rewind_count(void); diff --git a/apps/playback.c b/apps/playback.c index c69bf39abd..4a5ef5c450 100644 --- a/apps/playback.c +++ b/apps/playback.c @@ -64,7 +64,10 @@ #include "icons.h" #include "peakmeter.h" #include "action.h" +#ifdef HAVE_ALBUMART #include "albumart.h" +#include "bmp.h" +#endif #endif #include "lang.h" #include "misc.h" @@ -158,6 +161,7 @@ enum filling_state { }; #define MAX_TRACK 128 +#define MAX_MULTIPLE_AA 1 #define MAX_TRACK_MASK (MAX_TRACK-1) @@ -206,7 +210,7 @@ struct track_info { int id3_hid; /* The ID for the track's metadata handle */ int codec_hid; /* The ID for the track's codec handle */ #ifdef HAVE_ALBUMART - int aa_hid; /* The ID for the track's album art handle */ + int aa_hid[MAX_MULTIPLE_AA];/* The ID for the track's album art handle */ #endif int cuesheet_hid; /* The ID for the track's parsed cueesheet handle */ @@ -215,6 +219,16 @@ struct track_info { bool taginfo_ready; /* Is metadata read */ }; + +#ifdef HAVE_ALBUMART +struct albumart_slot { + struct dim dim; /* holds width, height of the albumart */ + int used; /* counter, increments if something uses it */ +} albumart_slots[MAX_MULTIPLE_AA]; + +#define FOREACH_ALBUMART(i) for(i = 0;i < MAX_MULTIPLE_AA; i++) +#endif + static struct track_info tracks[MAX_TRACK]; static volatile int track_ridx = 0; /* Track being decoded (A/C-) */ static int track_widx = 0; /* Track being buffered (A) */ @@ -367,11 +381,17 @@ static bool clear_track_info(struct track_info *track) } #ifdef HAVE_ALBUMART - if (track->aa_hid >= 0) { - if (bufclose(track->aa_hid)) - track->aa_hid = -1; - else - return false; + { + int i; + FOREACH_ALBUMART(i) + { + if (track->aa_hid[i] >= 0) { + if (bufclose(track->aa_hid[i])) + track->aa_hid[i] = -1; + else + return false; + } + } } #endif @@ -538,18 +558,6 @@ void audio_remove_encoder(void) #endif /* HAVE_RECORDING */ -#ifdef HAVE_ALBUMART -int audio_current_aa_hid(void) -{ - int cur_idx; - int offset = ci.new_track + wps_offset; - - cur_idx = track_ridx + offset; - cur_idx &= MAX_TRACK_MASK; - - return tracks[cur_idx].aa_hid; -} -#endif struct mp3entry* audio_current_track(void) { @@ -673,6 +681,58 @@ struct mp3entry* audio_next_track(void) return NULL; } +#ifdef HAVE_ALBUMART +int playback_current_aa_hid(int slot) +{ + if (slot < 0) + return -1; + int cur_idx; + int offset = ci.new_track + wps_offset; + + cur_idx = track_ridx + offset; + cur_idx &= MAX_TRACK_MASK; + + return tracks[cur_idx].aa_hid[slot]; +} + +int playback_claim_aa_slot(struct dim *dim) +{ + int i; + /* first try to find a slot already having the size to reuse it + * since we don't want albumart of the same size buffered multiple times */ + FOREACH_ALBUMART(i) + { + struct albumart_slot *slot = &albumart_slots[i]; + if (slot->dim.width == dim->width + && slot->dim.height == dim->height) + { + slot->used++; + return i; + } + } + /* size is new, find a free slot */ + FOREACH_ALBUMART(i) + { + if (!albumart_slots[i].used) + { + albumart_slots[i].used++; + albumart_slots[i].dim = *dim; + return i; + } + } + /* sorry, no free slot */ + return -1; +} + +void playback_release_aa_slot(int slot) +{ + /* invalidate the albumart_slot */ + struct albumart_slot *aa_slot = &albumart_slots[slot]; + if (aa_slot->used > 0) + aa_slot->used--; +} + +#endif void audio_play(long offset) { logf("audio_play"); @@ -1671,7 +1731,7 @@ static bool audio_loadcodec(bool start_play) codec_get_full_path(codec_path, codec_fn); - tracks[track_widx].codec_hid = bufopen(codec_path, 0, TYPE_CODEC); + tracks[track_widx].codec_hid = bufopen(codec_path, 0, TYPE_CODEC, NULL); if (tracks[track_widx].codec_hid < 0) return false; @@ -1757,7 +1817,7 @@ static bool audio_load_track(size_t offset, bool start_play) /* Get track metadata if we don't already have it. */ if (tracks[track_widx].id3_hid < 0) { - tracks[track_widx].id3_hid = bufopen(trackname, 0, TYPE_ID3); + tracks[track_widx].id3_hid = bufopen(trackname, 0, TYPE_ID3, NULL); if (tracks[track_widx].id3_hid < 0) { @@ -1859,26 +1919,37 @@ static void audio_finish_load_track(void) } } #ifdef HAVE_ALBUMART - if (tracks[track_widx].aa_hid < 0) { + int i; char aa_path[MAX_PATH]; - /* find_albumart will error out if the wps doesn't have AA */ - if (find_albumart(track_id3, aa_path, sizeof(aa_path))) + FOREACH_ALBUMART(i) { - tracks[track_widx].aa_hid = bufopen(aa_path, 0, TYPE_BITMAP); - - if(tracks[track_widx].aa_hid == ERR_BUFFER_FULL) + /* albumart_slots may change during a yield of bufopen, + * but that's no problem */ + if (tracks[track_widx].aa_hid[i] >= 0 || !albumart_slots[i].used) + continue; + /* find_albumart will error out if the wps doesn't have AA */ + if (find_albumart(track_id3, aa_path, sizeof(aa_path), + &(albumart_slots[i].dim))) { - filling = STATE_FULL; - logf("buffer is full for now"); - return; /* No space for track's album art, not an error */ - } - else if (tracks[track_widx].aa_hid < 0) - { - /* another error, ignore AlbumArt */ - logf("Album art loading failed"); + int aa_hid = bufopen(aa_path, 0, TYPE_BITMAP, + &(albumart_slots[i].dim)); + + if(aa_hid == ERR_BUFFER_FULL) + { + filling = STATE_FULL; + logf("buffer is full for now"); + return; /* No space for track's album art, not an error */ + } + else if (aa_hid < 0) + { + /* another error, ignore AlbumArt */ + logf("Album art loading failed"); + } + tracks[track_widx].aa_hid[i] = aa_hid; } } + } #endif @@ -1954,7 +2025,8 @@ static void audio_finish_load_track(void) else file_offset = 0; - tracks[track_widx].audio_hid = bufopen(track_id3->path, file_offset, type); + tracks[track_widx].audio_hid = bufopen(track_id3->path, file_offset, type, + NULL); /* No space left, not an error */ if (tracks[track_widx].audio_hid == ERR_BUFFER_FULL) @@ -2558,7 +2630,6 @@ static void audio_thread(void) LOGFQUEUE("audio < Q_AUDIO_TRACK_CHANGED"); audio_finalise_track_change(); break; - #ifndef SIMULATOR case SYS_USB_CONNECTED: LOGFQUEUE("audio < SYS_USB_CONNECTED"); @@ -2681,11 +2752,18 @@ void audio_init(void) tracks[i].audio_hid = -1; tracks[i].id3_hid = -1; tracks[i].codec_hid = -1; -#ifdef HAVE_ALBUMART - tracks[i].aa_hid = -1; -#endif tracks[i].cuesheet_hid = -1; } +#ifdef HAVE_ALBUMART + FOREACH_ALBUMART(i) + { + int j; + for (j = 0; j < MAX_TRACK; j++) + { + tracks[j].aa_hid[i] = -1; + } + } +#endif add_event(BUFFER_EVENT_REBUFFER, false, buffering_handle_rebuffer_callback); add_event(BUFFER_EVENT_FINISHED, false, buffering_handle_finished_callback); diff --git a/apps/playback.h b/apps/playback.h index 378d5effec..add11e296b 100644 --- a/apps/playback.h +++ b/apps/playback.h @@ -25,6 +25,32 @@ #include #include "config.h" +#ifdef HAVE_ALBUMART + +#include "bmp.h" +/* + * Returns the handle id of the buffered albumart for the given slot id + **/ +int playback_current_aa_hid(int slot); + +/* + * Hands out an albumart slot for buffering albumart using the size + * int the passed dim struct, it copies the data of dim in order to + * be safe to be reused for other code + * + * The slot may be reused if other code calls this with the same dimensions + * in dim, so if you change dim release and claim a new slot + * + * Save to call from other threads */ +int playback_claim_aa_slot(struct dim *dim); + +/* + * Releases the albumart slot with given id + * + * Save to call from other threads */ +void playback_release_aa_slot(int slot); +#endif + /* Functions */ const char *get_codec_filename(int cod_spec); void voice_wait(void); @@ -45,9 +71,6 @@ bool audio_restore_playback(int type); /* Restores the audio buffer to handle th void codec_thread_do_callback(void (*fn)(void), unsigned int *codec_thread_id); -#ifdef HAVE_ALBUMART -int audio_current_aa_hid(void); -#endif #if CONFIG_CODEC == SWCODEC /* This #ifdef is better here than gui/wps.c */ extern void audio_next_dir(void); diff --git a/apps/plugin.h b/apps/plugin.h index b060104373..41375a6adb 100644 --- a/apps/plugin.h +++ b/apps/plugin.h @@ -789,7 +789,8 @@ struct plugin_api { #if (CONFIG_CODEC == SWCODEC) /* buffering API */ - int (*bufopen)(const char *file, size_t offset, enum data_type type); + int (*bufopen)(const char *file, size_t offset, enum data_type type, + void *user_data); int (*bufalloc)(const void *src, size_t size, enum data_type type); bool (*bufclose)(int handle_id); int (*bufseek)(int handle_id, size_t newpos); diff --git a/apps/recorder/albumart.c b/apps/recorder/albumart.c index bb2fae4af7..b668bf4eba 100644 --- a/apps/recorder/albumart.c +++ b/apps/recorder/albumart.c @@ -274,22 +274,18 @@ bool search_albumart_files(const struct mp3entry *id3, const char *size_string, /* Look for albumart bitmap in the same dir as the track and in its parent dir. * Stores the found filename in the buf parameter. * Returns true if a bitmap was found, false otherwise */ -bool find_albumart(const struct mp3entry *id3, char *buf, int buflen) +bool find_albumart(const struct mp3entry *id3, char *buf, int buflen, + struct dim *dim) { if (!id3 || !buf) return false; char size_string[9]; - int width = 0, height = 0; - - if (!wps_uses_albumart(&width, &height)) - return false; - logf("Looking for album art for %s", id3->path); /* Write the size string, e.g. ".100x100". */ snprintf(size_string, sizeof(size_string), ".%dx%d", - width, height); + dim->width, dim->height); /* First we look for a bitmap of the right size */ if (search_albumart_files(id3, size_string, buf, buflen)) @@ -376,17 +372,4 @@ void draw_album_art(struct gui_wps *gwps, int handle_id, bool clear) } } -void get_albumart_size(struct bitmap *bmp) -{ - /* FIXME: What should we do with albumart on remote? */ - int width, height; - - if (!wps_uses_albumart(&width, &height)) - { - width = 0; height = 0; - } - - bmp->width = width; - bmp->height = height; -} #endif /* PLUGIN */ diff --git a/apps/recorder/albumart.h b/apps/recorder/albumart.h index d1c2dfa7bd..51f456d175 100644 --- a/apps/recorder/albumart.h +++ b/apps/recorder/albumart.h @@ -29,9 +29,11 @@ #include "skin_engine/skin_engine.h" /* Look for albumart bitmap in the same dir as the track and in its parent dir. + * Calls size_func to get the dimensions to look for * Stores the found filename in the buf parameter. * Returns true if a bitmap was found, false otherwise */ -bool find_albumart(const struct mp3entry *id3, char *buf, int buflen); +bool find_albumart(const struct mp3entry *id3, char *buf, int buflen, + struct dim *dim); /* Draw the album art bitmap from the given handle ID onto the given WPS. Call with clear = true to clear the bitmap instead of drawing it. */ -- cgit v1.2.3