| Message ID | 20220122125745.5037-1-vr_qemu@t-online.de |
|---|---|
| State | New |
| Headers | show |
| Series | reduce audio playback latency | expand |
On Samstag, 22. Januar 2022 13:57:31 CET Volker Rümelin wrote: > Replace open-coded buffer arithmetic with the new function > audio_ring_posb(). That's the position in backward direction > of a given point at a given distance. > > Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> > --- First of all, getting rid of all those redundant, open coded ringbuffer traversal code places highly makes sense! > audio/audio.c | 25 +++++++------------------ > audio/audio_int.h | 6 ++++++ > audio/coreaudio.c | 10 ++++------ > audio/sdlaudio.c | 11 +++++------ > 4 files changed, 22 insertions(+), 30 deletions(-) > > diff --git a/audio/audio.c b/audio/audio.c > index dc28685d22..e7a139e289 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -574,19 +574,13 @@ static size_t audio_pcm_sw_get_rpos_in(SWVoiceIn *sw) > { > HWVoiceIn *hw = sw->hw; > ssize_t live = hw->total_samples_captured - sw->total_hw_samples_acquired; > - ssize_t rpos; > > if (audio_bug(__func__, live < 0 || live > hw->conv_buf->size)) { > dolog("live=%zu hw->conv_buf->size=%zu\n", live, hw->conv_buf->size); > return 0; > } > > - rpos = hw->conv_buf->pos - live; > - if (rpos >= 0) { > - return rpos; > - } else { > - return hw->conv_buf->size + rpos; > - } > + return audio_ring_posb(hw->conv_buf->pos, live, hw->conv_buf->size); > } > > static size_t audio_pcm_sw_read(SWVoiceIn *sw, void *buf, size_t size) > @@ -1394,12 +1388,10 @@ void audio_generic_run_buffer_in(HWVoiceIn *hw) > > void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size) > { > - ssize_t start = (ssize_t)hw->pos_emul - hw->pending_emul; > + size_t start; > > - if (start < 0) { > - start += hw->size_emul; > - } > - assert(start >= 0 && start < hw->size_emul); > + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, hw->size_emul); > + assert(start < hw->size_emul); > > *size = MIN(*size, hw->pending_emul); > *size = MIN(*size, hw->size_emul - start); > @@ -1415,13 +1407,10 @@ void audio_generic_put_buffer_in(HWVoiceIn *hw, void > *buf, size_t size) void audio_generic_run_buffer_out(HWVoiceOut *hw) > { > while (hw->pending_emul) { > - size_t write_len, written; > - ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul; > + size_t write_len, written, start; > > - if (start < 0) { > - start += hw->size_emul; > - } > - assert(start >= 0 && start < hw->size_emul); > + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, hw->size_emul); > + assert(start < hw->size_emul); > > write_len = MIN(hw->pending_emul, hw->size_emul - start); Just refactoring so far, which looks good so far. > diff --git a/audio/audio_int.h b/audio/audio_int.h > index 428a091d05..2fb459f34e 100644 > --- a/audio/audio_int.h > +++ b/audio/audio_int.h > @@ -266,6 +266,12 @@ static inline size_t audio_ring_dist(size_t dst, size_t src, size_t len) > return (dst >= src) ? (dst - src) : (len - src + dst); > } You haven't touched this function, but while I am looking at it, all function arguments are unsigned. So probably modulo operator might be used to get rid of a branch here. > > +/* return new position in backward direction at given distance */ > +static inline size_t audio_ring_posb(size_t pos, size_t dist, size_t len) > +{ > + return pos >= dist ? pos - dist : len - dist + pos; > +} > + Which is the exact same code as the already existing audio_ring_dist() function above, and I see that you actually already used this in v1 before: #define audio_ring_posb(pos, dist, len) audio_ring_dist(pos, dist, len) I would definitely not copy-paste the body. Thomas just suggested in v1 to add a comment, not to duplicate the actual math code: https://lore.kernel.org/qemu-devel/20220106111718.0ec25383@tuxfamily.org/ Also for consistency, I would have called the function audio_ring_rpos() and would have commented each argument. /** * Returns new position in ringbuffer in backward direction at given distance. * @pos: current position in ringbuffer * @dist: distance in ringbuffer to walk in reverse direction * @len: size of ringbuffer */ static inline size_t audio_ring_rpos(pos, dist, len) { return audio_ring_dist(pos, dist, len); } At least IMO a bit more comments on math code barely hurts. > #define dolog(fmt, ...) AUD_log(AUDIO_CAP, fmt, ## __VA_ARGS__) > > #ifdef DEBUG > diff --git a/audio/coreaudio.c b/audio/coreaudio.c > index d8a21d3e50..1fdd1d4b14 100644 > --- a/audio/coreaudio.c > +++ b/audio/coreaudio.c > @@ -333,12 +333,10 @@ static OSStatus audioDeviceIOProc( > > len = frameCount * hw->info.bytes_per_frame; > while (len) { > - size_t write_len; > - ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul; > - if (start < 0) { > - start += hw->size_emul; > - } > - assert(start >= 0 && start < hw->size_emul); > + size_t write_len, start; > + > + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, hw->size_emul); > + assert(start < hw->size_emul); > > write_len = MIN(MIN(hw->pending_emul, len), > hw->size_emul - start); > diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c > index c68c62a3e4..d6f3aa1a9a 100644 > --- a/audio/sdlaudio.c > +++ b/audio/sdlaudio.c > @@ -224,12 +224,11 @@ static void sdl_callback_out(void *opaque, Uint8 *buf, > int len) /* dolog("callback_out: len=%d avail=%zu\n", len, > hw->pending_emul); */ > > while (hw->pending_emul && len) { > - size_t write_len; > - ssize_t start = (ssize_t)hw->pos_emul - hw->pending_emul; > - if (start < 0) { > - start += hw->size_emul; > - } > - assert(start >= 0 && start < hw->size_emul); > + size_t write_len, start; > + > + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, > + hw->size_emul); > + assert(start < hw->size_emul); > > write_len = MIN(MIN(hw->pending_emul, len), > hw->size_emul - start); This rest looks fine to me. Best regards, Christian Schoenebeck
> On Samstag, 22. Januar 2022 13:57:31 CET Volker Rümelin wrote: >> Replace open-coded buffer arithmetic with the new function >> audio_ring_posb(). That's the position in backward direction >> of a given point at a given distance. >> >> Signed-off-by: Volker Rümelin<vr_qemu@t-online.de> >> --- > First of all, getting rid of all those redundant, open coded ringbuffer > traversal code places highly makes sense! > >> audio/audio.c | 25 +++++++------------------ >> audio/audio_int.h | 6 ++++++ >> audio/coreaudio.c | 10 ++++------ >> audio/sdlaudio.c | 11 +++++------ >> 4 files changed, 22 insertions(+), 30 deletions(-) >> >> diff --git a/audio/audio.c b/audio/audio.c >> index dc28685d22..e7a139e289 100644 >> --- a/audio/audio.c >> +++ b/audio/audio.c >> @@ -574,19 +574,13 @@ static size_t audio_pcm_sw_get_rpos_in(SWVoiceIn *sw) >> { >> HWVoiceIn *hw = sw->hw; >> ssize_t live = hw->total_samples_captured - sw->total_hw_samples_acquired; >> - ssize_t rpos; >> >> if (audio_bug(__func__, live < 0 || live > hw->conv_buf->size)) { >> dolog("live=%zu hw->conv_buf->size=%zu\n", live, hw->conv_buf->size); >> return 0; >> } >> >> - rpos = hw->conv_buf->pos - live; >> - if (rpos >= 0) { >> - return rpos; >> - } else { >> - return hw->conv_buf->size + rpos; >> - } >> + return audio_ring_posb(hw->conv_buf->pos, live, hw->conv_buf->size); >> } >> >> static size_t audio_pcm_sw_read(SWVoiceIn *sw, void *buf, size_t size) >> @@ -1394,12 +1388,10 @@ void audio_generic_run_buffer_in(HWVoiceIn *hw) >> >> void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size) >> { >> - ssize_t start = (ssize_t)hw->pos_emul - hw->pending_emul; >> + size_t start; >> >> - if (start < 0) { >> - start += hw->size_emul; >> - } >> - assert(start >= 0 && start < hw->size_emul); >> + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, hw->size_emul); >> + assert(start < hw->size_emul); >> >> *size = MIN(*size, hw->pending_emul); >> *size = MIN(*size, hw->size_emul - start); >> @@ -1415,13 +1407,10 @@ void audio_generic_put_buffer_in(HWVoiceIn *hw, void >> *buf, size_t size) void audio_generic_run_buffer_out(HWVoiceOut *hw) >> { >> while (hw->pending_emul) { >> - size_t write_len, written; >> - ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul; >> + size_t write_len, written, start; >> >> - if (start < 0) { >> - start += hw->size_emul; >> - } >> - assert(start >= 0 && start < hw->size_emul); >> + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, hw->size_emul); >> + assert(start < hw->size_emul); >> >> write_len = MIN(hw->pending_emul, hw->size_emul - start); > Just refactoring so far, which looks good so far. > >> diff --git a/audio/audio_int.h b/audio/audio_int.h >> index 428a091d05..2fb459f34e 100644 >> --- a/audio/audio_int.h >> +++ b/audio/audio_int.h >> @@ -266,6 +266,12 @@ static inline size_t audio_ring_dist(size_t dst, size_t src, size_t len) >> return (dst >= src) ? (dst - src) : (len - src + dst); >> } > You haven't touched this function, but while I am looking at it, all function > arguments are unsigned. So probably modulo operator might be used to get rid > of a branch here. That would be "return (len - dist + pos) % len;" but on my x86_64 system I always prefer a conditional move instruction to a 64 bit integer division instruction. >> +/* return new position in backward direction at given distance */ >> +static inline size_t audio_ring_posb(size_t pos, size_t dist, size_t len) >> +{ >> + return pos >= dist ? pos - dist : len - dist + pos; >> +} >> + > Which is the exact same code as the already existing audio_ring_dist() > function above, and I see that you actually already used this in v1 before: > > #define audio_ring_posb(pos, dist, len) audio_ring_dist(pos, dist, len) > > I would definitely not copy-paste the body. Thomas just suggested in v1 to add > a comment, not to duplicate the actual math code: > https://lore.kernel.org/qemu-devel/20220106111718.0ec25383@tuxfamily.org/ > > Also for consistency, I would have called the function audio_ring_rpos() > and would have commented each argument. In the audio subsystem rpos is typically the read position. I chose posb to distinguish it from read position. > /** > * Returns new position in ringbuffer in backward direction at given distance. > * @pos: current position in ringbuffer > * @dist: distance in ringbuffer to walk in reverse direction > * @len: size of ringbuffer > */ This comment is better than my comment. I'll use it in my v3 series. > static inline size_t audio_ring_rpos(pos, dist, len) { > return audio_ring_dist(pos, dist, len); > } I don't think this inline function improves readability compared to my macro from v1. To understand the code you still have to replace parameter names in your mind. My v2 inline function can be understood at first glance. With best regards, Volker > At least IMO a bit more comments on math code barely hurts. > >> #define dolog(fmt, ...) AUD_log(AUDIO_CAP, fmt, ## __VA_ARGS__) >> >> #ifdef DEBUG >> diff --git a/audio/coreaudio.c b/audio/coreaudio.c >> index d8a21d3e50..1fdd1d4b14 100644 >> --- a/audio/coreaudio.c >> +++ b/audio/coreaudio.c >> @@ -333,12 +333,10 @@ static OSStatus audioDeviceIOProc( >> >> len = frameCount * hw->info.bytes_per_frame; >> while (len) { >> - size_t write_len; >> - ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul; >> - if (start < 0) { >> - start += hw->size_emul; >> - } >> - assert(start >= 0 && start < hw->size_emul); >> + size_t write_len, start; >> + >> + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, hw->size_emul); >> + assert(start < hw->size_emul); >> >> write_len = MIN(MIN(hw->pending_emul, len), >> hw->size_emul - start); >> diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c >> index c68c62a3e4..d6f3aa1a9a 100644 >> --- a/audio/sdlaudio.c >> +++ b/audio/sdlaudio.c >> @@ -224,12 +224,11 @@ static void sdl_callback_out(void *opaque, Uint8 *buf, >> int len) /* dolog("callback_out: len=%d avail=%zu\n", len, >> hw->pending_emul); */ >> >> while (hw->pending_emul && len) { >> - size_t write_len; >> - ssize_t start = (ssize_t)hw->pos_emul - hw->pending_emul; >> - if (start < 0) { >> - start += hw->size_emul; >> - } >> - assert(start >= 0 && start < hw->size_emul); >> + size_t write_len, start; >> + >> + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, >> + hw->size_emul); >> + assert(start < hw->size_emul); >> >> write_len = MIN(MIN(hw->pending_emul, len), >> hw->size_emul - start); > This rest looks fine to me. > > Best regards, > Christian Schoenebeck > >
Hi, > > > --- a/audio/audio_int.h > > > +++ b/audio/audio_int.h > > > @@ -266,6 +266,12 @@ static inline size_t audio_ring_dist(size_t dst, size_t src, size_t len) > > > return (dst >= src) ? (dst - src) : (len - src + dst); > > > } > > You haven't touched this function, but while I am looking at it, all function > > arguments are unsigned. So probably modulo operator might be used to get rid > > of a branch here. > > That would be "return (len - dist + pos) % len;" but on my x86_64 system I > always prefer a conditional move instruction to a 64 bit integer division > instruction. Why? Performance? Don't underestimate the optimizer of a modern compiler. In many cases it simply isn't worth the effort. Better optimize the code for humans, i.e. make it easy to read and understand. take care, Gerd
On Dienstag, 25. Januar 2022 19:57:59 CET Volker Rümelin wrote: > > On Samstag, 22. Januar 2022 13:57:31 CET Volker Rümelin wrote: > >> Replace open-coded buffer arithmetic with the new function > >> audio_ring_posb(). That's the position in backward direction > >> of a given point at a given distance. > >> > >> Signed-off-by: Volker Rümelin<vr_qemu@t-online.de> > >> --- > > > > First of all, getting rid of all those redundant, open coded ringbuffer > > traversal code places highly makes sense! > > > >> audio/audio.c | 25 +++++++------------------ > >> audio/audio_int.h | 6 ++++++ > >> audio/coreaudio.c | 10 ++++------ > >> audio/sdlaudio.c | 11 +++++------ > >> 4 files changed, 22 insertions(+), 30 deletions(-) > >> > >> diff --git a/audio/audio.c b/audio/audio.c > >> index dc28685d22..e7a139e289 100644 > >> --- a/audio/audio.c > >> +++ b/audio/audio.c > >> @@ -574,19 +574,13 @@ static size_t audio_pcm_sw_get_rpos_in(SWVoiceIn > >> *sw) > >> > >> { > >> > >> HWVoiceIn *hw = sw->hw; > >> ssize_t live = hw->total_samples_captured - > >> sw->total_hw_samples_acquired; > >> > >> - ssize_t rpos; > >> > >> if (audio_bug(__func__, live < 0 || live > hw->conv_buf->size)) { > >> > >> dolog("live=%zu hw->conv_buf->size=%zu\n", live, > >> hw->conv_buf->size); > >> return 0; > >> > >> } > >> > >> - rpos = hw->conv_buf->pos - live; > >> - if (rpos >= 0) { > >> - return rpos; > >> - } else { > >> - return hw->conv_buf->size + rpos; > >> - } > >> + return audio_ring_posb(hw->conv_buf->pos, live, hw->conv_buf->size); > >> > >> } > >> > >> static size_t audio_pcm_sw_read(SWVoiceIn *sw, void *buf, size_t size) > >> > >> @@ -1394,12 +1388,10 @@ void audio_generic_run_buffer_in(HWVoiceIn *hw) > >> > >> void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size) > >> { > >> > >> - ssize_t start = (ssize_t)hw->pos_emul - hw->pending_emul; > >> + size_t start; > >> > >> - if (start < 0) { > >> - start += hw->size_emul; > >> - } > >> - assert(start >= 0 && start < hw->size_emul); > >> + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, > >> hw->size_emul); + assert(start < hw->size_emul); > >> > >> *size = MIN(*size, hw->pending_emul); > >> *size = MIN(*size, hw->size_emul - start); > >> > >> @@ -1415,13 +1407,10 @@ void audio_generic_put_buffer_in(HWVoiceIn *hw, > >> void *buf, size_t size) void audio_generic_run_buffer_out(HWVoiceOut > >> *hw)>> > >> { > >> > >> while (hw->pending_emul) { > >> > >> - size_t write_len, written; > >> - ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul; > >> + size_t write_len, written, start; > >> > >> - if (start < 0) { > >> - start += hw->size_emul; > >> - } > >> - assert(start >= 0 && start < hw->size_emul); > >> + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, > >> hw->size_emul); + assert(start < hw->size_emul); > >> > >> write_len = MIN(hw->pending_emul, hw->size_emul - start); > > > > Just refactoring so far, which looks good so far. > > > >> diff --git a/audio/audio_int.h b/audio/audio_int.h > >> index 428a091d05..2fb459f34e 100644 > >> --- a/audio/audio_int.h > >> +++ b/audio/audio_int.h > >> @@ -266,6 +266,12 @@ static inline size_t audio_ring_dist(size_t dst, > >> size_t src, size_t len)>> > >> return (dst >= src) ? (dst - src) : (len - src + dst); > >> > >> } > > > > You haven't touched this function, but while I am looking at it, all > > function arguments are unsigned. So probably modulo operator might be > > used to get rid of a branch here. > > That would be "return (len - dist + pos) % len;" but on my x86_64 system > I always prefer a conditional move instruction to a 64 bit integer > division instruction. Nevermind, I just mentioned it as a side note, here in QEMU it probably doesn't matter at all. In other apps this is sometimes used in tight loops where it can make a measurable difference to get rid of the branch(es). > > >> +/* return new position in backward direction at given distance */ > >> +static inline size_t audio_ring_posb(size_t pos, size_t dist, size_t > >> len) > >> +{ > >> + return pos >= dist ? pos - dist : len - dist + pos; > >> +} > >> + > > > > Which is the exact same code as the already existing audio_ring_dist() > > function above, and I see that you actually already used this in v1 > > before: > > > > #define audio_ring_posb(pos, dist, len) audio_ring_dist(pos, dist, len) > > > > I would definitely not copy-paste the body. Thomas just suggested in v1 to > > add a comment, not to duplicate the actual math code: > > https://lore.kernel.org/qemu-devel/20220106111718.0ec25383@tuxfamily.org/ > > > > Also for consistency, I would have called the function audio_ring_rpos() > > and would have commented each argument. > > In the audio subsystem rpos is typically the read position. I chose posb > to distinguish it from read position. > > > /** > > > > * Returns new position in ringbuffer in backward direction at given > > distance. * @pos: current position in ringbuffer > > * @dist: distance in ringbuffer to walk in reverse direction > > * @len: size of ringbuffer > > */ > > This comment is better than my comment. I'll use it in my v3 series. > > > static inline size_t audio_ring_rpos(pos, dist, len) { > > > > return audio_ring_dist(pos, dist, len); > > > > } > > I don't think this inline function improves readability compared to my > macro from v1. To understand the code you still have to replace > parameter names in your mind. My v2 inline function can be understood at > first glance. I didn't mean readability. Believe it or not, it took me a bit to realize that it was the exact same code as above. Best regards, Christian Schoenebeck
Am 26.01.22 um 09:48 schrieb Gerd Hoffmann: > Hi, > >>>> --- a/audio/audio_int.h >>>> +++ b/audio/audio_int.h >>>> @@ -266,6 +266,12 @@ static inline size_t audio_ring_dist(size_t dst, size_t src, size_t len) >>>> return (dst >= src) ? (dst - src) : (len - src + dst); >>>> } >>> You haven't touched this function, but while I am looking at it, all function >>> arguments are unsigned. So probably modulo operator might be used to get rid >>> of a branch here. >> That would be "return (len - dist + pos) % len;" but on my x86_64 system I >> always prefer a conditional move instruction to a 64 bit integer division >> instruction. > Why? Performance? > > Don't underestimate the optimizer of a modern compiler. In many cases > it simply isn't worth the effort. Better optimize the code for humans, > i.e. make it easy to read and understand. I know this micro optimization is useless. I will not try to defend it because there are no good reasons for it. But I can't see that "return (len - dist + pos) % len;" is more readable than "return pos >= dist ? pos - dist : len - dist + pos;". With best regards Volker > take care, > Gerd >
diff --git a/audio/audio.c b/audio/audio.c index dc28685d22..e7a139e289 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -574,19 +574,13 @@ static size_t audio_pcm_sw_get_rpos_in(SWVoiceIn *sw) { HWVoiceIn *hw = sw->hw; ssize_t live = hw->total_samples_captured - sw->total_hw_samples_acquired; - ssize_t rpos; if (audio_bug(__func__, live < 0 || live > hw->conv_buf->size)) { dolog("live=%zu hw->conv_buf->size=%zu\n", live, hw->conv_buf->size); return 0; } - rpos = hw->conv_buf->pos - live; - if (rpos >= 0) { - return rpos; - } else { - return hw->conv_buf->size + rpos; - } + return audio_ring_posb(hw->conv_buf->pos, live, hw->conv_buf->size); } static size_t audio_pcm_sw_read(SWVoiceIn *sw, void *buf, size_t size) @@ -1394,12 +1388,10 @@ void audio_generic_run_buffer_in(HWVoiceIn *hw) void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size) { - ssize_t start = (ssize_t)hw->pos_emul - hw->pending_emul; + size_t start; - if (start < 0) { - start += hw->size_emul; - } - assert(start >= 0 && start < hw->size_emul); + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, hw->size_emul); + assert(start < hw->size_emul); *size = MIN(*size, hw->pending_emul); *size = MIN(*size, hw->size_emul - start); @@ -1415,13 +1407,10 @@ void audio_generic_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size) void audio_generic_run_buffer_out(HWVoiceOut *hw) { while (hw->pending_emul) { - size_t write_len, written; - ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul; + size_t write_len, written, start; - if (start < 0) { - start += hw->size_emul; - } - assert(start >= 0 && start < hw->size_emul); + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, hw->size_emul); + assert(start < hw->size_emul); write_len = MIN(hw->pending_emul, hw->size_emul - start); diff --git a/audio/audio_int.h b/audio/audio_int.h index 428a091d05..2fb459f34e 100644 --- a/audio/audio_int.h +++ b/audio/audio_int.h @@ -266,6 +266,12 @@ static inline size_t audio_ring_dist(size_t dst, size_t src, size_t len) return (dst >= src) ? (dst - src) : (len - src + dst); } +/* return new position in backward direction at given distance */ +static inline size_t audio_ring_posb(size_t pos, size_t dist, size_t len) +{ + return pos >= dist ? pos - dist : len - dist + pos; +} + #define dolog(fmt, ...) AUD_log(AUDIO_CAP, fmt, ## __VA_ARGS__) #ifdef DEBUG diff --git a/audio/coreaudio.c b/audio/coreaudio.c index d8a21d3e50..1fdd1d4b14 100644 --- a/audio/coreaudio.c +++ b/audio/coreaudio.c @@ -333,12 +333,10 @@ static OSStatus audioDeviceIOProc( len = frameCount * hw->info.bytes_per_frame; while (len) { - size_t write_len; - ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul; - if (start < 0) { - start += hw->size_emul; - } - assert(start >= 0 && start < hw->size_emul); + size_t write_len, start; + + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, hw->size_emul); + assert(start < hw->size_emul); write_len = MIN(MIN(hw->pending_emul, len), hw->size_emul - start); diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c index c68c62a3e4..d6f3aa1a9a 100644 --- a/audio/sdlaudio.c +++ b/audio/sdlaudio.c @@ -224,12 +224,11 @@ static void sdl_callback_out(void *opaque, Uint8 *buf, int len) /* dolog("callback_out: len=%d avail=%zu\n", len, hw->pending_emul); */ while (hw->pending_emul && len) { - size_t write_len; - ssize_t start = (ssize_t)hw->pos_emul - hw->pending_emul; - if (start < 0) { - start += hw->size_emul; - } - assert(start >= 0 && start < hw->size_emul); + size_t write_len, start; + + start = audio_ring_posb(hw->pos_emul, hw->pending_emul, + hw->size_emul); + assert(start < hw->size_emul); write_len = MIN(MIN(hw->pending_emul, len), hw->size_emul - start);
Replace open-coded buffer arithmetic with the new function audio_ring_posb(). That's the position in backward direction of a given point at a given distance. Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> --- audio/audio.c | 25 +++++++------------------ audio/audio_int.h | 6 ++++++ audio/coreaudio.c | 10 ++++------ audio/sdlaudio.c | 11 +++++------ 4 files changed, 22 insertions(+), 30 deletions(-)