summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Giacomelli <giac2000@hotmail.com>2020-12-09 13:17:05 -0500
committerSolomon Peachy <pizza@shaftnet.org>2020-12-25 17:47:19 +0000
commitca09f91f647f6677fea7804930d7d1b2c876f76e (patch)
tree03f6b4e5659deb7f52b315478d8106babaf358ab
parentb5e6c30a61047e89f6cb1299a81453a80b79cc37 (diff)
downloadrockbox-ca09f91f647f6677fea7804930d7d1b2c876f76e.tar.gz
rockbox-ca09f91f647f6677fea7804930d7d1b2c876f76e.zip
Fix deadlocks when trying to buffer large album art.
Internally, buffering tries to load the entire album art file into the audio buffer, which will fail if the file is larger than the buffer. Playback.c interprets a file failing to buffer to mean that the buffer is full, so it waits for more space and tries again. This results in a deadlock since the file will never fit. Change bufopen to return a new error condition when an image file will not fit on the buffer because it is too large: ERR_BITMAP_TOO_LARGE. Note that we arbitrarily set "too large" to be within 64KB of the entire buffer size or larger, this could be adjusted if needed. Change audio_load_albumart to pass through error messages from bufopen. In playback.c, check to see why audio_load_albumart fails. If it fails because the file is too large to buffer, simply ignore the file. If it fails because the file would fit but the buffer is full, try again later. Change-Id: I66799ae26f124b495e1522fce7285332f4cf986f
-rwxr-xr-x[-rw-r--r--]apps/buffering.c7
-rwxr-xr-x[-rw-r--r--]apps/buffering.h1
-rwxr-xr-x[-rw-r--r--]apps/playback.c15
-rwxr-xr-x[-rw-r--r--]apps/radio/radioart.c2
4 files changed, 21 insertions, 4 deletions
diff --git a/apps/buffering.c b/apps/buffering.c
index 9ffd35714c..3adbc4a6b9 100644..100755
--- a/apps/buffering.c
+++ b/apps/buffering.c
@@ -999,7 +999,14 @@ int bufopen(const char *file, off_t offset, enum data_type type,
999 DEBUGF("%s(): failed to add handle\n", __func__); 999 DEBUGF("%s(): failed to add handle\n", __func__);
1000 mutex_unlock(&llist_mutex); 1000 mutex_unlock(&llist_mutex);
1001 close(fd); 1001 close(fd);
1002
1003 /*warn playback.c if it is trying to buffer too large of an image*/
1004 if(type == TYPE_BITMAP && padded_size >= buffer_len - 64*1024)
1005 {
1006 return ERR_BITMAP_TOO_LARGE;
1007 }
1002 return ERR_BUFFER_FULL; 1008 return ERR_BUFFER_FULL;
1009
1003 } 1010 }
1004 1011
1005 handle_id = h->id; 1012 handle_id = h->id;
diff --git a/apps/buffering.h b/apps/buffering.h
index fcffcf086c..1a75d865ae 100644..100755
--- a/apps/buffering.h
+++ b/apps/buffering.h
@@ -47,6 +47,7 @@ enum data_type {
47#define ERR_HANDLE_NOT_DONE -5 47#define ERR_HANDLE_NOT_DONE -5
48#define ERR_UNSUPPORTED_TYPE -6 48#define ERR_UNSUPPORTED_TYPE -6
49#define ERR_WRONG_THREAD -7 49#define ERR_WRONG_THREAD -7
50#define ERR_BITMAP_TOO_LARGE -8
50 51
51/* Initialise the buffering subsystem */ 52/* Initialise the buffering subsystem */
52void buffering_init(void) INIT_ATTR; 53void buffering_init(void) INIT_ATTR;
diff --git a/apps/playback.c b/apps/playback.c
index 31aed2abe1..5419888c27 100644..100755
--- a/apps/playback.c
+++ b/apps/playback.c
@@ -1689,7 +1689,7 @@ static bool audio_load_cuesheet(struct track_info *infop,
1689 1689
1690#ifdef HAVE_ALBUMART 1690#ifdef HAVE_ALBUMART
1691/* Load any album art for the file - returns false if the buffer is full */ 1691/* Load any album art for the file - returns false if the buffer is full */
1692static bool audio_load_albumart(struct track_info *infop, 1692static int audio_load_albumart(struct track_info *infop,
1693 struct mp3entry *track_id3) 1693 struct mp3entry *track_id3)
1694{ 1694{
1695 FOREACH_ALBUMART(i) 1695 FOREACH_ALBUMART(i)
@@ -1730,7 +1730,11 @@ static bool audio_load_albumart(struct track_info *infop,
1730 if (hid == ERR_BUFFER_FULL) 1730 if (hid == ERR_BUFFER_FULL)
1731 { 1731 {
1732 logf("buffer is full for now (%s)", __func__); 1732 logf("buffer is full for now (%s)", __func__);
1733 return false; 1733 return ERR_BUFFER_FULL;
1734 }
1735 else if (hid == ERR_BITMAP_TOO_LARGE){
1736 logf("image is too large to fit in buffer (%s)", __func__);
1737 return ERR_BITMAP_TOO_LARGE;
1734 } 1738 }
1735 else 1739 else
1736 { 1740 {
@@ -1981,7 +1985,12 @@ static int audio_finish_load_track(struct track_info *infop)
1981 1985
1982#ifdef HAVE_ALBUMART 1986#ifdef HAVE_ALBUMART
1983 /* Try to load album art for the track */ 1987 /* Try to load album art for the track */
1984 if (!audio_load_albumart(infop, track_id3)) 1988 int retval = audio_load_albumart(infop, track_id3);
1989 if (retval == ERR_BITMAP_TOO_LARGE)
1990 {
1991 /* No space for album art on buffer because the file is larger than the buffer.
1992 Ignore the file and keep buffering */
1993 } else if (retval == ERR_BUFFER_FULL)
1985 { 1994 {
1986 /* No space for album art on buffer, not an error */ 1995 /* No space for album art on buffer, not an error */
1987 filling = STATE_FULL; 1996 filling = STATE_FULL;
diff --git a/apps/radio/radioart.c b/apps/radio/radioart.c
index 5e1a0ad5cf..87d37cd52c 100644..100755
--- a/apps/radio/radioart.c
+++ b/apps/radio/radioart.c
@@ -88,7 +88,7 @@ static int load_radioart_image(struct radioart *ra, const char* preset_name,
88 user_data.embedded_albumart = NULL; 88 user_data.embedded_albumart = NULL;
89 user_data.dim = &ra->dim; 89 user_data.dim = &ra->dim;
90 ra->handle = bufopen(path, 0, TYPE_BITMAP, &user_data); 90 ra->handle = bufopen(path, 0, TYPE_BITMAP, &user_data);
91 if (ra->handle == ERR_BUFFER_FULL) 91 if (ra->handle == ERR_BUFFER_FULL || ra->handle == ERR_BITMAP_TOO_LARGE)
92 { 92 {
93 int i = find_oldest_image_index(); 93 int i = find_oldest_image_index();
94 if (i != -1) 94 if (i != -1)