[Bug 28039] New: IAudioClock_GetPosition must ignore underruns (MacOS)

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

Summary: IAudioClock_GetPosition must ignore underruns (MacOS)
           Product: Wine
           Version: 1.3.25
          Platform: x86
        OS/Version: Mac OS X
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: mmdevapi
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

GetPosition is critical because the new "winmm on mmdevapi" layer relies on it
for its buffer management. Currently, neither winealsa nor winecoreaudio
entirely fulfill GetPosition's contract.

GetPosition yields "the stream position of the sample that is currently playing
through the speakers". My tests show that it ignores underruns and is therefore
not identical to a clock.  One can derive at least 2 tests from that:

1. GetPosition <= elapsed time * samples/sec (cannot hear the future).
2. GetPosition <= sum of samples fed to the device.
3. GetPosition == sum of samples once all have been played.
4. Getposition is monotically increasing (except when Reset).

Actually GetPosition does not yield samples, replace the above with GetPosition
/ GetFrequency * samples per sec to be correct.

My tests attached to bug #27937 show that:
- winecoreaudio fails test 3 (then 2).
- winealsa with dmix fails test 1 during the first seconds of play.
- winealsa with pulse fails test 1 much worse.
See bug #27937 comment #1 and 2 for log snippets.

On MacOS, GetPosition appears to continue to grow in the presence of underruns,
violating 2 and 3.  Hence the name of the present issue.

Note that underruns are unknown to WINMM. A perfectly legal use is:
waveOutOpen()
waveOutPrepare(hugeboomheader)
waveoutPrepare(clickheader)
waveoutPrepare(ouchheader)
waveoutPrepare(beepheader)
then write the headers as the conditions arise (arguably dsound's mixing would
be more adequate).  Guess what happens when GetPosition grows playing nothing
between 2 beeps?

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

Jörg Höhle  changed:

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

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |1.4.0

--- Comment #1 from Austin English  2011-10-09 15:21:17 CDT ---
Sound bug => 1.4 milestone.

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

Jörg Höhle  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |major

--- Comment #2 from Jörg Höhle  2011-11-22 04:35:33 CST ---
Ken Thomases explained how to possibly implement it correctly in
http://www.winehq.org/pipermail/wine-devel/2011-October/092743.html

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

--- Comment #3 from Andrew Eikum  2011-11-22 07:36:32 CST ---
Created attachment 37574
  --> http://bugs.winehq.org/attachment.cgi?id=37574
winecoreaudio.drv: Make the driver sample accurate

And here's a patch, which Ken reviewed off-list yesterday :) Going to submit it
today, probably.

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

