[Bug 28517] New: Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

Bug #: 28517
           Summary: Pikachu Volleyball stops playing sound effects
           Product: Wine
           Version: 1.3.29
          Platform: x86-64
               URL: http://andrew.jaww.net/blog/article/54/pikachu-volleyb
                    all
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: directx-dsound
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
                CC: [e-mail]
    Classification: Unclassified

Created attachment 36598
  --> http://bugs.winehq.org/attachment.cgi?id=36598
Gzipped +tid,+mmdevapi,+winmm,+dsound,+alsa log (3.2MB)

The sound effects in Pikachu Volleyball stop playing after a short while,
though the background music continues (it's MIDI). Sometimes distorted noises
can be heard before the sound effects stop entirely. Regression testing
indicated:

e786998daff4ad49521a4c9c39c172ddcdcad82a is the first bad commit
commit e786998daff4ad49521a4c9c39c172ddcdcad82a
Author: Andrew Eikum 
Date:   Tue Sep 27 08:51:07 2011 -0500

dsound: Reimplement rendering devices on mmdevapi.

:040000 040000 53407fdfd9131c99d768c90eeda5dadc66a63abb
af37caec04c5e8abedfd395abcd88a16e71a3b85 M    dlls

Reverting the patch fixed the problem. I use ALSA (no Pulseaudio).

I first found (seemingly) the same problem in a game called Alonix, but it's
easier to reproduce in this one:

1. Start a single player game (top option in main menu)
2. Do nothing

Sound effects are played on the first turn, but not on any subsequent turn. (I
haven't heard any distortion this way.)

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

A Wine user  changed:

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

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

A Wine user  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
    Regression SHA1|                            |e786998daff4ad49521a4c9c39c
                   |                            |172ddcdcad82a

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #1 from Andrew Eikum  2011-09-28 14:33:19 CDT ---
This seems to be driver-specific. I can reproduce this with ALSA, but not with
OSS or PulseAudio via ALSA.

It looks like what's happening is the application has periods where it plays no
sound effects, so dsound runs out of data to send. That's fine and by design,
but the ALSA driver doesn't handle the underrun situation very well. Once an
underrun occurs, everything falls apart and ALSA refuses to report underruns or
recover in any consistent way.

I'll keep looking at it.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

Raymond  changed:

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

--- Comment #2 from Raymond  2011-09-30 02:54:34 CDT ---
(In reply to comment #1)
> This seems to be driver-specific. I can reproduce this with ALSA, but not with
> OSS or PulseAudio via ALSA.
>
> It looks like what's happening is the application has periods where it plays no
> sound effects, so dsound runs out of data to send. That's fine and by design,
> but the ALSA driver doesn't handle the underrun situation very well. Once an
> underrun occurs, everything falls apart and ALSA refuses to report underruns or
> recover in any consistent way.
>
> I'll keep looking at it.

what is the meaning of "This->bufsize_frames" ?

if(!duration)
        duration = 300000; /* 0.03s */
    This->bufsize_frames = ceil((duration / 10000000.) * fmt->nSamplesPerSec);
    This->local_buffer = HeapAlloc(GetProcessHeap(), 0,
            This->bufsize_frames * fmt->nBlockAlign);

can you add snd_pcm_dump() between snd_pcm_sw_params() and snd_pcm_prepare()

snd_pcm_uframes_t boundary;
+   snd_output_t *output = NULL;
    const WAVEFORMATEXTENSIBLE *fmtex = (const WAVEFORMATEXTENSIBLE *)fmt;

if((err = snd_pcm_sw_params(This->pcm_handle, sw_params)) < 0){
        WARN("Unable to set sw params: %d (%s)\n", err, snd_strerror(err));
        hr = E_FAIL;
        goto exit;
    }

+   snd_output_stdio_attach(&output, stdout, 0);
+   if (err == 0) {
+       snd_pcm_dump(This->pcm_handle, output);
+        snd_output_close(output);
+   };

if((err = snd_pcm_prepare(This->pcm_handle)) < 0){

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #3 from Andrew Eikum  2011-09-30 08:12:01 CDT ---
(In reply to comment #2)
> what is the meaning of "This->bufsize_frames" ?
> if(!duration)
>     duration = 300000; /* 0.03s */
> This->bufsize_frames = ceil((duration / 10000000.) * fmt->nSamplesPerSec);
> This->local_buffer = HeapAlloc(GetProcessHeap(), 0,
>         This->bufsize_frames * fmt->nBlockAlign);

The size of the local buffer in frames. The local buffer is needed since ALSA
can't guarantee buffer sizes as large as MMDevAPI needs, so we need a place to
store the extra data before it gets placed in ALSA's smaller buffer.

Why do you ask?

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

Jörg Höhle  changed:

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

--- Comment #4 from Jörg Höhle  2011-09-30 16:22:01 CDT ---
> I can reproduce this with ALSA, but not with OSS or PulseAudio via ALSA.

Intriguing. So far it's only pulseaudio (those versions affected by
handle_underrun=1) that somehow stopped in case of underrun...

Or is that simply a case of "ALSA underrun => play silence until app catches
up" which I mentioned a couple of times?
http://www.winehq.org/pipermail/wine-devel/2011-August/091333.html
One of my notes says to add snd_pcm_reset or snd_pcm_forward when an
avail>buffer prior to snd_pcm_write, but somehow I forgot about that one.
... I haven't yet written that patch because I've seen snd_pcm_forward cause
100% CPU useage on 2 year old ALSA systems, and snd_pcm_reset doesn't allow the
same amount of control w.r.t. skipped frames so as to keep GetPosition easily
correct.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #5 from Raymond  2011-09-30 23:11:34 CDT ---
(In reply to comment #3)
> The size of the local buffer in frames. The local buffer is needed since ALSA
> can't guarantee buffer sizes as large as MMDevAPI needs, so we need a place to
> store the extra data before it gets placed in ALSA's smaller buffer.
>
> Why do you ask?

Have you checked the output of snd_pcm_dump ?

the buffer size of pulse plugin is larget than wine's buffer (1 seconds)

if only set_period_time_near() to 10ms , alsa-lib will select maximum buffer
size

ALSA <-> PulseAudio PCM I/O Plugin
Its setup is:
  stream       : PLAYBACK
  access       : RW_INTERLEAVED
  format       : S16_LE
  subformat    : STD
  channels     : 1
  rate         : 44100
  exact rate   : 44100 (44100/1)
  msbits       : 16
  buffer_size  : 451584
  period_size  : 441
  period_time  : 10000
  tstamp_mode  : NONE
  period_step  : 1
  avail_min    : 441
  period_event : 0
  start_threshold  : 1849688064
  stop_threshold   : 1849688064
  silence_threshold: 0
  silence_size : 1849688064
  boundary     : 1849688064

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #6 from A Wine user  2011-10-01 03:23:00 CDT ---
Created attachment 36621
  --> http://bugs.winehq.org/attachment.cgi?id=36621
Output of snd_pcm_dump

(In reply to comment #2)
> can you add snd_pcm_dump() between snd_pcm_sw_params() and snd_pcm_prepare()

Sorry for the delay, but here's the output I got from snd_pcm_dump while
running this game.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #7 from el  2011-10-02 11:57:37 CDT ---
Created attachment 36662
  --> http://bugs.winehq.org/attachment.cgi?id=36662
Output of snd_pcm_dump

Confirming this problem with the game Diablo II.
Vanishing sound, same commit id for the regression.
My output of snd_pcm_dump is similar to yours.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #8 from Jörg Höhle  2011-10-04 10:00:15 CDT ---
IMHO it was an error to stop writing silence in DSound *before* fixing ALSA.  I
don't know whether Andrew is working on this right now, but I might have a
patch by tomorrow.  This is a major bug to fix before 1.3.30 otherwise
regressions are going to rain.

Admins, please set 1.4 milestone and major importance.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #9 from Andrew Eikum  2011-10-04 12:44:12 CDT ---
Created attachment 36707
  --> http://bugs.winehq.org/attachment.cgi?id=36707
winealsa.drv: Detach ALSA's period timer from MMDevAPI's

I have been working on improving the ALSA driver. Here's the current patch. I'd
appreciate review on it. I think it's near final, and I'm planning to get it
into Wine before the next release.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |1.4.0
     Ever Confirmed|0                           |1
           Severity|normal                      |major

--- Comment #10 from Austin English  2011-10-04 13:20:16 CDT ---
(In reply to comment #8)
> IMHO it was an error to stop writing silence in DSound *before* fixing ALSA.  I
> don't know whether Andrew is working on this right now, but I might have a
> patch by tomorrow.  This is a major bug to fix before 1.3.30 otherwise
> regressions are going to rain.
>
> Admins, please set 1.4 milestone and major importance.

Done.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

GyB  changed:

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

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #11 from Andrew Eikum  2011-10-04 14:43:53 CDT ---
Created attachment 36712
  --> http://bugs.winehq.org/attachment.cgi?id=36712
dsound: Fix fragment position calculations

Okay, I think I've got this fixed. With both the patch from Comment 9 and this
patch, I have good audio with this application. I'll try to submit them
tomorrow, most likely.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #12 from el  2011-10-04 20:44:58 CDT ---
Did a quick test in Diablo II with both patches applied.
Could no longer reproduce the disappearing sound problem.
Thanks.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #13 from Raymond  2011-10-04 22:11:51 CDT ---
(In reply to comment #9)
> Created attachment 36707 [details]
> winealsa.drv: Detach ALSA's period timer from MMDevAPI's
>
> I have been working on improving the ALSA driver. Here's the current patch. I'd
> appreciate review on it. I think it's near final, and I'm planning to get it
> into Wine before the next release.

I have doubt about your patch remove snd_pcm_hw_params_set_period_time_near()

this means that you will let alsa-lib to determine the period time, buffer_size

so user have to check /usr/share/alsa/alsa.conf if they are using "hw" as
"default"

defaults.pcm.minperiodtime 5000        # in us

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #14 from Jörg Höhle  2011-10-05 03:04:22 CDT ---
(In reply to comment #9)
> I'd appreciate review on it.

Maybe it's an artefact of bugzilla's new two column diff mode, but where do you
initialize mmdev_period_rt? I don't see it in
http://bugs.winehq.org/attachment.cgi?id=36707&action=diff

Why do you reintroduce bufsize_frames etc. as UINT64?

I think it's good that the patch delegates write() entirely to the feeder
thread.  I once said that having it too inside ReleaseBuffer should be nothing
more than a performance optimization.  Yet this optimization may be critical to
avoid underruns when event signaling is detached (not sync'ed) from ALSA's
actual need, which always happens with 2 timers from independent clocks, but it
was too early for me to analyse that.

I see no more sw_params_set_*.  while it's ok to go with defaults as far as
possible, there are a few sw_params that you want to set.  For instance, ALSA
must either enter XRUN mode and stop or play silence upon underrun.  Not
setting any sw_params does not guarantee that, in particular the circular HW
buffers may loop ad infinitum.

Why do you care at all about ALSA's period?  It can be as low as 1ms (e.g. with
a 8000Hz sample I got 1ms period and 8.192ms buffer).  I don't want to bear
1000 interrupts a second.  Wine should ignore ALSA's period for as long as we
don't use poll(alsa's_fd) and the buffer is large enough for our periodic
feeder. See
http://mailman.alsa-project.org/pipermail/alsa-devel/2011-August/042837.html

Alternatively and if it matches, Wine should equate its published period with
ALSA's, e.g. try to use 10ms like mmdevapi's mixer does.  The best in that case
would be to sync' ALSA signaling readiness with mmdevapi's event-triggered
feeder.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #15 from Jörg Höhle  2011-10-05 03:13:09 CDT ---
Created attachment 36724
  --> http://bugs.winehq.org/attachment.cgi?id=36724
Underrun recovery requires a fast forward.

My patch likely conflicts with Andrew Eikum's but addresses something
different, yet still related to underruns.  In "streaming mode" (which dmix and
pulse use), when an underrun happens, for instance because the apps sends no
data, which it is perfectly allowed to do, ALSA must be told to skip over the N
seconds of underruns that it keeps counting while playing nothing.

I'd be interested to hear how that patch helps.  Such a mechanism is needed
regardless of Andrew's other changes.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #16 from A Wine user  2011-10-05 04:45:03 CDT ---
(In reply to comment #15)
> I'd be interested to hear how that patch helps.  Such a mechanism is needed
> regardless of Andrew's other changes.

On its own, this patch prevented sound effects from vanishing entirely, but
they quickly became distorted - too high-pitched, echoed, cut off, etc. I tried
combining it with Andrew Eikum's dsound patch, and this combination works quite
a bit better, but rarely, a lone sound effect still gets distorted.

I also tried using both of Andrew Eikum's patches, and everything seems to work
correctly thus far, but I'll try to test them a bit more later just in case the
random distortion is still there and I happened to get lucky this time.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #17 from A Wine user  2011-10-05 07:04:17 CDT ---
(In reply to comment #16)
> I also tried using both of Andrew Eikum's patches, and everything seems to work
> correctly thus far, but I'll try to test them a bit more later just in case the
> random distortion is still there and I happened to get lucky this time.

Well, I've spent another 25 minutes testing these patches and haven't heard any
distortion at all.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #18 from Andrew Eikum  2011-10-05 09:34:32 CDT ---
Created attachment 36726
  --> http://bugs.winehq.org/attachment.cgi?id=36726
winealsa.drv: Don't try to control ALSA's behavior

(In reply to comment #13)
> I have doubt about your patch remove snd_pcm_hw_params_set_period_time_near()
>
> this means that you will let alsa-lib to determine the period time, buffer_size
>
> so user have to check /usr/share/alsa/alsa.conf if they are using "hw" as
> "default"
>
> defaults.pcm.minperiodtime 5000        # in us

Can you elaborate on this? I don't see the problem with letting ALSA determine
its period size.

(In reply to comment #14)
> Maybe it's an artefact of bugzilla's new two column diff mode, but where do you
> initialize mmdev_period_rt? I don't see it in
> http://bugs.winehq.org/attachment.cgi?id=36707&action=diff
>
> Why do you reintroduce bufsize_frames etc. as UINT64?

These were me being clumsy. Fixed.
> I see no more sw_params_set_*.  while it's ok to go with defaults as far as
> possible, there are a few sw_params that you want to set.  For instance, ALSA
> must either enter XRUN mode and stop or play silence upon underrun.  Not
> setting any sw_params does not guarantee that, in particular the circular HW
> buffers may loop ad infinitum.

Okay, I've explicitly set start_threshold=1 and
stop_threshold=alsa_bufsize_frames. These match the defaults on my system,
which do what I expect. Did you have other things in mind?
> Why do you care at all about ALSA's period?  It can be as low as 1ms (e.g. with
> a 8000Hz sample I got 1ms period and 8.192ms buffer).  I don't want to bear
> 1000 interrupts a second.  Wine should ignore ALSA's period for as long as we
> don't use poll(alsa's_fd) and the buffer is large enough for our periodic
> feeder. See
> http://mailman.alsa-project.org/pipermail/alsa-devel/2011-August/042837.html

Yeah, good point. This also gets rid of the "two timers" business that I didn't
like.

I've attached a new patch with your improvements.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #19 from Andrew Eikum  2011-10-05 10:01:07 CDT ---
(In reply to comment #15)
> Created attachment 36724 [details]
> Underrun recovery requires a fast forward.
>
> My patch likely conflicts with Andrew Eikum's but addresses something
> different, yet still related to underruns.  In "streaming mode" (which dmix and
> pulse use), when an underrun happens, for instance because the apps sends no
> data, which it is perfectly allowed to do, ALSA must be told to skip over the N
> seconds of underruns that it keeps counting while playing nothing.
>
> I'd be interested to hear how that patch helps.  Such a mechanism is needed
> regardless of Andrew's other changes.

Would you agree that this is addressed by the patch I just attached in Comment
18? Take a look at
http://bugs.winehq.org/attachment.cgi?id=36726&action=diff#a/dlls/winealsa.drv/mmdevdrv.c_sec10.
When an underrun occurs, the PCM handle is recovered, reset, and prepared,
which is about the same as what your patch in Comment 15 does.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #20 from Jörg Höhle  2011-10-05 10:40:53 CDT ---
There are two kinds of underruns, yours addresses XRUN, mine handles the case
of devices (like dmix IIRC) which never enter XRUN state.  Both are needed.
My general idea is that the code in Wine ought to work with both kinds of
devices and I believe that's not hard to achieve.
Perhaps the best is to ask for XRUN (via stop_threshold) and set silence mode,
then cope with what I now call "soft underruns" from streaming devices without
XRUN auto-stop, where avail_update grows > alsa_buffer_size.

I believe your patch fixes several things at once, e.g. _Stop does not call
snd_pcm_drop anymore, which caused havoc with GetPosition and waveOutRestart.
That's good.  Reset is indeed a good place to stop the device that won't
conflict with GetPosition.  Likewise, I found out that snd_pcm_reset does not
only set a few vars to 0, it may also change state.

Now that you removed write() from within ReleaseBuffer, it looks like prefill
after Start+Stop becomes trivial to handle correctly.
> start_threshold=1

Hmm, auto-start should perhaps be disabled because the mmdevapi is designed to
have explicit control via _Start and _Stop.  OTOH why start without data?  In
that area I've not yet conducted enough tests to feel comfortable, esp. about
the former if(avail < alsa_period_size) condition.
Indeed it looks like some devices really start only when fed more than
alsa_period_size frames (and same with OSS apparently), and that caused short
winmm sounds to hang.

BTW, regarding my 1ms/8.192ms example, it shows that in some cases, ALSA's
defaults are no match for the typical 10ms mmdevapi period and underruns hence
guaranteed.  But I've not yet found a receipe for what to do then (e.g.
automatically reduce mmdevapi's period to ALSA's buffer_time if smaller?)

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #21 from Andrew Eikum  2011-10-05 11:28:58 CDT ---
(In reply to comment #20)
> There are two kinds of underruns, yours addresses XRUN, mine handles the case
> of devices (like dmix IIRC) which never enter XRUN state.  Both are needed.
> My general idea is that the code in Wine ought to work with both kinds of
> devices and I believe that's not hard to achieve.
> Perhaps the best is to ask for XRUN (via stop_threshold) and set silence mode,
> then cope with what I now call "soft underruns" from streaming devices without
> XRUN auto-stop, where avail_update grows > alsa_buffer_size.

Doesn't the email you linked above say that this should never happen? I thought
sw_params were supposed to work regardless of the hardware underneath.
> I believe your patch fixes several things at once, e.g. _Stop does not call
> snd_pcm_drop anymore, which caused havoc with GetPosition and waveOutRestart.
> That's good.  Reset is indeed a good place to stop the device that won't
> conflict with GetPosition.  Likewise, I found out that snd_pcm_reset does not
> only set a few vars to 0, it may also change state.
>
> Now that you removed write() from within ReleaseBuffer, it looks like prefill
> after Start+Stop becomes trivial to handle correctly.

Perhaps I should add a call to alsa_write_data() in Start().
> > start_threshold=1
>
> Hmm, auto-start should perhaps be disabled because the mmdevapi is designed to
> have explicit control via _Start and _Stop.

Well, we absolutely never give ALSA data unless the MMDevAPI device is
Start()ed. Then in Stop(), we kill the feed timer, so ALSA will run out its
current buffer and stop playing. So this seems to be satisfied.
>                                         OTOH why start without data?  In
>
> that area I've not yet conducted enough tests to feel comfortable, esp. about
> the former if(avail < alsa_period_size) condition.
> Indeed it looks like some devices really start only when fed more than
> alsa_period_size frames (and same with OSS apparently), and that caused short
> winmm sounds to hang.

Hmm, that's a real concern. I feel that with start_threshold=1, if ALSA does
not start the device, then it's an ALSA bug. Again, sw_params should always
work. I also can't think of a sane way to work around it. Have you seen this
happen?
> BTW, regarding my 1ms/8.192ms example, it shows that in some cases, ALSA's
> defaults are no match for the typical 10ms mmdevapi period and underruns hence
> guaranteed.  But I've not yet found a receipe for what to do then (e.g.
> automatically reduce mmdevapi's period to ALSA's buffer_time if smaller?)

Another ugly problem...

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #22 from Raymond  2011-10-06 01:30:41 CDT ---
(In reply to comment #18)
> Created attachment 36726 [details]
> winealsa.drv: Don't try to control ALSA's behavior
>
> (In reply to comment #13)
> > I have doubt about your patch remove snd_pcm_hw_params_set_period_time_near()
> >
> > this means that you will let alsa-lib to determine the period time, buffer_size
> >
> > so user have to check /usr/share/alsa/alsa.conf if they are using "hw" as
> > "default"
> >
> > defaults.pcm.minperiodtime 5000        # in us
>
> Can you elaborate on this? I don't see the problem with letting ALSA determine
> its period size.

This is because your "default" are using "dmix" but there are sound cards which
does not use dmix (e.g. emu10k1, ymfpci, au88x0, cs46xx )

maximum buffer size of pulse plugin , hda-intel and those pcie sound cards are
4Mb Bytes (i.e about 21 seconds @ 44100Hz , but those pci sound cards have less
than 0.5 seconds buffer (e.g. 0.37 seconds @ 44100Hz for those sound card with
64bytes )

http://git.alsa-project.org/?p=alsa-lib.git;a=commit;h=de606e9c256f5a776c162...

http://git.alsa-project.org/?p=alsa-lib.git;a=commit;h=cd7070bf4b7afcdcd9dbd...

BTW gatAudiofromat() seem wrong since dmix the use S32_LE for hda-intel o

(In reply to comment #14)
> Why do you care at all about ALSA's period?  It can be as low as 1ms (e.g. with
> a 8000Hz sample I got 1ms period and 8.192ms buffer).

I have a ymf724f whcih can only update hwptr every 5.333ms intervals

(i.e. snd_pcm_available only change at the boundary of minimum period
size/time)

does it meet the requirement of WAVECAPS_SAMPLEACCURATE ?

http://git.alsa-project.org/?p=alsa-kernel.git;a=commit;h=bd1c9a3abd71554deb...

ALSA: usb: refine delay information with USB frame counter
> I don't want to bear
> 1000 interrupts a second.  Wine should ignore ALSA's period for as long as we
> don't use poll(alsa's_fd) and the buffer is large enough for our periodic
> feeder. See
> http://mailman.alsa-project.org/pipermail/alsa-devel/2011-August/042837.html

"the possibility to disable interrupts (period_wakeup) was added to a few
drivers; PulseAudio uses this."

Only two or three drivers support this feature (e.g. hda-intel, oxygen)

http://git.alsa-project.org/?p=alsa-kernel.git&a=search&h=c035877c26...
> Alternatively and if it matches, Wine should equate its published period with
> ALSA's, e.g. try to use 10ms like mmdevapi's mixer does.  The best in that case
> would be to sync' ALSA signaling readiness with mmdevapi's event-triggered
> feeder.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #23 from Jörg Höhle  2011-10-06 02:38:30 CDT ---
> ["soft underruns"] this should never happen?

That's what he wrote.  My experience is different, and there are posts too
about users expressing their surprise with dmix not stopping.
> [#samples < period never start] Have you seen this happen?

Bug #27087, comment #12.  That was before 1.3.25.
> [ALSA buffer < mmdevapi period]Another ugly problem...
> e.g. 0.37 seconds @ 44100Hz

My memory was wrong about the 8000Hz case.  The period is truly 8(!) frames,
1ms, however the buffer is large enough with 8192 frames, >1s.

I don't know the maximum allowed mmdevapi period (500ms?)
In our case, I'd say a FIXME is enough for now:
if (10 * alsa_buffer_time <= 12 * mmdevapi_period)
  FIXME("Tiny ALSA buffer %u. Expect underruns\n");
The 12/10=20% margin is because ALSA's "buffer empty" is not sync'ed with
mmdevapi's feeder event.
> Perhaps I should add a call to alsa_write_data() in Start().

Maybe at a later time (there's potential for underrun depending on the amount
and how far away the next event tick is).  For now, let's behave like what I
believe the native mixer does: every 10ms it mixes all available data, nothing
more. Perhaps, in shared mode, all streams start on the 10ms boundary!
(I've been thinking about an audible test with two streams with 180° phase
delta not written at the same time, yet they should all enter the same 10ms
period and might annihilate themselves...)

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

Andrew Eikum  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #36707|0                           |1
        is obsolete|                            |
  Attachment #36726|0                           |1
        is obsolete|                            |

--- Comment #24 from Andrew Eikum  2011-10-06 09:29:52 CDT ---
Created attachment 36738
  --> http://bugs.winehq.org/attachment.cgi?id=36738
winealsa.drv: Don't try to control ALSA's behavior

Okay, one more try. New changes here include changing the default period time
to 10ms, adding your warning about the ALSA buffer size, and adding an
additional check for underruns in alsa_write_data() using the return value from
avail_update().

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #25 from Jörg Höhle  2011-10-06 16:43:12 CDT ---
> dmix not stopping

I'll correct: plug:dmix can stop and enter XRUN given an appropriate
stop_threshold.
> [#samples < period never start] Have you seen this happen?

Definitely, in both XRUN and free running modes. plug:dmix starts only once
written the frame #period_size, one by one or en bloc. snd_pcm_start or
set_start_threshold won't change anything.

plughw is different and starts as soon as the first sample is written.
> one more try

Did you try out my render tests from bug #27937? It's not my newest, but still
useful. In particular, you can change the sleep(350) to test underrun behaviour
as well as change "default" to "pulse", "plug:dmix" or "plughw:0" in
mmdevdrv.c. I can't apply your code yet because my ALSA machine is not sync'ed
with git now.
>   TRACE("PCM state: %u\n", snd_pcm_state(This->pcm_handle));

This looks like left over from debugging. To make it useful, please add
avail_frames and possibly padding.
> Initialize: snd_pcm_hw_params_current after snd_pcm_hw_params

is superfluous, This->hw_params is up to date. ALSA does not behave like a OO
factory.
> MinimumPeriod = 100000

I'd make it smaller than DefaultPeriod, e.g. 5ms.
> this call seems to be required to get an accurate snd_pcm_state

I observed the same requirement for snd_pcm_delay

Do you think you need
snd_pcm_state(This->pcm_handle) == SND_PCM_STATE_XRUN
My tests show this test is superseded by avail > alsa_size. I.e. after XRUN,
avail is larger. Not cross-checked with PA.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #26 from Austin English  2011-10-06 17:48:05 CDT ---
(In reply to comment #11)
> Created attachment 36712 [details]
> dsound: Fix fragment position calculations
>
> Okay, I think I've got this fixed. With both the patch from Comment 9 and this
> patch, I have good audio with this application. I'll try to submit them
> tomorrow, most likely.

Committed as:
http://source.winehq.org/git/wine.git/commitdiff/ecd13dcbef16ccee9ae7785cc60...

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #27 from GyB  2011-10-07 09:27:05 CDT ---
In my experience, the audio problem in the Pikachu game as well as in several
other games has been resolved as of wine-1.3.29-245-g895b48e.

On the other hand, there are still problems with audio playback in certain
games, and these problems are originating from
commit e786998daff4ad49521a4c9c39c172ddcdcad82a
dsound: Reimplement rendering devices on mmdevapi.

Two examples:

Plants vs. Zombies: no music (only sound effects and voices are playing)
Tom Clancy's Ghost Recon: no audio at all

Fedora 15
Alsa 1.0.24
Pulseaudio is not running

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #28 from Austin English  2011-10-07 13:29:06 CDT ---
(In reply to comment #27)
> In my experience, the audio problem in the Pikachu game as well as in several
> other games has been resolved as of wine-1.3.29-245-g895b48e.

Fixed.
> On the other hand, there are still problems with audio playback in certain
> games, and these problems are originating from
> commit e786998daff4ad49521a4c9c39c172ddcdcad82a
> dsound: Reimplement rendering devices on mmdevapi.
>
> Two examples:
>
> Plants vs. Zombies: no music (only sound effects and voices are playing)
> Tom Clancy's Ghost Recon: no audio at all

New bug please.

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |ecd13dcbef16ccee9ae7785cc60
                   |                            |ec63f5772ae19

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

--- Comment #29 from Raymond  2011-10-09 21:02:27 CDT ---
(In reply to comment #18)
> > Why do you care at all about ALSA's period?  It can be as low as 1ms (e.g. with
> > a 8000Hz sample I got 1ms period and 8.192ms buffer).  I don't want to bear
> > 1000 interrupts a second.  Wine should ignore ALSA's period for as long as we
> > don't use poll(alsa's_fd) and the buffer is large enough for our periodic
> > feeder. See
> > http://mailman.alsa-project.org/pipermail/alsa-devel/2011-August/042837.html
>
> Yeah, good point. This also gets rid of the "two timers" business that I didn't
> like.

disable period wakeup is mainly for low power consumption, this has impact on
those application require low latency

How can you get exact 10ms period time when the rate is 11025 Hz and 22050 Hz ?

what is the difference between DSBCAPS_TRUEPLAYPOSITION and
DSBCAPS_GETCURRENTPOSITION2 ?

[Bug 28517] Pikachu Volleyball stops playing sound effects

http://bugs.winehq.org/show_bug.cgi?id=28517

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #30 from Alexandre Julliard  2011-10-10 13:13:18 CDT ---
Closing bugs fixed in 1.3.30.