| Message ID | 1294185947-29250-1-git-send-email-michael@walle.cc |
|---|---|
| State | New |
| Headers | show |
Am 05.01.2011 01:05, Michael Walle wrote: > Refactor the volume mixing, so it can be reused for capturing devices. > Additionally, it removes superfluous multiplications with the nominal > volume within the hardware voice code path. At least based on tests done with the Musicpal, I see problems with this patch, volume control still fine here. Jan > > Signed-off-by: Michael Walle <michael@walle.cc> > --- > audio/alsaaudio.c | 2 +- > audio/audio.c | 11 ++++++----- > audio/audio_int.h | 2 +- > audio/dsoundaudio.c | 4 ++-- > audio/esdaudio.c | 3 +-- > audio/fmodaudio.c | 4 ++-- > audio/mixeng.c | 25 +++++++++++++++++++++++++ > audio/mixeng.h | 4 ++-- > audio/mixeng_template.h | 39 +++++++-------------------------------- > audio/ossaudio.c | 3 +-- > audio/paaudio.c | 2 +- > audio/spiceaudio.c | 5 ++--- > audio/winwaveaudio.c | 3 +-- > 13 files changed, 52 insertions(+), 55 deletions(-) > > diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c > index fef24f6..301a6af 100644 > --- a/audio/alsaaudio.c > +++ b/audio/alsaaudio.c > @@ -1094,7 +1094,7 @@ static int alsa_run_in (HWVoiceIn *hw) > } > } > > - hw->conv (dst, src, nread, &nominal_volume); > + hw->conv (dst, src, nread); > > src = advance (src, nread << hwshift); > dst += nread; > diff --git a/audio/audio.c b/audio/audio.c > index 1707446..1729c0b 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -104,7 +104,7 @@ static struct { > > static AudioState glob_audio_state; > > -struct mixeng_volume nominal_volume = { > +const struct mixeng_volume nominal_volume = { > .mute = 0, > #ifdef FLOAT_MIXENG > .r = 1.0, > @@ -702,13 +702,11 @@ void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len) > /* > * Capture > */ > -static void noop_conv (struct st_sample *dst, const void *src, > - int samples, struct mixeng_volume *vol) > +static void noop_conv (struct st_sample *dst, const void *src, int samples) > { > (void) src; > (void) dst; > (void) samples; > - (void) vol; > } > > static CaptureVoiceOut *audio_pcm_capture_find_specific ( > @@ -956,6 +954,8 @@ int audio_pcm_sw_read (SWVoiceIn *sw, void *buf, int size) > total += isamp; > } > > + mixeng_volume (sw->buf, ret, &sw->vol); > + > sw->clip (buf, sw->buf, ret); > sw->total_hw_samples_acquired += total; > return ret << sw->info.shift; > @@ -1037,7 +1037,8 @@ int audio_pcm_sw_write (SWVoiceOut *sw, void *buf, int size) > swlim = ((int64_t) dead << 32) / sw->ratio; > swlim = audio_MIN (swlim, samples); > if (swlim) { > - sw->conv (sw->buf, buf, swlim, &sw->vol); > + sw->conv (sw->buf, buf, swlim); > + mixeng_volume (sw->buf, swlim, &sw->vol); > } > > while (swlim) { > diff --git a/audio/audio_int.h b/audio/audio_int.h > index d66f2c3..2003f8b 100644 > --- a/audio/audio_int.h > +++ b/audio/audio_int.h > @@ -211,7 +211,7 @@ extern struct audio_driver esd_audio_driver; > extern struct audio_driver pa_audio_driver; > extern struct audio_driver spice_audio_driver; > extern struct audio_driver winwave_audio_driver; > -extern struct mixeng_volume nominal_volume; > +extern const struct mixeng_volume nominal_volume; > > void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as); > void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len); > diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c > index e547955..e2d89fd 100644 > --- a/audio/dsoundaudio.c > +++ b/audio/dsoundaudio.c > @@ -831,11 +831,11 @@ static int dsound_run_in (HWVoiceIn *hw) > decr = len1 + len2; > > if (p1 && len1) { > - hw->conv (hw->conv_buf + hw->wpos, p1, len1, &nominal_volume); > + hw->conv (hw->conv_buf + hw->wpos, p1, len1); > } > > if (p2 && len2) { > - hw->conv (hw->conv_buf, p2, len2, &nominal_volume); > + hw->conv (hw->conv_buf, p2, len2); > } > > dsound_unlock_in (dscb, p1, p2, blen1, blen2); > diff --git a/audio/esdaudio.c b/audio/esdaudio.c > index 9a1f2f8..ff97b39 100644 > --- a/audio/esdaudio.c > +++ b/audio/esdaudio.c > @@ -346,8 +346,7 @@ static void *qesd_thread_in (void *arg) > break; > } > > - hw->conv (hw->conv_buf + wpos, buf, nread >> hw->info.shift, > - &nominal_volume); > + hw->conv (hw->conv_buf + wpos, buf, nread >> hw->info.shift); > wpos = (wpos + chunk) % hw->samples; > to_grab -= chunk; > } > diff --git a/audio/fmodaudio.c b/audio/fmodaudio.c > index 7f08e14..c34cf53 100644 > --- a/audio/fmodaudio.c > +++ b/audio/fmodaudio.c > @@ -488,10 +488,10 @@ static int fmod_run_in (HWVoiceIn *hw) > decr = len1 + len2; > > if (p1 && blen1) { > - hw->conv (hw->conv_buf + hw->wpos, p1, len1, &nominal_volume); > + hw->conv (hw->conv_buf + hw->wpos, p1, len1); > } > if (p2 && len2) { > - hw->conv (hw->conv_buf, p2, len2, &nominal_volume); > + hw->conv (hw->conv_buf, p2, len2); > } > > fmod_unlock_sample (fmd->fmod_sample, p1, p2, blen1, blen2); > diff --git a/audio/mixeng.c b/audio/mixeng.c > index 9f1d93f..4a9e8eb 100644 > --- a/audio/mixeng.c > +++ b/audio/mixeng.c > @@ -333,3 +333,28 @@ void mixeng_clear (struct st_sample *buf, int len) > { > memset (buf, 0, len * sizeof (struct st_sample)); > } > + > +void mixeng_volume (struct st_sample *buf, int len, struct mixeng_volume *vol) > +{ > +#ifdef CONFIG_MIXEMU > + if (vol->mute) { > + mixeng_clear (buf, len); > + return; > + } > + > + while (len--) { > +#ifdef FLOAT_MIXENG > + buf->l = buf->l * vol->l; > + buf->r = buf->r * vol->r; > +#else > + buf->l = (buf->l * vol->l) >> 32; > + buf->r = (buf->r * vol->r) >> 32; > +#endif > + buf += 1; > + } > +#else > + (void) buf; > + (void) len; > + (void) vol; > +#endif > +} > diff --git a/audio/mixeng.h b/audio/mixeng.h > index 4af1dd9..9de443b 100644 > --- a/audio/mixeng.h > +++ b/audio/mixeng.h > @@ -33,8 +33,7 @@ struct mixeng_volume { int mute; int64_t r; int64_t l; }; > struct st_sample { int64_t l; int64_t r; }; > #endif > > -typedef void (t_sample) (struct st_sample *dst, const void *src, > - int samples, struct mixeng_volume *vol); > +typedef void (t_sample) (struct st_sample *dst, const void *src, int samples); > typedef void (f_sample) (void *dst, const struct st_sample *src, int samples); > > extern t_sample *mixeng_conv[2][2][2][3]; > @@ -47,5 +46,6 @@ void st_rate_flow_mix (void *opaque, struct st_sample *ibuf, struct st_sample *o > int *isamp, int *osamp); > void st_rate_stop (void *opaque); > void mixeng_clear (struct st_sample *buf, int len); > +void mixeng_volume (struct st_sample *buf, int len, struct mixeng_volume *vol); > > #endif /* mixeng.h */ > diff --git a/audio/mixeng_template.h b/audio/mixeng_template.h > index 5617705..a2d0ef8 100644 > --- a/audio/mixeng_template.h > +++ b/audio/mixeng_template.h > @@ -31,16 +31,6 @@ > #define HALF (IN_MAX >> 1) > #endif > > -#ifdef CONFIG_MIXEMU > -#ifdef FLOAT_MIXENG > -#define VOL(a, b) ((a) * (b)) > -#else > -#define VOL(a, b) ((a) * (b)) >> 32 > -#endif > -#else > -#define VOL(a, b) a > -#endif > - > #define ET glue (ENDIAN_CONVERSION, glue (_, IN_T)) > > #ifdef FLOAT_MIXENG > @@ -109,40 +99,26 @@ static inline IN_T glue (clip_, ET) (int64_t v) > #endif > > static void glue (glue (conv_, ET), _to_stereo) > - (struct st_sample *dst, const void *src, int samples, struct mixeng_volume *vol) > + (struct st_sample *dst, const void *src, int samples) > { > struct st_sample *out = dst; > IN_T *in = (IN_T *) src; > -#ifdef CONFIG_MIXEMU > - if (vol->mute) { > - mixeng_clear (dst, samples); > - return; > - } > -#else > - (void) vol; > -#endif > + > while (samples--) { > - out->l = VOL (glue (conv_, ET) (*in++), vol->l); > - out->r = VOL (glue (conv_, ET) (*in++), vol->r); > + out->l = glue (conv_, ET) (*in++); > + out->r = glue (conv_, ET) (*in++); > out += 1; > } > } > > static void glue (glue (conv_, ET), _to_mono) > - (struct st_sample *dst, const void *src, int samples, struct mixeng_volume *vol) > + (struct st_sample *dst, const void *src, int samples) > { > struct st_sample *out = dst; > IN_T *in = (IN_T *) src; > -#ifdef CONFIG_MIXEMU > - if (vol->mute) { > - mixeng_clear (dst, samples); > - return; > - } > -#else > - (void) vol; > -#endif > + > while (samples--) { > - out->l = VOL (glue (conv_, ET) (in[0]), vol->l); > + out->l = glue (conv_, ET) (in[0]); > out->r = out->l; > out += 1; > in += 1; > @@ -174,4 +150,3 @@ static void glue (glue (clip_, ET), _from_mono) > > #undef ET > #undef HALF > -#undef VOL > diff --git a/audio/ossaudio.c b/audio/ossaudio.c > index 0439ef5..1fda519 100644 > --- a/audio/ossaudio.c > +++ b/audio/ossaudio.c > @@ -784,8 +784,7 @@ static int oss_run_in (HWVoiceIn *hw) > hw->info.align + 1); > } > read_samples += nread >> hwshift; > - hw->conv (hw->conv_buf + bufs[i].add, p, nread >> hwshift, > - &nominal_volume); > + hw->conv (hw->conv_buf + bufs[i].add, p, nread >> hwshift); > } > > if (bufs[i].len - nread) { > diff --git a/audio/paaudio.c b/audio/paaudio.c > index ff71dac..9cf685d 100644 > --- a/audio/paaudio.c > +++ b/audio/paaudio.c > @@ -195,7 +195,7 @@ static void *qpa_thread_in (void *arg) > return NULL; > } > > - hw->conv (hw->conv_buf + wpos, buf, chunk, &nominal_volume); > + hw->conv (hw->conv_buf + wpos, buf, chunk); > wpos = (wpos + chunk) % hw->samples; > to_grab -= chunk; > } > diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c > index 373e4c4..a5c0d6b 100644 > --- a/audio/spiceaudio.c > +++ b/audio/spiceaudio.c > @@ -268,11 +268,10 @@ static int line_in_run (HWVoiceIn *hw) > len[1] = 0; > } > > - hw->conv (hw->conv_buf + hw->wpos, samples, len[0], &nominal_volume); > + hw->conv (hw->conv_buf + hw->wpos, samples, len[0]); > > if (len[1]) { > - hw->conv (hw->conv_buf, samples + len[0], len[1], > - &nominal_volume); > + hw->conv (hw->conv_buf, samples + len[0], len[1]); > } > > hw->wpos = (hw->wpos + num_samples) % hw->samples; > diff --git a/audio/winwaveaudio.c b/audio/winwaveaudio.c > index cdf483b..e5ad3c6 100644 > --- a/audio/winwaveaudio.c > +++ b/audio/winwaveaudio.c > @@ -581,8 +581,7 @@ static int winwave_run_in (HWVoiceIn *hw) > int conv = audio_MIN (left, decr); > hw->conv (hw->conv_buf + hw->wpos, > advance (wave->pcm_buf, wave->rpos << hw->info.shift), > - conv, > - &nominal_volume); > + conv); > > wave->rpos = (wave->rpos + conv) % hw->samples; > hw->wpos = (hw->wpos + conv) % hw->samples;
On Mon, 10 Jan 2011, Jan Kiszka wrote: > Am 05.01.2011 01:05, Michael Walle wrote: > > Refactor the volume mixing, so it can be reused for capturing devices. > > Additionally, it removes superfluous multiplications with the nominal > > volume within the hardware voice code path. > > At least based on tests done with the Musicpal, I see problems with this > patch, volume control still fine here. "I see problems" or "I see _no_ problems". [..snip..]
Am 10.01.2011 21:52, malc wrote: > On Mon, 10 Jan 2011, Jan Kiszka wrote: > >> Am 05.01.2011 01:05, Michael Walle wrote: >>> Refactor the volume mixing, so it can be reused for capturing devices. >>> Additionally, it removes superfluous multiplications with the nominal >>> volume within the hardware voice code path. >> >> At least based on tests done with the Musicpal, I see problems with this >> patch, volume control still fine here. > > "I see problems" or "I see _no_ problems". Sorry, "no problem" of course. Jan
The fixed point path for scaling in mixeng_volume() seems to be under by a factor of two, IMHO. The right shift should be by 31, not 32. (Because the volume, which I assume is a signed 32 bit integer, can be 0.5 at most). Does the code work with FLOAT_MIXENG undefined? - Wolfgang Schildbach -----Original Message----- From: qemu-devel-bounces+wschi=dolby.com@nongnu.org [mailto:qemu-devel-bounces+wschi=dolby.com@nongnu.org] On Behalf Of Jan Kiszka Sent: Monday, January 10, 2011 9:44 PM To: Michael Walle Cc: qemu-devel@nongnu.org Subject: [Qemu-devel] Re: [PATCH] audio: split sample conversion and volumemixing Am 05.01.2011 01:05, Michael Walle wrote: > Refactor the volume mixing, so it can be reused for capturing devices. > Additionally, it removes superfluous multiplications with the nominal > volume within the hardware voice code path. At least based on tests done with the Musicpal, I see problems with this patch, volume control still fine here. Jan > > Signed-off-by: Michael Walle <michael@walle.cc> > --- > audio/alsaaudio.c | 2 +- > audio/audio.c | 11 ++++++----- > audio/audio_int.h | 2 +- > audio/dsoundaudio.c | 4 ++-- > audio/esdaudio.c | 3 +-- > audio/fmodaudio.c | 4 ++-- > audio/mixeng.c | 25 +++++++++++++++++++++++++ > audio/mixeng.h | 4 ++-- > audio/mixeng_template.h | 39 +++++++-------------------------------- > audio/ossaudio.c | 3 +-- > audio/paaudio.c | 2 +- > audio/spiceaudio.c | 5 ++--- > audio/winwaveaudio.c | 3 +-- > 13 files changed, 52 insertions(+), 55 deletions(-) > > diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c index > fef24f6..301a6af 100644 > --- a/audio/alsaaudio.c > +++ b/audio/alsaaudio.c > @@ -1094,7 +1094,7 @@ static int alsa_run_in (HWVoiceIn *hw) > } > } > > - hw->conv (dst, src, nread, &nominal_volume); > + hw->conv (dst, src, nread); > > src = advance (src, nread << hwshift); > dst += nread; > diff --git a/audio/audio.c b/audio/audio.c index 1707446..1729c0b > 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -104,7 +104,7 @@ static struct { > > static AudioState glob_audio_state; > > -struct mixeng_volume nominal_volume = { > +const struct mixeng_volume nominal_volume = { > .mute = 0, > #ifdef FLOAT_MIXENG > .r = 1.0, > @@ -702,13 +702,11 @@ void audio_pcm_info_clear_buf (struct > audio_pcm_info *info, void *buf, int len) > /* > * Capture > */ > -static void noop_conv (struct st_sample *dst, const void *src, > - int samples, struct mixeng_volume *vol) > +static void noop_conv (struct st_sample *dst, const void *src, int > +samples) > { > (void) src; > (void) dst; > (void) samples; > - (void) vol; > } > > static CaptureVoiceOut *audio_pcm_capture_find_specific ( @@ -956,6 > +954,8 @@ int audio_pcm_sw_read (SWVoiceIn *sw, void *buf, int size) > total += isamp; > } > > + mixeng_volume (sw->buf, ret, &sw->vol); > + > sw->clip (buf, sw->buf, ret); > sw->total_hw_samples_acquired += total; > return ret << sw->info.shift; > @@ -1037,7 +1037,8 @@ int audio_pcm_sw_write (SWVoiceOut *sw, void *buf, int size) > swlim = ((int64_t) dead << 32) / sw->ratio; > swlim = audio_MIN (swlim, samples); > if (swlim) { > - sw->conv (sw->buf, buf, swlim, &sw->vol); > + sw->conv (sw->buf, buf, swlim); > + mixeng_volume (sw->buf, swlim, &sw->vol); > } > > while (swlim) { > diff --git a/audio/audio_int.h b/audio/audio_int.h index > d66f2c3..2003f8b 100644 > --- a/audio/audio_int.h > +++ b/audio/audio_int.h > @@ -211,7 +211,7 @@ extern struct audio_driver esd_audio_driver; > extern struct audio_driver pa_audio_driver; extern struct > audio_driver spice_audio_driver; extern struct audio_driver > winwave_audio_driver; -extern struct mixeng_volume nominal_volume; > +extern const struct mixeng_volume nominal_volume; > > void audio_pcm_init_info (struct audio_pcm_info *info, struct > audsettings *as); void audio_pcm_info_clear_buf (struct > audio_pcm_info *info, void *buf, int len); diff --git > a/audio/dsoundaudio.c b/audio/dsoundaudio.c index e547955..e2d89fd > 100644 > --- a/audio/dsoundaudio.c > +++ b/audio/dsoundaudio.c > @@ -831,11 +831,11 @@ static int dsound_run_in (HWVoiceIn *hw) > decr = len1 + len2; > > if (p1 && len1) { > - hw->conv (hw->conv_buf + hw->wpos, p1, len1, &nominal_volume); > + hw->conv (hw->conv_buf + hw->wpos, p1, len1); > } > > if (p2 && len2) { > - hw->conv (hw->conv_buf, p2, len2, &nominal_volume); > + hw->conv (hw->conv_buf, p2, len2); > } > > dsound_unlock_in (dscb, p1, p2, blen1, blen2); diff --git > a/audio/esdaudio.c b/audio/esdaudio.c index 9a1f2f8..ff97b39 100644 > --- a/audio/esdaudio.c > +++ b/audio/esdaudio.c > @@ -346,8 +346,7 @@ static void *qesd_thread_in (void *arg) > break; > } > > - hw->conv (hw->conv_buf + wpos, buf, nread >> hw->info.shift, > - &nominal_volume); > + hw->conv (hw->conv_buf + wpos, buf, nread >> > + hw->info.shift); > wpos = (wpos + chunk) % hw->samples; > to_grab -= chunk; > } > diff --git a/audio/fmodaudio.c b/audio/fmodaudio.c index > 7f08e14..c34cf53 100644 > --- a/audio/fmodaudio.c > +++ b/audio/fmodaudio.c > @@ -488,10 +488,10 @@ static int fmod_run_in (HWVoiceIn *hw) > decr = len1 + len2; > > if (p1 && blen1) { > - hw->conv (hw->conv_buf + hw->wpos, p1, len1, &nominal_volume); > + hw->conv (hw->conv_buf + hw->wpos, p1, len1); > } > if (p2 && len2) { > - hw->conv (hw->conv_buf, p2, len2, &nominal_volume); > + hw->conv (hw->conv_buf, p2, len2); > } > > fmod_unlock_sample (fmd->fmod_sample, p1, p2, blen1, blen2); diff > --git a/audio/mixeng.c b/audio/mixeng.c index 9f1d93f..4a9e8eb 100644 > --- a/audio/mixeng.c > +++ b/audio/mixeng.c > @@ -333,3 +333,28 @@ void mixeng_clear (struct st_sample *buf, int > len) { > memset (buf, 0, len * sizeof (struct st_sample)); } > + > +void mixeng_volume (struct st_sample *buf, int len, struct > +mixeng_volume *vol) { #ifdef CONFIG_MIXEMU > + if (vol->mute) { > + mixeng_clear (buf, len); > + return; > + } > + > + while (len--) { > +#ifdef FLOAT_MIXENG > + buf->l = buf->l * vol->l; > + buf->r = buf->r * vol->r; > +#else > + buf->l = (buf->l * vol->l) >> 32; > + buf->r = (buf->r * vol->r) >> 32; #endif > + buf += 1; > + } > +#else > + (void) buf; > + (void) len; > + (void) vol; > +#endif > +} > diff --git a/audio/mixeng.h b/audio/mixeng.h index 4af1dd9..9de443b > 100644 > --- a/audio/mixeng.h > +++ b/audio/mixeng.h > @@ -33,8 +33,7 @@ struct mixeng_volume { int mute; int64_t r; int64_t > l; }; struct st_sample { int64_t l; int64_t r; }; #endif > > -typedef void (t_sample) (struct st_sample *dst, const void *src, > - int samples, struct mixeng_volume *vol); > +typedef void (t_sample) (struct st_sample *dst, const void *src, int > +samples); > typedef void (f_sample) (void *dst, const struct st_sample *src, int > samples); > > extern t_sample *mixeng_conv[2][2][2][3]; @@ -47,5 +46,6 @@ void > st_rate_flow_mix (void *opaque, struct st_sample *ibuf, struct st_sample *o > int *isamp, int *osamp); void st_rate_stop > (void *opaque); void mixeng_clear (struct st_sample *buf, int len); > +void mixeng_volume (struct st_sample *buf, int len, struct > +mixeng_volume *vol); > > #endif /* mixeng.h */ > diff --git a/audio/mixeng_template.h b/audio/mixeng_template.h index > 5617705..a2d0ef8 100644 > --- a/audio/mixeng_template.h > +++ b/audio/mixeng_template.h > @@ -31,16 +31,6 @@ > #define HALF (IN_MAX >> 1) > #endif > > -#ifdef CONFIG_MIXEMU > -#ifdef FLOAT_MIXENG > -#define VOL(a, b) ((a) * (b)) > -#else > -#define VOL(a, b) ((a) * (b)) >> 32 > -#endif > -#else > -#define VOL(a, b) a > -#endif > - > #define ET glue (ENDIAN_CONVERSION, glue (_, IN_T)) > > #ifdef FLOAT_MIXENG > @@ -109,40 +99,26 @@ static inline IN_T glue (clip_, ET) (int64_t v) > #endif > > static void glue (glue (conv_, ET), _to_stereo) > - (struct st_sample *dst, const void *src, int samples, struct mixeng_volume *vol) > + (struct st_sample *dst, const void *src, int samples) > { > struct st_sample *out = dst; > IN_T *in = (IN_T *) src; > -#ifdef CONFIG_MIXEMU > - if (vol->mute) { > - mixeng_clear (dst, samples); > - return; > - } > -#else > - (void) vol; > -#endif > + > while (samples--) { > - out->l = VOL (glue (conv_, ET) (*in++), vol->l); > - out->r = VOL (glue (conv_, ET) (*in++), vol->r); > + out->l = glue (conv_, ET) (*in++); > + out->r = glue (conv_, ET) (*in++); > out += 1; > } > } > > static void glue (glue (conv_, ET), _to_mono) > - (struct st_sample *dst, const void *src, int samples, struct mixeng_volume *vol) > + (struct st_sample *dst, const void *src, int samples) > { > struct st_sample *out = dst; > IN_T *in = (IN_T *) src; > -#ifdef CONFIG_MIXEMU > - if (vol->mute) { > - mixeng_clear (dst, samples); > - return; > - } > -#else > - (void) vol; > -#endif > + > while (samples--) { > - out->l = VOL (glue (conv_, ET) (in[0]), vol->l); > + out->l = glue (conv_, ET) (in[0]); > out->r = out->l; > out += 1; > in += 1; > @@ -174,4 +150,3 @@ static void glue (glue (clip_, ET), _from_mono) > > #undef ET > #undef HALF > -#undef VOL > diff --git a/audio/ossaudio.c b/audio/ossaudio.c index > 0439ef5..1fda519 100644 > --- a/audio/ossaudio.c > +++ b/audio/ossaudio.c > @@ -784,8 +784,7 @@ static int oss_run_in (HWVoiceIn *hw) > hw->info.align + 1); > } > read_samples += nread >> hwshift; > - hw->conv (hw->conv_buf + bufs[i].add, p, nread >> hwshift, > - &nominal_volume); > + hw->conv (hw->conv_buf + bufs[i].add, p, nread >> > + hwshift); > } > > if (bufs[i].len - nread) { diff --git a/audio/paaudio.c > b/audio/paaudio.c index ff71dac..9cf685d 100644 > --- a/audio/paaudio.c > +++ b/audio/paaudio.c > @@ -195,7 +195,7 @@ static void *qpa_thread_in (void *arg) > return NULL; > } > > - hw->conv (hw->conv_buf + wpos, buf, chunk, &nominal_volume); > + hw->conv (hw->conv_buf + wpos, buf, chunk); > wpos = (wpos + chunk) % hw->samples; > to_grab -= chunk; > } > diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c index > 373e4c4..a5c0d6b 100644 > --- a/audio/spiceaudio.c > +++ b/audio/spiceaudio.c > @@ -268,11 +268,10 @@ static int line_in_run (HWVoiceIn *hw) > len[1] = 0; > } > > - hw->conv (hw->conv_buf + hw->wpos, samples, len[0], &nominal_volume); > + hw->conv (hw->conv_buf + hw->wpos, samples, len[0]); > > if (len[1]) { > - hw->conv (hw->conv_buf, samples + len[0], len[1], > - &nominal_volume); > + hw->conv (hw->conv_buf, samples + len[0], len[1]); > } > > hw->wpos = (hw->wpos + num_samples) % hw->samples; diff --git > a/audio/winwaveaudio.c b/audio/winwaveaudio.c index cdf483b..e5ad3c6 > 100644 > --- a/audio/winwaveaudio.c > +++ b/audio/winwaveaudio.c > @@ -581,8 +581,7 @@ static int winwave_run_in (HWVoiceIn *hw) > int conv = audio_MIN (left, decr); > hw->conv (hw->conv_buf + hw->wpos, > advance (wave->pcm_buf, wave->rpos << hw->info.shift), > - conv, > - &nominal_volume); > + conv); > > wave->rpos = (wave->rpos + conv) % hw->samples; > hw->wpos = (hw->wpos + conv) % hw->samples;
Hi Wolfgang, > The fixed point path for scaling in mixeng_volume() seems to be under by > a factor of two, IMHO. The right shift should be by 31, not 32. (Because > the volume, which I assume is a signed 32 bit integer, can be 0.5 at > most). I must admit i just copied the shift from the original define. But the volume and the samples are signed 64bit values. @malc: any comments on this? > Does the code work with FLOAT_MIXENG undefined? At least for my input voice, it works with integer arithmetic.
On Tue, 11 Jan 2011, Michael Walle wrote: > > Hi Wolfgang, > > > The fixed point path for scaling in mixeng_volume() seems to be under by > > a factor of two, IMHO. The right shift should be by 31, not 32. (Because > > the volume, which I assume is a signed 32 bit integer, can be 0.5 at > > most). > I must admit i just copied the shift from the original define. But the volume > and the samples are signed 64bit values. > > @malc: any comments on this? I don't understand the problem, the result of multiplication is converted from 32.32 fixed point to integer by shifting right by 32.. It escapes me how 31 fits here. > > > Does the code work with FLOAT_MIXENG undefined? > At least for my input voice, it works with integer arithmetic. > >
Ah, I assumed that the volume would be stored in a 32 bit integer, and be represented as a signed Q31 fixed point. Sorry for the noise. - Wolfgang -----Original Message----- From: malc [mailto:av1474@comtv.ru] Sent: Wednesday, January 12, 2011 12:15 AM To: Michael Walle Cc: Schildbach, Wolfgang; Jan Kiszka; qemu-devel@nongnu.org Subject: Re: [PATCH] audio: split sample conversion and volumemixing On Tue, 11 Jan 2011, Michael Walle wrote: > > Hi Wolfgang, > > > The fixed point path for scaling in mixeng_volume() seems to be > > under by a factor of two, IMHO. The right shift should be by 31, not > > 32. (Because the volume, which I assume is a signed 32 bit integer, > > can be 0.5 at most). > I must admit i just copied the shift from the original define. But the > volume and the samples are signed 64bit values. > > @malc: any comments on this? I don't understand the problem, the result of multiplication is converted from 32.32 fixed point to integer by shifting right by 32.. It escapes me how 31 fits here. > > > Does the code work with FLOAT_MIXENG undefined? > At least for my input voice, it works with integer arithmetic. > > -- mailto:av1474@comtv.ru
On Wed, 5 Jan 2011, Michael Walle wrote: > Refactor the volume mixing, so it can be reused for capturing devices. > Additionally, it removes superfluous multiplications with the nominal > volume within the hardware voice code path. Thanks, applied. [..snip..]
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c index fef24f6..301a6af 100644 --- a/audio/alsaaudio.c +++ b/audio/alsaaudio.c @@ -1094,7 +1094,7 @@ static int alsa_run_in (HWVoiceIn *hw) } } - hw->conv (dst, src, nread, &nominal_volume); + hw->conv (dst, src, nread); src = advance (src, nread << hwshift); dst += nread; diff --git a/audio/audio.c b/audio/audio.c index 1707446..1729c0b 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -104,7 +104,7 @@ static struct { static AudioState glob_audio_state; -struct mixeng_volume nominal_volume = { +const struct mixeng_volume nominal_volume = { .mute = 0, #ifdef FLOAT_MIXENG .r = 1.0, @@ -702,13 +702,11 @@ void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len) /* * Capture */ -static void noop_conv (struct st_sample *dst, const void *src, - int samples, struct mixeng_volume *vol) +static void noop_conv (struct st_sample *dst, const void *src, int samples) { (void) src; (void) dst; (void) samples; - (void) vol; } static CaptureVoiceOut *audio_pcm_capture_find_specific ( @@ -956,6 +954,8 @@ int audio_pcm_sw_read (SWVoiceIn *sw, void *buf, int size) total += isamp; } + mixeng_volume (sw->buf, ret, &sw->vol); + sw->clip (buf, sw->buf, ret); sw->total_hw_samples_acquired += total; return ret << sw->info.shift; @@ -1037,7 +1037,8 @@ int audio_pcm_sw_write (SWVoiceOut *sw, void *buf, int size) swlim = ((int64_t) dead << 32) / sw->ratio; swlim = audio_MIN (swlim, samples); if (swlim) { - sw->conv (sw->buf, buf, swlim, &sw->vol); + sw->conv (sw->buf, buf, swlim); + mixeng_volume (sw->buf, swlim, &sw->vol); } while (swlim) { diff --git a/audio/audio_int.h b/audio/audio_int.h index d66f2c3..2003f8b 100644 --- a/audio/audio_int.h +++ b/audio/audio_int.h @@ -211,7 +211,7 @@ extern struct audio_driver esd_audio_driver; extern struct audio_driver pa_audio_driver; extern struct audio_driver spice_audio_driver; extern struct audio_driver winwave_audio_driver; -extern struct mixeng_volume nominal_volume; +extern const struct mixeng_volume nominal_volume; void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as); void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len); diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c index e547955..e2d89fd 100644 --- a/audio/dsoundaudio.c +++ b/audio/dsoundaudio.c @@ -831,11 +831,11 @@ static int dsound_run_in (HWVoiceIn *hw) decr = len1 + len2; if (p1 && len1) { - hw->conv (hw->conv_buf + hw->wpos, p1, len1, &nominal_volume); + hw->conv (hw->conv_buf + hw->wpos, p1, len1); } if (p2 && len2) { - hw->conv (hw->conv_buf, p2, len2, &nominal_volume); + hw->conv (hw->conv_buf, p2, len2); } dsound_unlock_in (dscb, p1, p2, blen1, blen2); diff --git a/audio/esdaudio.c b/audio/esdaudio.c index 9a1f2f8..ff97b39 100644 --- a/audio/esdaudio.c +++ b/audio/esdaudio.c @@ -346,8 +346,7 @@ static void *qesd_thread_in (void *arg) break; } - hw->conv (hw->conv_buf + wpos, buf, nread >> hw->info.shift, - &nominal_volume); + hw->conv (hw->conv_buf + wpos, buf, nread >> hw->info.shift); wpos = (wpos + chunk) % hw->samples; to_grab -= chunk; } diff --git a/audio/fmodaudio.c b/audio/fmodaudio.c index 7f08e14..c34cf53 100644 --- a/audio/fmodaudio.c +++ b/audio/fmodaudio.c @@ -488,10 +488,10 @@ static int fmod_run_in (HWVoiceIn *hw) decr = len1 + len2; if (p1 && blen1) { - hw->conv (hw->conv_buf + hw->wpos, p1, len1, &nominal_volume); + hw->conv (hw->conv_buf + hw->wpos, p1, len1); } if (p2 && len2) { - hw->conv (hw->conv_buf, p2, len2, &nominal_volume); + hw->conv (hw->conv_buf, p2, len2); } fmod_unlock_sample (fmd->fmod_sample, p1, p2, blen1, blen2); diff --git a/audio/mixeng.c b/audio/mixeng.c index 9f1d93f..4a9e8eb 100644 --- a/audio/mixeng.c +++ b/audio/mixeng.c @@ -333,3 +333,28 @@ void mixeng_clear (struct st_sample *buf, int len) { memset (buf, 0, len * sizeof (struct st_sample)); } + +void mixeng_volume (struct st_sample *buf, int len, struct mixeng_volume *vol) +{ +#ifdef CONFIG_MIXEMU + if (vol->mute) { + mixeng_clear (buf, len); + return; + } + + while (len--) { +#ifdef FLOAT_MIXENG + buf->l = buf->l * vol->l; + buf->r = buf->r * vol->r; +#else + buf->l = (buf->l * vol->l) >> 32; + buf->r = (buf->r * vol->r) >> 32; +#endif + buf += 1; + } +#else + (void) buf; + (void) len; + (void) vol; +#endif +} diff --git a/audio/mixeng.h b/audio/mixeng.h index 4af1dd9..9de443b 100644 --- a/audio/mixeng.h +++ b/audio/mixeng.h @@ -33,8 +33,7 @@ struct mixeng_volume { int mute; int64_t r; int64_t l; }; struct st_sample { int64_t l; int64_t r; }; #endif -typedef void (t_sample) (struct st_sample *dst, const void *src, - int samples, struct mixeng_volume *vol); +typedef void (t_sample) (struct st_sample *dst, const void *src, int samples); typedef void (f_sample) (void *dst, const struct st_sample *src, int samples); extern t_sample *mixeng_conv[2][2][2][3]; @@ -47,5 +46,6 @@ void st_rate_flow_mix (void *opaque, struct st_sample *ibuf, struct st_sample *o int *isamp, int *osamp); void st_rate_stop (void *opaque); void mixeng_clear (struct st_sample *buf, int len); +void mixeng_volume (struct st_sample *buf, int len, struct mixeng_volume *vol); #endif /* mixeng.h */ diff --git a/audio/mixeng_template.h b/audio/mixeng_template.h index 5617705..a2d0ef8 100644 --- a/audio/mixeng_template.h +++ b/audio/mixeng_template.h @@ -31,16 +31,6 @@ #define HALF (IN_MAX >> 1) #endif -#ifdef CONFIG_MIXEMU -#ifdef FLOAT_MIXENG -#define VOL(a, b) ((a) * (b)) -#else -#define VOL(a, b) ((a) * (b)) >> 32 -#endif -#else -#define VOL(a, b) a -#endif - #define ET glue (ENDIAN_CONVERSION, glue (_, IN_T)) #ifdef FLOAT_MIXENG @@ -109,40 +99,26 @@ static inline IN_T glue (clip_, ET) (int64_t v) #endif static void glue (glue (conv_, ET), _to_stereo) - (struct st_sample *dst, const void *src, int samples, struct mixeng_volume *vol) + (struct st_sample *dst, const void *src, int samples) { struct st_sample *out = dst; IN_T *in = (IN_T *) src; -#ifdef CONFIG_MIXEMU - if (vol->mute) { - mixeng_clear (dst, samples); - return; - } -#else - (void) vol; -#endif + while (samples--) { - out->l = VOL (glue (conv_, ET) (*in++), vol->l); - out->r = VOL (glue (conv_, ET) (*in++), vol->r); + out->l = glue (conv_, ET) (*in++); + out->r = glue (conv_, ET) (*in++); out += 1; } } static void glue (glue (conv_, ET), _to_mono) - (struct st_sample *dst, const void *src, int samples, struct mixeng_volume *vol) + (struct st_sample *dst, const void *src, int samples) { struct st_sample *out = dst; IN_T *in = (IN_T *) src; -#ifdef CONFIG_MIXEMU - if (vol->mute) { - mixeng_clear (dst, samples); - return; - } -#else - (void) vol; -#endif + while (samples--) { - out->l = VOL (glue (conv_, ET) (in[0]), vol->l); + out->l = glue (conv_, ET) (in[0]); out->r = out->l; out += 1; in += 1; @@ -174,4 +150,3 @@ static void glue (glue (clip_, ET), _from_mono) #undef ET #undef HALF -#undef VOL diff --git a/audio/ossaudio.c b/audio/ossaudio.c index 0439ef5..1fda519 100644 --- a/audio/ossaudio.c +++ b/audio/ossaudio.c @@ -784,8 +784,7 @@ static int oss_run_in (HWVoiceIn *hw) hw->info.align + 1); } read_samples += nread >> hwshift; - hw->conv (hw->conv_buf + bufs[i].add, p, nread >> hwshift, - &nominal_volume); + hw->conv (hw->conv_buf + bufs[i].add, p, nread >> hwshift); } if (bufs[i].len - nread) { diff --git a/audio/paaudio.c b/audio/paaudio.c index ff71dac..9cf685d 100644 --- a/audio/paaudio.c +++ b/audio/paaudio.c @@ -195,7 +195,7 @@ static void *qpa_thread_in (void *arg) return NULL; } - hw->conv (hw->conv_buf + wpos, buf, chunk, &nominal_volume); + hw->conv (hw->conv_buf + wpos, buf, chunk); wpos = (wpos + chunk) % hw->samples; to_grab -= chunk; } diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c index 373e4c4..a5c0d6b 100644 --- a/audio/spiceaudio.c +++ b/audio/spiceaudio.c @@ -268,11 +268,10 @@ static int line_in_run (HWVoiceIn *hw) len[1] = 0; } - hw->conv (hw->conv_buf + hw->wpos, samples, len[0], &nominal_volume); + hw->conv (hw->conv_buf + hw->wpos, samples, len[0]); if (len[1]) { - hw->conv (hw->conv_buf, samples + len[0], len[1], - &nominal_volume); + hw->conv (hw->conv_buf, samples + len[0], len[1]); } hw->wpos = (hw->wpos + num_samples) % hw->samples; diff --git a/audio/winwaveaudio.c b/audio/winwaveaudio.c index cdf483b..e5ad3c6 100644 --- a/audio/winwaveaudio.c +++ b/audio/winwaveaudio.c @@ -581,8 +581,7 @@ static int winwave_run_in (HWVoiceIn *hw) int conv = audio_MIN (left, decr); hw->conv (hw->conv_buf + hw->wpos, advance (wave->pcm_buf, wave->rpos << hw->info.shift), - conv, - &nominal_volume); + conv); wave->rpos = (wave->rpos + conv) % hw->samples; hw->wpos = (hw->wpos + conv) % hw->samples;
Refactor the volume mixing, so it can be reused for capturing devices. Additionally, it removes superfluous multiplications with the nominal volume within the hardware voice code path. Signed-off-by: Michael Walle <michael@walle.cc> --- audio/alsaaudio.c | 2 +- audio/audio.c | 11 ++++++----- audio/audio_int.h | 2 +- audio/dsoundaudio.c | 4 ++-- audio/esdaudio.c | 3 +-- audio/fmodaudio.c | 4 ++-- audio/mixeng.c | 25 +++++++++++++++++++++++++ audio/mixeng.h | 4 ++-- audio/mixeng_template.h | 39 +++++++-------------------------------- audio/ossaudio.c | 3 +-- audio/paaudio.c | 2 +- audio/spiceaudio.c | 5 ++--- audio/winwaveaudio.c | 3 +-- 13 files changed, 52 insertions(+), 55 deletions(-)