Re: [PATCH] winealsa: fix input sound distortion and heap corruption
On Fri, Nov 07, 2014 at 04:06:55AM +0000, Mark Harmstone wrote:
At present, winealsa attempts to call map_channels when a capture device is opened. If a microphone can be opened in either stereo or mono mode, and mono is requested, Wine will still return stereo data. Not only does this result in distorted in audio, it also very frequently results in heap corruption when snd_pcm_readi overruns the buffer allocated by the program.
Thanks for fixing this. The patch doesn't apply because it got mangled in transit: http://source.winehq.org/patches/data/107529 The idea look OK to me, but don't we want to disable channel remapping for _all_ capture devices? Your patch only disables it if the application is using WAVE_FORMAT_EXTENSIBLE I don't think we can do capture channel remapping reliably without switching to the new ALSA channel mapping API. So it makes sense to disable it entirely for now. Andrew
I don't have a copy of World of Warcraft to test this on, but this may also fix bug 31684. It's the same symptoms, at least.
--- dlls/winealsa.drv/mmdevdrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c index a4c02bf..91fba2f 100644 --- a/dlls/winealsa.drv/mmdevdrv.c +++ b/dlls/winealsa.drv/mmdevdrv.c @@ -1164,7 +1164,7 @@ static DWORD get_channel_mask(unsigned int channels)
static HRESULT map_channels(ACImpl *This, const WAVEFORMATEX *fmt) { - if(fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE || fmt->nChannels > 2){ + if( (This->dataflow != eCapture && fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE) || fmt->nChannels > 2){ WAVEFORMATEXTENSIBLE *fmtex = (void*)fmt; DWORD mask, flag = SPEAKER_FRONT_LEFT; UINT i = 0; -- 2.0.4
Thanks. I've attached a non-mangled version, changed so it excludes all capture devices from remapping. On 07/11/14 14:12, Andrew Eikum wrote:
On Fri, Nov 07, 2014 at 04:06:55AM +0000, Mark Harmstone wrote:
At present, winealsa attempts to call map_channels when a capture device is opened. If a microphone can be opened in either stereo or mono mode, and mono is requested, Wine will still return stereo data. Not only does this result in distorted in audio, it also very frequently results in heap corruption when snd_pcm_readi overruns the buffer allocated by the program.
Thanks for fixing this.
The patch doesn't apply because it got mangled in transit:
http://source.winehq.org/patches/data/107529
The idea look OK to me, but don't we want to disable channel remapping for _all_ capture devices? Your patch only disables it if the application is using WAVE_FORMAT_EXTENSIBLE
I don't think we can do capture channel remapping reliably without switching to the new ALSA channel mapping API. So it makes sense to disable it entirely for now.
Andrew
I don't have a copy of World of Warcraft to test this on, but this may also fix bug 31684. It's the same symptoms, at least.
--- dlls/winealsa.drv/mmdevdrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c index a4c02bf..91fba2f 100644 --- a/dlls/winealsa.drv/mmdevdrv.c +++ b/dlls/winealsa.drv/mmdevdrv.c @@ -1164,7 +1164,7 @@ static DWORD get_channel_mask(unsigned int channels)
static HRESULT map_channels(ACImpl *This, const WAVEFORMATEX *fmt) { - if(fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE || fmt->nChannels > 2){ + if( (This->dataflow != eCapture && fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE) || fmt->nChannels > 2){ WAVEFORMATEXTENSIBLE *fmtex = (void*)fmt; DWORD mask, flag = SPEAKER_FRONT_LEFT; UINT i = 0; -- 2.0.4
On Fri, Nov 07, 2014 at 02:38:00PM +0000, Mark Harmstone wrote:
Thanks. I've attached a non-mangled version, changed so it excludes all capture devices from remapping.
Looks good. Make sure to throw it at wine-patches for inclusion. Also, I'd tweak the subject to something more descriptive like "Disable channel remapping for capture devices" or whatever. Andrew
On 07/11/14 14:12, Andrew Eikum wrote:
On Fri, Nov 07, 2014 at 04:06:55AM +0000, Mark Harmstone wrote:
At present, winealsa attempts to call map_channels when a capture device is opened. If a microphone can be opened in either stereo or mono mode, and mono is requested, Wine will still return stereo data. Not only does this result in distorted in audio, it also very frequently results in heap corruption when snd_pcm_readi overruns the buffer allocated by the program.
Thanks for fixing this.
The patch doesn't apply because it got mangled in transit:
http://source.winehq.org/patches/data/107529
The idea look OK to me, but don't we want to disable channel remapping for _all_ capture devices? Your patch only disables it if the application is using WAVE_FORMAT_EXTENSIBLE
I don't think we can do capture channel remapping reliably without switching to the new ALSA channel mapping API. So it makes sense to disable it entirely for now.
Andrew
I don't have a copy of World of Warcraft to test this on, but this may also fix bug 31684. It's the same symptoms, at least.
--- dlls/winealsa.drv/mmdevdrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c index a4c02bf..91fba2f 100644 --- a/dlls/winealsa.drv/mmdevdrv.c +++ b/dlls/winealsa.drv/mmdevdrv.c @@ -1164,7 +1164,7 @@ static DWORD get_channel_mask(unsigned int channels)
static HRESULT map_channels(ACImpl *This, const WAVEFORMATEX *fmt) { - if(fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE || fmt->nChannels > 2){ + if( (This->dataflow != eCapture && fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE) || fmt->nChannels > 2){ WAVEFORMATEXTENSIBLE *fmtex = (void*)fmt; DWORD mask, flag = SPEAKER_FRONT_LEFT; UINT i = 0; -- 2.0.4
From 1b07897dd4da96dc1ede03ebe47a4d4a50b7a68a Mon Sep 17 00:00:00 2001 From: Mark Harmstone <mark(a)harmstone.com> Date: Fri, 7 Nov 2014 03:41:35 +0000 Subject: [PATCH] winealsa: fix input sound distortion and heap corruption
--- dlls/winealsa.drv/mmdevdrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c index a4c02bf..91fba2f 100644 --- a/dlls/winealsa.drv/mmdevdrv.c +++ b/dlls/winealsa.drv/mmdevdrv.c @@ -1164,7 +1164,7 @@ static DWORD get_channel_mask(unsigned int channels)
static HRESULT map_channels(ACImpl *This, const WAVEFORMATEX *fmt) { - if(fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE || fmt->nChannels > 2){ + if(This->dataflow != eCapture && (fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE || fmt->nChannels > 2) ){ WAVEFORMATEXTENSIBLE *fmtex = (void*)fmt; DWORD mask, flag = SPEAKER_FRONT_LEFT; UINT i = 0; -- 2.0.4
participants (2)
-
Andrew Eikum -
Mark Harmstone