--- Comment #4 from Austin English  2011-11-22 13:26:44 CST ---
(In reply to comment #3)
> Created attachment 37574 [details]
> winecoreaudio.drv: Make the driver sample accurate
>
> And here's a patch, which Ken reviewed off-list yesterday :) Going to submit it
> today, probably.

http://source.winehq.org/git/wine.git/commitdiff/2da60016affdbc13356aef162e0...

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

--- Comment #5 from Jörg Höhle  2011-11-23 02:21:57 CST ---
Created attachment 37588
  --> http://bugs.winehq.org/attachment.cgi?id=37588
render.ok test output of my extended mmdevapi tests

My mmdevapi position tests now mostly pass, great.
What's striking is that position is generally equal to sum of written frames -
GetCurrentPadding.  This is not unlike native's exclusive mode.  Perhaps you
want to check for rounding errors because it's sometimes off by one as in:
render.c:1143: padding 17617 position 17420/17421 slept 550ms iteration 1
render.c:1143: padding 17622 position 30700/30701 slept 850ms iteration 4
Or is that simply an effect of time advancing between 2 calls?

I've yet to spend more time, but I've noticed that if you increase the big
sleep  prior to my rendering loop from 350 to 450ms, there's a test failure at
iteration 1 (normal after an underrun) but also at iteration 2:
render.c:1151: Test failed: Position delta 4933 not regular
That does not make GetPosition appear like a true speaker position. It should
not advance by more than 4454 frames in 101ms.

You may want to increase the sleep even more to see that it's a bit odd:
render.c:1090: data at 0x421dc000 for prefill 20671
render.c:1119: hpctime 551 after 550ms
render.c:1138: hpctime 651 pcpos 651
render.c:1143: padding 0 position 20671/20671 slept 650ms iteration 0
render.c:1172: data at 0x421dc000
render.c:1138: hpctime 752 pcpos 752
render.c:1143: padding 15086 position 26255/26256 slept 750ms iteration 1
render.c:1151: Test failed: Position delta 5584 not regular
After an underrun, GetPosition increases too fast (perhaps frames are silently
skipped when a buffer's start time lies in the past).

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

--- Comment #6 from Jörg Höhle  2011-11-23 02:45:34 CST ---
Oops, I wrote to bug #21655, comment #5 instead of here. That other bug
mentioned a past "Non-aligned pointer being freed" error. Except this time, I
get no backtrace.

After applying your patch atop 1.3.32 I obtain this shortly after the end of
the second and subsequent test runs, apparently as Wine unloads, but not
always.

wine(5770,0x4080c200) malloc: *** error for object 0x402086c0: Non-aligned
pointer being freed (2)
*** set a breakpoint in malloc_error_break to debug
wine(5770,0x4080c200) malloc: *** error for object 0x402085c0: Non-aligned
pointer being freed (2)
*** set a breakpoint in malloc_error_break to debug

git checkout HEAD^; make; and that is gone.

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

--- Comment #7 from Jörg Höhle  2011-12-17 16:16:17 CST ---
The remaining issue becomes obvious when increasing the sleep even more, e.g.
to 3150ms:
render.c:1090: data at 0x422dc000 for prefill 20671
render.c:1119: hpctime 3151 after 3150ms
render.c:1138: hpctime 3252 pcpos 3252
render.c:1143: padding 0 position 20671/20671 slept 3250ms iteration 0
# normal padding 0 past underrun
render.c:1138: hpctime 3352 pcpos 3352
render.c:1143: padding 0 position 41342/41342 slept 3350ms iteration 1
render.c:1166: Test marked todo: GetBuffer large (20671) at iteration 1

WineCoreAudio 1.3.35 pretends advancing by 20671 frames within 100ms(!), as if
underrun had never occurred during the last 3 seconds.

render.c:1138: hpctime 3453 pcpos 3453
render.c:1143: padding 0 position 62013/62013 slept 3450ms iteration 2
render.c:1166: Test marked todo: GetBuffer large (20671) at iteration 2
render.c:1138: hpctime 3553 pcpos 3553
render.c:1143: padding 0 position 82684/82684 slept 3550ms iteration 3
render.c:1166: Test marked todo: GetBuffer large (20671) at iteration 3
... advancing by 1.5 seconds worth of frames within 300ms.

This behaviour is bogus and reminds of ALSA's dmix at the time Wine used the
"play silence during underruns" mode instead of XRUN that I reported in bug
#28517, comment #4. There too, the app could throw megabytes of data to the
audio engine, normalizing only once as many frames had be written fast as the
underrun gap had caused.
A difference probably is: ALSA plays silence, CoreAudio might buffer all that
stuff and play it late. I can't tell as my render tests are solely playing
silence.

The resulting behaviour is very different from native and may cause havoc with
apps using winmm, because GetPosition governs WHDR_DONE messages. They will be
sent out too fast after an underrun occurred, causing loss of sync and other
effects.

The good news is that GetPosition will not report a position higher than the
number of written frames.

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

Jörg Höhle  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |28093

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|1.4.0                       |1.6.0

--- Comment #8 from Austin English  2012-03-07 13:27:12 CST ---
Moving milestone to 1.6.

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|1.6.0                       |1.4.0

--- Comment #9 from Austin English  2012-03-07 13:34:26 CST ---
My mistake, 1.6 criteria are not set yet, so these need to stay in 1.4.

Sorry for the noise.

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

joaopa  changed:

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

--- Comment #10 from joaopa  2012-10-13 01:57:00 CDT ---
Still  a bug in current wine?

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

Jörg Höhle  changed:

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

--- Comment #11 from Jörg Höhle  2013-03-02 03:26:28 CST ---
The approach of remembering each buffer's start time and comparing with
AudioQueueGetCurrentTime  (see comment #2) worked out well.

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

Jörg Höhle  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |57bb0898f353d34a22a2d895a14
                   |                            |33877e9a61ab9

[Bug 28039] IAudioClock_GetPosition must ignore underruns (MacOS)

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

Alexandre Julliard  changed:

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

--- Comment #12 from Alexandre Julliard  2013-03-15 14:46:13 CDT ---
Closing bugs fixed in 1.5.26.