With current code audio recording with all audio backends
except PulseAudio and DirectSound is broken. The generic audio
recording buffer management forgot to update the current read
position after a read.
Fixes: ff095e5231 "audio: api for mixeng code free backends"
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
audio/audio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/audio/audio.c b/audio/audio.c
index 7fc3aa9d16..56fae55047 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1390,6 +1390,7 @@ void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size)
size_t read = hw->pcm_ops->read(hw, hw->buf_emul + hw->pos_emul,
read_len);
hw->pending_emul += read;
+ hw->pos_emul = (hw->pos_emul + read) % hw->size_emul;
if (read < read_len) {
break;
}
--
2.16.4
On 2019-11-19 07:58, Volker Rümelin wrote: > With current code audio recording with all audio backends > except PulseAudio and DirectSound is broken. The generic audio > recording buffer management forgot to update the current read > position after a read. Indeed, pos_emul is updated in audio_generic_put_buffer_out_nowrite but it's never updated on the capture end. I tested it with alsa and hda-micro and it fixes the problem (that is, if I add in.try-poll=off, but I need that for output too). Thanks. > > Fixes: ff095e5231 "audio: api for mixeng code free backends" > > Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> Reviewed-by: Zoltán Kővágó <DirtY.iCE.hu@gmail.com> > --- > audio/audio.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/audio/audio.c b/audio/audio.c > index 7fc3aa9d16..56fae55047 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -1390,6 +1390,7 @@ void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size) > size_t read = hw->pcm_ops->read(hw, hw->buf_emul + hw->pos_emul, > read_len); > hw->pending_emul += read; > + hw->pos_emul = (hw->pos_emul + read) % hw->size_emul; > if (read < read_len) { > break; > } >
On Tue, Nov 19, 2019 at 07:58:49AM +0100, Volker Rümelin wrote: > With current code audio recording with all audio backends > except PulseAudio and DirectSound is broken. The generic audio > recording buffer management forgot to update the current read > position after a read. > > Fixes: ff095e5231 "audio: api for mixeng code free backends" > > Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> Queued for 4.2 thanks, Gerd
Cc'ing Zoltán. On 11/19/19 7:58 AM, Volker Rümelin wrote: > With current code audio recording with all audio backends > except PulseAudio and DirectSound is broken. The generic audio > recording buffer management forgot to update the current read > position after a read. > > Fixes: ff095e5231 "audio: api for mixeng code free backends" > > Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> > --- > audio/audio.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/audio/audio.c b/audio/audio.c > index 7fc3aa9d16..56fae55047 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -1390,6 +1390,7 @@ void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size) > size_t read = hw->pcm_ops->read(hw, hw->buf_emul + hw->pos_emul, > read_len); > hw->pending_emul += read; > + hw->pos_emul = (hw->pos_emul + read) % hw->size_emul; Anyway since read() can return a negative value, both previous assignments should go after this if/break check... > if (read < read_len) { > break; > } >
On 11/19/19 9:01 AM, Philippe Mathieu-Daudé wrote: > Cc'ing Zoltán. > > On 11/19/19 7:58 AM, Volker Rümelin wrote: >> With current code audio recording with all audio backends >> except PulseAudio and DirectSound is broken. The generic audio >> recording buffer management forgot to update the current read >> position after a read. >> >> Fixes: ff095e5231 "audio: api for mixeng code free backends" >> >> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> >> --- >> audio/audio.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/audio/audio.c b/audio/audio.c >> index 7fc3aa9d16..56fae55047 100644 >> --- a/audio/audio.c >> +++ b/audio/audio.c >> @@ -1390,6 +1390,7 @@ void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t >> *size) >> size_t read = hw->pcm_ops->read(hw, hw->buf_emul + hw->pos_emul, >> read_len); >> hw->pending_emul += read; >> + hw->pos_emul = (hw->pos_emul + read) % hw->size_emul; > > Anyway since read() can return a negative value, both previous assignments > should go after this if/break check... This isn't read(2). size_t (*read) (HWVoiceIn *hw, void *buf, size_t size); Since this isn't ssize_t, no negative return value possible. r~
On 2019-11-19 20:43, Richard Henderson wrote: > On 11/19/19 9:01 AM, Philippe Mathieu-Daudé wrote: >> Cc'ing Zoltán. >> >> On 11/19/19 7:58 AM, Volker Rümelin wrote: >>> With current code audio recording with all audio backends >>> except PulseAudio and DirectSound is broken. The generic audio >>> recording buffer management forgot to update the current read >>> position after a read. >>> >>> Fixes: ff095e5231 "audio: api for mixeng code free backends" >>> >>> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> >>> --- >>> audio/audio.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/audio/audio.c b/audio/audio.c >>> index 7fc3aa9d16..56fae55047 100644 >>> --- a/audio/audio.c >>> +++ b/audio/audio.c >>> @@ -1390,6 +1390,7 @@ void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t >>> *size) >>> size_t read = hw->pcm_ops->read(hw, hw->buf_emul + hw->pos_emul, >>> read_len); >>> hw->pending_emul += read; >>> + hw->pos_emul = (hw->pos_emul + read) % hw->size_emul; >> >> Anyway since read() can return a negative value, both previous assignments >> should go after this if/break check... > > This isn't read(2). > > size_t (*read) (HWVoiceIn *hw, void *buf, size_t size); > > Since this isn't ssize_t, no negative return value possible. Yes, read failures are handled inside the backends. If the backend really can't read anything, it'll return zero, which is harmless here. Zoltan
© 2016 - 2024 Red Hat, Inc.