mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-09 03:29:43 -10:00
(PSP/Vita/PS4) The room check could not tell a full ring from an empty one
write_pos and read_pos are both masked into the ring, so the writer having filled every frame and the writer having written nothing both read as a distance of zero, and the free-space computation answered with the whole ring. A writer that caught up exactly lapped: it wrote from read_pos forward, over the window the device was reading, while the worker - reading the same distance - took the full ring for an empty one and played silence. Hold a frame back, so the two states differ. Found with a threaded model of the mainloop against the write path using the tree's own retro_atomic ops, under TSan: the race reproduces with the frame returned and does not with it held back, and it is independent of the read_pos publication order - each fix is needed and neither covers the other.
This commit is contained in:
1 parent
ba5339c419
commit
d36404a114
3 files changed
+36
-18
No files matched your search
@@ -71,6 +71,12 @@ typedef struct ps4_audio
|
||||
#define AUDIO_BUFFER_SIZE (1u<<13u)
|
||||
#define AUDIO_BUFFER_SIZE_MASK (AUDIO_BUFFER_SIZE-1)
|
||||
|
||||
/* What the writer may fill. One frame short of the ring, so that a
|
||||
* full ring and an empty one do not both read as write_pos ==
|
||||
* read_pos - which let the writer lap and overwrite the window the
|
||||
* device was reading. */
|
||||
#define AUDIO_BUFFER_USABLE (AUDIO_BUFFER_SIZE - 1u)
|
||||
|
||||
/* The silence sits past the ring. read_pos only ever advances by
|
||||
* AUDIO_OUT_COUNT, which divides AUDIO_BUFFER_SIZE, so no window
|
||||
* handed to the output syscall crosses into it. */
|
||||
@@ -234,7 +240,7 @@ static ssize_t ps4_audio_write(void *data, const void *s, size_t len)
|
||||
* frames here too. */
|
||||
if (ps4->nonblock)
|
||||
{
|
||||
if (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
|
||||
if (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&ps4->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK) < sample_count)
|
||||
return 0;
|
||||
@@ -249,7 +255,7 @@ static ssize_t ps4_audio_write(void *data, const void *s, size_t len)
|
||||
* are re-checked inside the eventcount's window, so a period
|
||||
* freed between the check and the park costs nothing. */
|
||||
int laps = PS4_AUDIO_WAIT_LAPS;
|
||||
while (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
|
||||
while (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&ps4->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK) < sample_count)
|
||||
{
|
||||
@@ -257,7 +263,7 @@ static ssize_t ps4_audio_write(void *data, const void *s, size_t len)
|
||||
if (--laps < 0 || !retro_atomic_load_acquire_int(&ps4->running))
|
||||
return 0;
|
||||
key = retro_eventcount_prepare_wait(&ps4->park);
|
||||
if ( (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
|
||||
if ( (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&ps4->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK) >= sample_count)
|
||||
|| !retro_atomic_load_acquire_int(&ps4->running))
|
||||
@@ -348,7 +354,7 @@ static size_t ps4_write_avail(void *data)
|
||||
|
||||
if (!ps4 || !retro_atomic_load_acquire_int(&ps4->running))
|
||||
return 0;
|
||||
_len = AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
|
||||
_len = AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
|
||||
retro_atomic_load_relaxed_int(&ps4->write_pos) - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&ps4->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK);
|
||||
@@ -376,7 +382,7 @@ static size_t ps4_wait_writable(void *data, size_t len)
|
||||
int key;
|
||||
if (!retro_atomic_load_acquire_int(&ps4->running))
|
||||
return 0;
|
||||
avail = AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
|
||||
avail = AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
|
||||
retro_atomic_load_relaxed_int(&ps4->write_pos) - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&ps4->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK);
|
||||
@@ -388,7 +394,7 @@ static size_t ps4_wait_writable(void *data, size_t len)
|
||||
if (--laps < 0)
|
||||
return 0;
|
||||
key = retro_eventcount_prepare_wait(&ps4->park);
|
||||
if ((AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
|
||||
if ((AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
|
||||
retro_atomic_load_relaxed_int(&ps4->write_pos) - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&ps4->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK)) >= want
|
||||
|
||||
@@ -73,6 +73,12 @@ typedef struct psp2_audio
|
||||
#define AUDIO_BUFFER_SIZE (1u<<13u)
|
||||
#define AUDIO_BUFFER_SIZE_MASK (AUDIO_BUFFER_SIZE-1)
|
||||
|
||||
/* What the writer may fill. One frame short of the ring, so that a
|
||||
* full ring and an empty one do not both read as write_pos ==
|
||||
* read_pos - which let the writer lap and overwrite the window the
|
||||
* device was reading. */
|
||||
#define AUDIO_BUFFER_USABLE (AUDIO_BUFFER_SIZE - 1u)
|
||||
|
||||
/* The silence sits past the ring. read_pos only ever advances by
|
||||
* AUDIO_OUT_COUNT, which divides AUDIO_BUFFER_SIZE, so no window
|
||||
* handed to the output syscall crosses into it. */
|
||||
@@ -233,7 +239,7 @@ static ssize_t psp2_audio_write(void *data, const void *s, size_t len)
|
||||
* frames here too. */
|
||||
if (psp->nonblock)
|
||||
{
|
||||
if (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
|
||||
if (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&psp->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK) < sample_count)
|
||||
return 0;
|
||||
@@ -248,7 +254,7 @@ static ssize_t psp2_audio_write(void *data, const void *s, size_t len)
|
||||
* are re-checked inside the eventcount's window, so a period
|
||||
* freed between the check and the park costs nothing. */
|
||||
int laps = PSP2_AUDIO_WAIT_LAPS;
|
||||
while (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
|
||||
while (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&psp->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK) < sample_count)
|
||||
{
|
||||
@@ -256,7 +262,7 @@ static ssize_t psp2_audio_write(void *data, const void *s, size_t len)
|
||||
if (--laps < 0 || !retro_atomic_load_acquire_int(&psp->running))
|
||||
return 0;
|
||||
key = retro_eventcount_prepare_wait(&psp->park);
|
||||
if ( (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
|
||||
if ( (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&psp->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK) >= sample_count)
|
||||
|| !retro_atomic_load_acquire_int(&psp->running))
|
||||
@@ -347,7 +353,7 @@ static size_t psp2_write_avail(void *data)
|
||||
|
||||
if (!psp || !retro_atomic_load_acquire_int(&psp->running))
|
||||
return 0;
|
||||
_len = AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
|
||||
_len = AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
|
||||
retro_atomic_load_relaxed_int(&psp->write_pos) - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&psp->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK);
|
||||
@@ -375,7 +381,7 @@ static size_t psp2_wait_writable(void *data, size_t len)
|
||||
int key;
|
||||
if (!retro_atomic_load_acquire_int(&psp->running))
|
||||
return 0;
|
||||
avail = AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
|
||||
avail = AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
|
||||
retro_atomic_load_relaxed_int(&psp->write_pos) - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&psp->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK);
|
||||
@@ -387,7 +393,7 @@ static size_t psp2_wait_writable(void *data, size_t len)
|
||||
if (--laps < 0)
|
||||
return 0;
|
||||
key = retro_eventcount_prepare_wait(&psp->park);
|
||||
if ((AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
|
||||
if ((AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
|
||||
retro_atomic_load_relaxed_int(&psp->write_pos) - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&psp->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK)) >= want
|
||||
|
||||
@@ -71,6 +71,12 @@ typedef struct psp_audio
|
||||
#define AUDIO_BUFFER_SIZE (1u<<13u)
|
||||
#define AUDIO_BUFFER_SIZE_MASK (AUDIO_BUFFER_SIZE-1)
|
||||
|
||||
/* What the writer may fill. One frame short of the ring, so that a
|
||||
* full ring and an empty one do not both read as write_pos ==
|
||||
* read_pos - which let the writer lap and overwrite the window the
|
||||
* device was reading. */
|
||||
#define AUDIO_BUFFER_USABLE (AUDIO_BUFFER_SIZE - 1u)
|
||||
|
||||
/* The silence sits past the ring. read_pos only ever advances by
|
||||
* AUDIO_OUT_COUNT, which divides AUDIO_BUFFER_SIZE, so no window
|
||||
* handed to the output syscall crosses into it. */
|
||||
@@ -231,7 +237,7 @@ static ssize_t psp_audio_write(void *data, const void *s, size_t len)
|
||||
* frames here too. */
|
||||
if (psp->nonblock)
|
||||
{
|
||||
if (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
|
||||
if (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&psp->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK) < sample_count)
|
||||
return 0;
|
||||
@@ -246,7 +252,7 @@ static ssize_t psp_audio_write(void *data, const void *s, size_t len)
|
||||
* are re-checked inside the eventcount's window, so a period
|
||||
* freed between the check and the park costs nothing. */
|
||||
int laps = PSP_AUDIO_WAIT_LAPS;
|
||||
while (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
|
||||
while (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&psp->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK) < sample_count)
|
||||
{
|
||||
@@ -254,7 +260,7 @@ static ssize_t psp_audio_write(void *data, const void *s, size_t len)
|
||||
if (--laps < 0 || !retro_atomic_load_acquire_int(&psp->running))
|
||||
return 0;
|
||||
key = retro_eventcount_prepare_wait(&psp->park);
|
||||
if ( (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
|
||||
if ( (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&psp->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK) >= sample_count)
|
||||
|| !retro_atomic_load_acquire_int(&psp->running))
|
||||
@@ -345,7 +351,7 @@ static size_t psp_write_avail(void *data)
|
||||
|
||||
if (!psp || !retro_atomic_load_acquire_int(&psp->running))
|
||||
return 0;
|
||||
_len = AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
|
||||
_len = AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
|
||||
retro_atomic_load_relaxed_int(&psp->write_pos) - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&psp->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK);
|
||||
@@ -373,7 +379,7 @@ static size_t psp_wait_writable(void *data, size_t len)
|
||||
int key;
|
||||
if (!retro_atomic_load_acquire_int(&psp->running))
|
||||
return 0;
|
||||
avail = AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
|
||||
avail = AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
|
||||
retro_atomic_load_relaxed_int(&psp->write_pos) - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&psp->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK);
|
||||
@@ -385,7 +391,7 @@ static size_t psp_wait_writable(void *data, size_t len)
|
||||
if (--laps < 0)
|
||||
return 0;
|
||||
key = retro_eventcount_prepare_wait(&psp->park);
|
||||
if ((AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
|
||||
if ((AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
|
||||
retro_atomic_load_relaxed_int(&psp->write_pos) - (uint16_t)
|
||||
retro_atomic_load_acquire_int(&psp->read_pos))
|
||||
& AUDIO_BUFFER_SIZE_MASK)) >= want
|
||||
|
||||
Reference in new issue
Block a user