[PATCH 1/3] winepulse.drv: Stop writing the same data again on underflow.

[Rémi Bernon]

On 12/16/21 18:20, Rémi Bernon wrote:
> Data in [lcl_offs_bytes;lcl_offs_bytes+held_bytes] is the data we've
> received from the client and that we consider being used by pulse,
> regardless of how we write it to it.
> 
> A subset of this is [pa_offs_bytes;pa_offs_bytes+pa_held_bytes], which
> is the data we have received from the client but not yet written to
> pulse.
> 
> If pulsed has underflown it means that it has used all the data we've
> written already, and resetting pa_offs_bytes and pa_held_bytes like here
> will only make us write the same data again (in addition to silence) to
> mend the underflow, ending up with potentially duplicate audio frames
> being played.
> 
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52225
> Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
> ---
>   dlls/winepulse.drv/pulse.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c
> index bce70ac358c..c329e8c423c 100644
> --- a/dlls/winepulse.drv/pulse.c
> +++ b/dlls/winepulse.drv/pulse.c
> @@ -260,9 +260,6 @@ static void pulse_underflow_callback(pa_stream *s, void *userdata)
>       struct pulse_stream *stream = userdata;
>       WARN("%p: Underflow\n", userdata);
>       stream->just_underran = TRUE;
> -    /* re-sync */
> -    stream->pa_offs_bytes = stream->lcl_offs_bytes;
> -    stream->pa_held_bytes = stream->held_bytes;
>   }
>   
>   static void pulse_started_callback(pa_stream *s, void *userdata)
> 

Regarding this, although I couldn't confirm with Still Life 2, I suspect 
it may be the cause of duplicate audio frames, as illustrated in the 
attached picture.

For this picture, I captured pulse output (with Prince of Persia: 
Forgotten Sands), with Wine 7.0-rc1 (so without this patch), but with 
some local changes to add a positive spike before pushing new data, and 
a negative spike after pushing the data, right after we detected an 
underflow.

On the picture, the output audio frames are in blue. The silence is 
likely the underflow, and I suspect pulse to be keeping a bit of data 
internally, which would explain the frames after it, and before the 
first positive spike.

I then added on top, in red, a copy of the portion between the spikes, 
shifted in time to better show that it matches audio frames that have 
already been provided to pulse, and played (and split, before the 
silence, and after). Only the part between the positive blue spike and 
the negative red spike are new audio frames that were not yet sent to pulse.

-- 
Rémi Bernon <rbernon at codeweavers.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: underflow.png
Type: image/png
Size: 13192 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20211216/78ed75e4/attachment.png>