devs/foxbox
1
0
Fork 0
forked from len0rd/rockbox
Code Pull requests Activity

Discontinue any use of 'swp(b)' on PP5020. While clocking is stable, some testing revealed this instruction can still cause problems without concurrent access. Make sure mpegplayer is safe while not using spinlock (no longer atomic) between cores to protect the stream byte counters - use nonwrapping head and tail pointers.

Browse source 
git-svn-id: svn://svn.rockbox.org/rockbox/trunk@15005 a1c6a512-1295-4272-9138-f99709370657
This commit is contained in:
Michael Sevakis 2007-10-06 15:01:37 +00:00
parent a5133307c7
commit f6de0d4083
2 changed files with 19 additions and 42 deletions
Download patch file Download diff file Expand all files Collapse all files

59
apps/plugins/mpegplayer/mpegplayer.c
View file

@ -201,7 +201,8 @@ typedef struct
uint8_t* next_packet; /* Next stream packet beginning */ uint8_t* next_packet; /* Next stream packet beginning */
size_t guard_bytes; /* Number of bytes in guardbuf used */ size_t guard_bytes; /* Number of bytes in guardbuf used */
size_t buffer_remaining; /* How much data is left in the buffer */ uint64_t buffer_tail; /* Accumulation of bytes added */
uint64_t buffer_head; /* Accumulation of bytes removed */
uint32_t curr_pts; /* Current presentation timestamp */ uint32_t curr_pts; /* Current presentation timestamp */
uint32_t curr_time; /* Current time in samples */ uint32_t curr_time; /* Current time in samples */
uint32_t tagged; /* curr_pts is valid */ uint32_t tagged; /* curr_pts is valid */
@ -300,7 +301,8 @@ static intptr_t str_send_msg(Stream *str, int id, intptr_t data)
return str->dispatch_fn(str, msg); return str->dispatch_fn(str, msg);
#endif #endif
/* Only one thread at a time, please */ /* Only one thread at a time, please - only one core may safely send
right now */
rb->spinlock_lock(&str->msg_lock); rb->spinlock_lock(&str->msg_lock);
str->ev.id = id; str->ev.id = id;
@ -335,24 +337,6 @@ static uint8_t *disk_buf IBSS_ATTR;
static uint8_t *disk_buf_end IBSS_ATTR; static uint8_t *disk_buf_end IBSS_ATTR;
static uint8_t *disk_buf_tail IBSS_ATTR; static uint8_t *disk_buf_tail IBSS_ATTR;
static size_t buffer_size IBSS_ATTR; static size_t buffer_size IBSS_ATTR;
#if NUM_CORES > 1
/* Some stream variables are shared between cores */
struct mutex stream_lock IBSS_ATTR;
static inline void init_stream_lock(void)
{ rb->spinlock_init(&stream_lock); }
static inline void lock_stream(void)
{ rb->spinlock_lock(&stream_lock); }
static inline void unlock_stream(void)
{ rb->spinlock_unlock(&stream_lock); }
#else
/* No RMW issue here */
static inline void init_stream_lock(void)
{ }
static inline void lock_stream(void)
{ }
static inline void unlock_stream(void)
{ }
#endif
#define MSG_BUFFER_NEARLY_EMPTY 1 #define MSG_BUFFER_NEARLY_EMPTY 1
#define MSG_EXIT_REQUESTED 2 #define MSG_EXIT_REQUESTED 2
@ -677,22 +661,17 @@ static void get_next_data( Stream* str )
if (str->curr_packet != NULL) if (str->curr_packet != NULL)
{ {
lock_stream();
if (str->curr_packet < str->prev_packet) if (str->curr_packet < str->prev_packet)
{ {
str->buffer_remaining -= (disk_buf_end - str->prev_packet) + str->buffer_head += (disk_buf_end - str->prev_packet) +
(str->curr_packet - disk_buf); (str->curr_packet - disk_buf);
str->buffer_remaining -= str->guard_bytes;
str->guard_bytes = 0; str->guard_bytes = 0;
} }
else else
{ {
str->buffer_remaining -= (str->curr_packet - str->prev_packet); str->buffer_head += (str->curr_packet - str->prev_packet);
} }
unlock_stream();
str->prev_packet = str->curr_packet; str->prev_packet = str->curr_packet;
} }
@ -1898,11 +1877,14 @@ enum plugin_status plugin_start(struct plugin_api* api, void* parameter)
disk_buf_tail = buffer+disk_buf_len; disk_buf_tail = buffer+disk_buf_len;
file_remaining -= disk_buf_len; file_remaining -= disk_buf_len;
video_str.guard_bytes = audio_str.guard_bytes = 0; audio_str.guard_bytes = 0;
video_str.prev_packet = disk_buf;
audio_str.prev_packet = disk_buf; audio_str.prev_packet = disk_buf;
video_str.buffer_remaining = disk_buf_len; audio_str.buffer_head = 0;
audio_str.buffer_remaining = disk_buf_len; audio_str.buffer_tail = disk_buf_len;
video_str.guard_bytes = 0;
video_str.prev_packet = disk_buf;
video_str.buffer_head = 0;
video_str.buffer_tail = disk_buf_len;
rb->spinlock_init(&audio_str.msg_lock); rb->spinlock_init(&audio_str.msg_lock);
rb->spinlock_init(&video_str.msg_lock); rb->spinlock_init(&video_str.msg_lock);
@ -1913,8 +1895,6 @@ enum plugin_status plugin_start(struct plugin_api* api, void* parameter)
gray_show(true); gray_show(true);
#endif #endif
init_stream_lock();
#if NUM_CORES > 1 #if NUM_CORES > 1
flush_icache(); flush_icache();
#endif #endif
@ -1940,8 +1920,8 @@ enum plugin_status plugin_start(struct plugin_api* api, void* parameter)
/* Wait until both threads have finished their work */ /* Wait until both threads have finished their work */
while ((audio_str.status >= 0) || (video_str.status >= 0)) while ((audio_str.status >= 0) || (video_str.status >= 0))
{ {
size_t audio_remaining = audio_str.buffer_remaining; size_t audio_remaining = audio_str.buffer_tail - audio_str.buffer_head;
size_t video_remaining = video_str.buffer_remaining; size_t video_remaining = video_str.buffer_tail - video_str.buffer_head;
if (MIN(audio_remaining,video_remaining) < MPEG_LOW_WATERMARK) { if (MIN(audio_remaining,video_remaining) < MPEG_LOW_WATERMARK) {
@ -1957,11 +1937,8 @@ enum plugin_status plugin_start(struct plugin_api* api, void* parameter)
bytes_to_read -= n; bytes_to_read -= n;
file_remaining -= n; file_remaining -= n;
lock_stream(); audio_str.buffer_tail += n;
audio_str.buffer_remaining += n; video_str.buffer_tail += n;
video_str.buffer_remaining += n;
unlock_stream();
disk_buf_tail += n; disk_buf_tail += n;
rb->yield(); rb->yield();

2
firmware/export/thread.h
View file

@ -150,7 +150,7 @@ struct core_entry {
*/ */
/* Macros generate better code than an inline function is this case */ /* Macros generate better code than an inline function is this case */
#if defined (CPU_PP) || defined (CPU_ARM) #if (defined (CPU_PP) || defined (CPU_ARM)) && CONFIG_CPU != PP5020
#define test_and_set(x_, v_) \ #define test_and_set(x_, v_) \
({ \ ({ \
uint32_t old; \ uint32_t old; \