[PATCH 6/7] winepulse: Allocate ACImpl volumes when channel count is known.

[Jacek Caban]

Signed-off-by: Jacek Caban 
---
  dlls/winepulse.drv/mmdevdrv.c | 43 ++++++++++++++++++++++-------------
  1 file changed, 27 insertions(+), 16 deletions(-)

[Andrew Eikum]

On Fri, May 21, 2021 at 02:36:38PM +0200, Jacek Caban wrote:
> diff --git a/dlls/winepulse.drv/mmdevdrv.c b/dlls/winepulse.drv/mmdevdrv.c
> index df38bb594c73..2d30cf866481 100644
> --- a/dlls/winepulse.drv/mmdevdrv.c
> +++ b/dlls/winepulse.drv/mmdevdrv.c
> @@ -585,15 +581,30 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface,
>      hr = pulse->create_stream(name, This->dataflow, mode, flags, duration, period, fmt,
>                                &This->channel_count, &This->pulse_stream);
>      free(name);
> - if (SUCCEEDED(hr)) {
> -
>    hr = get_audio_session(sessionguid, This->parent, This->channel_count, &This->session);
> -
>    if (SUCCEEDED(hr)) {
> -
>        set_stream_volumes(This);
> -
>        list_add_tail(&This->session->clients, &This->entry);
> -
>    } else {
> -
>        pulse->release_stream(This->pulse_stream, NULL);
> -
>        This->pulse_stream = NULL;
> -
>    }
> - if (FAILED(hr)) goto out;
> -
> - if (!(This->vol = malloc(This->channel_count * sizeof(*This->vol))))
> - {
> -
>    hr = E_OUTOFMEMORY;
> -
>    goto out;
> - }
> - for (i = 0; i < This->channel_count; i++)
> -
>    This->vol[i] = 1.f;
> -
> - hr = get_audio_session(sessionguid, This->parent, This->channel_count, &This->session);
> - if (FAILED(hr)) goto out;
> -
> - set_stream_volumes(This);
> - list_add_tail(&This->session->clients, &This->entry);
> -
>
> +out:
> - if (FAILED(hr))
> - {
> -
>    pulse->release_stream(This->pulse_stream, NULL);

This one causes hangs because we already are holding the lock here,
but release_stream() also takes the lock. You can see the hang in the
winecfg Test Sound button, or the winmm wave test.

Andrew

[Jacek Caban]

On 5/24/21 4:17 PM, Andrew Eikum wrote:
> On Fri, May 21, 2021 at 02:36:38PM +0200, Jacek Caban wrote:
> > diff --git a/dlls/winepulse.drv/mmdevdrv.c b/dlls/winepulse.drv/mmdevdrv.c
> > index df38bb594c73..2d30cf866481 100644
> > --- a/dlls/winepulse.drv/mmdevdrv.c
> > +++ b/dlls/winepulse.drv/mmdevdrv.c
> > @@ -585,15 +581,30 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface,
> >       hr = pulse->create_stream(name, This->dataflow, mode, flags, duration, period, fmt,
> >                                 &This->channel_count, &This->pulse_stream);
> >       free(name);
> > - if (SUCCEEDED(hr)) {
> > -
> >    hr = get_audio_session(sessionguid, This->parent, This->channel_count, &This->session);
> > -
> >    if (SUCCEEDED(hr)) {
> > -
> >        set_stream_volumes(This);
> > -
> >        list_add_tail(&This->session->clients, &This->entry);
> > -
> >    } else {
> > -
> >        pulse->release_stream(This->pulse_stream, NULL);
> > -
> >        This->pulse_stream = NULL;
> > -
> >    }
> > - if (FAILED(hr)) goto out;
> > -
> > - if (!(This->vol = malloc(This->channel_count * sizeof(*This->vol))))
> > - {
> > -
> >    hr = E_OUTOFMEMORY;
> > -
> >    goto out;
> > - }
> > - for (i = 0; i < This->channel_count; i++)
> > -
> >    This->vol[i] = 1.f;
> > -
> > - hr = get_audio_session(sessionguid, This->parent, This->channel_count, &This->session);
> > - if (FAILED(hr)) goto out;
> > -
> > - set_stream_volumes(This);
> > - list_add_tail(&This->session->clients, &This->entry);
> > -
> >
> > +out:
> > - if (FAILED(hr))
> > - {
> > -
> >    pulse->release_stream(This->pulse_stream, NULL);
>
> This one causes hangs because we already are holding the lock here,
> but release_stream() also takes the lock. You can see the hang in the
> winecfg Test Sound button, or the winmm wave test.

Sorry about that, I will send a fixed version. BTW, ultimate plan for 
this is to use a separated critical section for PE-side locking and make 
pulse_lock() and pulse_unlock() private to unix lib.

Thanks,

Jacek