[Bug 37890] New: MCI 24bit .wav playback on top of pulseaudio (both with winealsa.drv,winepulse.drv) broken

https://bugs.winehq.org/show_bug.cgi?id=37890

Bug ID: 37890
           Summary: MCI 24bit .wav playback on top of pulseaudio (both
                    with winealsa.drv,winepulse.drv) broken
           Product: Wine
           Version: unspecified
          Hardware: x86
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: winmm&mci
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

Created attachment 50462
  --> https://bugs.winehq.org/attachment.cgi?id=50462
output of: pacmd list-sinks

MCI 24bit .wav playback on top of pulseaudio (both with
winealsa.drv,winepulse.drv) appears to be broken. A minimal test case with the
necessary C code (<30 lines), the resulting .exe (compiled with MinGW) and a
test .wav file is attached.

The resulting test case:
* works on all Windows 7 64bit machines tested
* breaks with winealsa.drv on top of pulseaudio
* breaks with winepulse.drv on top of pulseaudio
* works with winealsa.drv on top of alsa (without pulseaudio)
* works if the file is converted to a 16bit .wav file

Furthermore, the attached output of pacmd list-sinks appears to show that
pulseaudio doesn't advertise 24bit capabilities - so wine probably mistakenly
tries to use 24bit audio with pulseaudio without support for that being
present.

[Bug 37890] MCI 24bit .wav playback on top of pulseaudio (both with winealsa.drv,winepulse.drv) broken

https://bugs.winehq.org/show_bug.cgi?id=37890

--- Comment #1 from [e-mail] ---
Created attachment 50463
  --> https://bugs.winehq.org/attachment.cgi?id=50463
minimal test case to reproduce issue

[Bug 37890] MCI 24bit .wav playback on top of pulseaudio (both with winealsa.drv,winepulse.drv) broken

https://bugs.winehq.org/show_bug.cgi?id=37890

Sebastian Lackner  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 37890] MCI 24bit .wav playback on top of pulseaudio (both with winealsa.drv,winepulse.drv) broken

https://bugs.winehq.org/show_bug.cgi?id=37890

--- Comment #2 from Sebastian Lackner  ---
Created attachment 50464
  --> https://bugs.winehq.org/attachment.cgi?id=50464
Log with WINEDEBUG=+winmm,+alsa

WARNING: Do not wear headphones while running this test app! ;)

I can confirm this issue with vanilla wine 1.7.34. In other wine apps (and also
in winecfg) sound is working fine, so its not a general issue with my sound
configuration. I have attached a log with WINEDEBUG=+winmm,+alsa.

$ pulseaudio --version
pulseaudio 5.0

$ git describe
wine-1.7.34

[Bug 37890] MCI 24bit .wav playback on top of pulseaudio (both with winealsa.drv,winepulse.drv) broken

https://bugs.winehq.org/show_bug.cgi?id=37890

--- Comment #3 from debuser  ---
Created attachment 50468
  --> https://bugs.winehq.org/attachment.cgi?id=50468
Check wBitsPerSample in PCM_StreamOpen()

This is a bug in wine: wav-file contains 24-bit audio. Pulse-alsa plugin only
advertises S16 and S32, no S24 format (see `aplay -v --dump-hw-params -d1
-Ddefault /dev/zero`). When attempting to set it snd_pcm_hw_params_set_format()
fails to set S24_3LE, failing winealsa:AudioClient_Initialize. WINMM_OpenDevice
catches the fail and calls WINMM_MapDevice to try other rates/formats. 16-bit
one succeeds, calling acmStreamOpen, which calls PCM_StreamOpen, which sets
cvtKeepRate to one of PCM_ConvertKeepRate[] values. This where things break.

PCM_StreamOpen assumes that wBitsPerSample is either 8 or 16, so it sets
conversion to 8->16 bits instead of 24->16 bits that it does not support. When
sound plays acmStreamConvert->PCM_StreamConvert->cvtKeepRate->cvtSS816K gets
called and turns 24-bit sound into noise.

