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

@@ -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)
         {

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);

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

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;

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) {
+            if (data->albumart) {
-                return "C";
+                if (playback_current_aa_hid(data->playback_aa_slot) >= 0)
+                    return "C";
             }
             return NULL;
             

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

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)

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);

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))
+        int i;
-            track->aa_hid = -1;
+        FOREACH_ALBUMART(i)
-        else
+        {
-            return false;
+            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 */
+        FOREACH_ALBUMART(i)
-        if (find_albumart(track_id3, aa_path, sizeof(aa_path)))
         {
-            tracks[track_widx].aa_hid = bufopen(aa_path, 0, TYPE_BITMAP);
+            /* 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)))
+            {
+                int aa_hid = bufopen(aa_path, 0, TYPE_BITMAP,
+                                                &(albumart_slots[i].dim));
 
-            if(tracks[track_widx].aa_hid == ERR_BUFFER_FULL)
+                if(aa_hid == ERR_BUFFER_FULL)
-            {
+                {
-                filling = STATE_FULL;
+                    filling = STATE_FULL;
-                logf("buffer is full for now");
+                    logf("buffer is full for now");
-                return;  /* No space for track's album art, not an error */
+                    return;  /* No space for track's album art, not an error */
-            }
+                }
-            else if (tracks[track_widx].aa_hid < 0)
+                else if (aa_hid < 0)
-            {
+                {
-                /* another error, ignore AlbumArt */
+                    /* another error, ignore AlbumArt */
-                logf("Album art loading failed");
+                    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);

apps/playback.h

@@ -25,6 +25,32 @@
 #include <stdbool.h>
 #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);

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);

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 */

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. */