diff options
author | Michael Giacomelli <giac2000@hotmail.com> | 2020-12-09 13:17:05 -0500 |
---|---|---|
committer | Solomon Peachy <pizza@shaftnet.org> | 2020-12-25 17:47:19 +0000 |
commit | ca09f91f647f6677fea7804930d7d1b2c876f76e (patch) | |
tree | 03f6b4e5659deb7f52b315478d8106babaf358ab /apps | |
parent | b5e6c30a61047e89f6cb1299a81453a80b79cc37 (diff) | |
download | rockbox-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
Diffstat (limited to 'apps')
-rwxr-xr-x[-rw-r--r--] | apps/buffering.c | 7 | ||||
-rwxr-xr-x[-rw-r--r--] | apps/buffering.h | 1 | ||||
-rwxr-xr-x[-rw-r--r--] | apps/playback.c | 15 | ||||
-rwxr-xr-x[-rw-r--r--] | apps/radio/radioart.c | 2 |
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 */ |
52 | void buffering_init(void) INIT_ATTR; | 53 | void 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 */ |
1692 | static bool audio_load_albumart(struct track_info *infop, | 1692 | static 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) |