Attached is a tiny patch for PCM_StreamOpen to fail if format is not 8 or
16-bit. It's not a workaround but it stops wine from producing noise.

The bug never happens with alsa because alsa provides a
default=plug->softvol->dmix->hw chain, and its "plug" autoconversion module
supports any possible input format, so snd_pcm_hw_params_set_format() never
fails and wine's conversion code never gets called.

To reproduce the bug and hear the noise without pulseaudio, using just alsa,
you need a "broken" ~/.asoundrc bypassing plug autoconversion and limited
supported formats, for example:
pcm.!default {
  type hw
  card 0
  format S16_LE
  rate 44100
  channels 2
}

The workaround for pulseaudio is to fix its alsa-plugin config (usually
/usr/share/alsa/alsa.conf.d/pulse.conf or /usr/share/alsa/pulse-alsa.conf or
similar) adding "plug" autoconversion there replacing:
pcm.!default {
  type pulse
}
with
pcm.!default {
  type plug
  slave.pcm {
    type pulse
  }
}

[Bug 37890] MCI 24bit .wav playback on top of pulseaudio (both with winealsa.drv,winepulse.drv) broken

https://bugs.winehq.org/show_bug.cgi?id=37890

Ken Sharp  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|unspecified                 |1.7.34

[Bug 37890] MCI 24bit .wav playback on top of pulseaudio (both with winealsa.drv,winepulse.drv) broken

https://bugs.winehq.org/show_bug.cgi?id=37890

Andrew Eikum  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #4 from Andrew Eikum  ---
Thanks for looking so far into this. Sounds like the correct fix would be to
support 24-bit formats in msacm32. We have a separate bug, where msacm32 fails
to error on formats it doesn't support.

[Bug 37890] MCI 24bit .wav playback on top of pulseaudio (both with winealsa.drv,winepulse.drv) broken

https://bugs.winehq.org/show_bug.cgi?id=37890

--- Comment #5 from Andrew Eikum  ---
Just curious, is there an actual game or application that uses this 24-bit
format? I don't know if I've ever encountered one in the wild, so it would be
nice to add it to my lits of audio test programs.

[Bug 37890] MCI 24bit .wav playback on top of pulseaudio ( both with winealsa.drv,winepulse.drv) broken

https://bugs.winehq.org/show_bug.cgi?id=37890

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #6 from [e-mail] ---
Still valid wine-staging 1.9.23. Cant test this bug against pure wine, since my
build is for some reason missing development files.

But did recent changes improve or fix this bug?

http://source.winehq.org/git/wine.git/commit/6cc673477dd4a0749a292a6d6f60474...

[Bug 37890] MCI 24bit .wav playback on top of pulseaudio ( both with winealsa.drv,winepulse.drv) broken

https://bugs.winehq.org/show_bug.cgi?id=37890

--- Comment #7 from Andrew Eikum  ---
(In reply to winetest from comment #6)
> Still valid wine-staging 1.9.23. Cant test this bug against pure wine, since
> my build is for some reason missing development files.
>
> But did recent changes improve or fix this bug?
>
> http://source.winehq.org/git/wine.git/commit/
> 6cc673477dd4a0749a292a6d6f604749d548fb6b

It's likely fixed, yes. Jonas, could you re-run your test case in wine-git (or
Wine 1.9.25, when it is released) to confirm that it's fixed?

[Bug 37890] MCI 24bit .wav playback on top of pulseaudio ( both with winealsa.drv,winepulse.drv) broken

https://bugs.winehq.org/show_bug.cgi?id=37890

Bruno Jesus  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |download, testcase

--- Comment #8 from Bruno Jesus  ---
I just tested but as expected on ALSA it works with or without the patch. Needs
real pulseaudio user.

[Bug 37890] MCI 24bit .wav playback on top of pulseaudio ( both with winealsa.drv,winepulse.drv) broken

https://bugs.winehq.org/show_bug.cgi?id=37890

--- Comment #9 from [e-mail] ---
wine 2.7-git winecfg says that I got winealsa.drv.
wine 2.7 winecfg says I got pulseaudio.drv

Works for me. Tried with fresh wineprefix each.