[PATCH 1/3] winegstreamer: Fix integer overflow in scale_uint64.

[Anton Baskanov]

Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
---
 dlls/winegstreamer/quartz_parser.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dlls/winegstreamer/quartz_parser.c b/dlls/winegstreamer/quartz_parser.c
index 243dc9f80ba..fbc9fd32628 100644
--- a/dlls/winegstreamer/quartz_parser.c
+++ b/dlls/winegstreamer/quartz_parser.c
@@ -597,8 +597,8 @@ static uint64_t scale_uint64(uint64_t value, uint32_t numerator, uint32_t denomi
         return 0;
 
     i.QuadPart = value;
-    low.QuadPart = i.u.LowPart * numerator;
-    high.QuadPart = i.u.HighPart * numerator + low.u.HighPart;
+    low.QuadPart = (ULONGLONG)i.u.LowPart * numerator;
+    high.QuadPart = (ULONGLONG)i.u.HighPart * numerator + low.u.HighPart;
     low.u.HighPart = 0;
 
     if (high.u.HighPart >= denominator)
-- 
2.25.1

[PATCH 2/3] quartz/dsoundrender: Don't let writepos advance past playpos.

[Anton Baskanov]

Fixes audio stuttering during video playback in multiple games (e.g.
Commandos 2, Alien Nations, Dino Crisis Demo).

Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
---
 dlls/quartz/dsoundrender.c       | 5 +----
 dlls/quartz/tests/dsoundrender.c | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/dlls/quartz/dsoundrender.c b/dlls/quartz/dsoundrender.c
index 6c6732e468a..f7f0255d4ab 100644
--- a/dlls/quartz/dsoundrender.c
+++ b/dlls/quartz/dsoundrender.c
@@ -149,7 +149,6 @@ static void DSoundRender_UpdatePositions(struct dsound_render *This, DWORD *seqw
 static HRESULT DSoundRender_GetWritePos(struct dsound_render *This,
         DWORD *ret_writepos, REFERENCE_TIME write_at, DWORD *pfree, DWORD *skip)
 {
-    WAVEFORMATEX *wfx = (WAVEFORMATEX *)This->sink.pin.mt.pbFormat;
     DWORD writepos, min_writepos, playpos;
     REFERENCE_TIME max_lag = 50 * 10000;
     REFERENCE_TIME cur, writepos_t, delta_t;
@@ -212,10 +211,8 @@ static HRESULT DSoundRender_GetWritePos(struct dsound_render *This,
         *ret_writepos = (min_writepos + aheadbytes) % This->buf_size;
     }
 end:
-    if (playpos > *ret_writepos)
+    if (playpos >= *ret_writepos)
         *pfree = playpos - *ret_writepos;
-    else if (playpos == *ret_writepos)
-        *pfree = This->buf_size - wfx->nBlockAlign;
     else
         *pfree = This->buf_size + playpos - *ret_writepos;
     if (time_from_pos(This, This->buf_size - *pfree) >= DSoundRenderer_Max_Fill) {
diff --git a/dlls/quartz/tests/dsoundrender.c b/dlls/quartz/tests/dsoundrender.c
index c33c074d93d..e591e1832d5 100644
--- a/dlls/quartz/tests/dsoundrender.c
+++ b/dlls/quartz/tests/dsoundrender.c
@@ -737,7 +737,7 @@ static HRESULT send_frame(IMemInputPin *sink)
     params->sink = sink;
     params->sample = sample;
     thread = CreateThread(NULL, 0, frame_thread, params, 0, NULL);
-    ret = WaitForSingleObject(thread, 500);
+    ret = WaitForSingleObject(thread, 2000);
     todo_wine_if (ret) ok(!ret, "Wait failed.\n");
     GetExitCodeThread(thread, &ret);
     CloseHandle(thread);
-- 
2.25.1

[PATCH 2/3] quartz/dsoundrender: Don't let writepos advance past playpos.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=89311

Your paranoid android.


=== w1064v1809 (64 bit report) ===

quartz:
dsoundrender.c:807: Test failed: Got hr 0x80040227.

[PATCH 2/3] quartz/dsoundrender: Don't let writepos advance past playpos.

[Zebediah Figura (she/her)]

On 4/25/21 9:51 PM, Anton Baskanov wrote:
> Fixes audio stuttering during video playback in multiple games (e.g.
> Commandos 2, Alien Nations, Dino Crisis Demo).

Can you please provide a +quartz,+strmbase,+gstreamer log that 
demonstrates the problem?
> Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
> ---
>   dlls/quartz/dsoundrender.c       | 5 +----
>   dlls/quartz/tests/dsoundrender.c | 2 +-
>   2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/dlls/quartz/dsoundrender.c b/dlls/quartz/dsoundrender.c
> index 6c6732e468a..f7f0255d4ab 100644
> --- a/dlls/quartz/dsoundrender.c
> +++ b/dlls/quartz/dsoundrender.c
> @@ -149,7 +149,6 @@ static void DSoundRender_UpdatePositions(struct dsound_render *This, DWORD *seqw
>   static HRESULT DSoundRender_GetWritePos(struct dsound_render *This,
>           DWORD *ret_writepos, REFERENCE_TIME write_at, DWORD *pfree, DWORD *skip)
>   {
> -    WAVEFORMATEX *wfx = (WAVEFORMATEX *)This->sink.pin.mt.pbFormat;
>       DWORD writepos, min_writepos, playpos;
>       REFERENCE_TIME max_lag = 50 * 10000;
>       REFERENCE_TIME cur, writepos_t, delta_t;
> @@ -212,10 +211,8 @@ static HRESULT DSoundRender_GetWritePos(struct dsound_render *This,
>           *ret_writepos = (min_writepos + aheadbytes) % This->buf_size;
>       }
>   end:
> -    if (playpos > *ret_writepos)
> +    if (playpos >= *ret_writepos)
>           *pfree = playpos - *ret_writepos;
> -    else if (playpos == *ret_writepos)
> -        *pfree = This->buf_size - wfx->nBlockAlign;
>       else
>           *pfree = This->buf_size + playpos - *ret_writepos;
>       if (time_from_pos(This, This->buf_size - *pfree) >= DSoundRenderer_Max_Fill) {
> diff --git a/dlls/quartz/tests/dsoundrender.c b/dlls/quartz/tests/dsoundrender.c
> index c33c074d93d..e591e1832d5 100644
> --- a/dlls/quartz/tests/dsoundrender.c
> +++ b/dlls/quartz/tests/dsoundrender.c
> @@ -737,7 +737,7 @@ static HRESULT send_frame(IMemInputPin *sink)
>       params->sink = sink;
>       params->sample = sample;
>       thread = CreateThread(NULL, 0, frame_thread, params, 0, NULL);
> -    ret = WaitForSingleObject(thread, 500);
> +    ret = WaitForSingleObject(thread, 2000);
>       todo_wine_if (ret) ok(!ret, "Wait failed.\n");
>       GetExitCodeThread(thread, &ret);
>       CloseHandle(thread);

[PATCH 2/3] quartz/dsoundrender: Don't let writepos advance past playpos.

[Anton Baskanov]

On вторник, 27 апреля 2021 г. 06:23:58 +07 you wrote:
> On 4/25/21 9:51 PM, Anton Baskanov wrote:
> > Fixes audio stuttering during video playback in multiple games (e.g.
> > Commandos 2, Alien Nations, Dino Crisis Demo).
>
> Can you please provide a +quartz,+strmbase,+gstreamer log that
> demonstrates the problem?

I've attached the log of the Commandos 2 (GOG version) first intro video 
playback. The relevant part is:

0154:trace:quartz:DSoundRender_SendSampleData Wrote 75500 bytes at 17108, next 
at 4408 - (75500/88200)
0154:trace:quartz:SystemClockImpl_GetTime clock 002D8440, time 03FBFC20, 
returning 1569156.542.
0154:trace:quartz:DSoundRender_SendSampleData Wrote 12700 bytes at 4408, next 
at 17108 - (88196/12700)

First, DSoundRender_GetWritePos reports 75500 free bytes, so we write 75500 
bytes to the buffer. At this point, DSoundRender_GetWritePos must report 0 free 
bytes, since the buffer is full, but it reports 88196 instead, so we write 
another 12700 bytes, overrunning playpos.

Some time after that an underrun happens when dsound write cursor advances 
past our write position:

0154:fixme:quartz:DSoundRender_UpdatePositions Underrun of data occurred!

To reproduce this issue there has to be enough data to fill the buffer, i.e. the 
samples must be at least 1s in length.
> > Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
> > ---
> >
> > dlls/quartz/dsoundrender.c       | 5 +----
> >   dlls/quartz/tests/dsoundrender.c | 2 +-
> >   2 files changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/dlls/quartz/dsoundrender.c b/dlls/quartz/dsoundrender.c
> > index 6c6732e468a..f7f0255d4ab 100644
> > --- a/dlls/quartz/dsoundrender.c
> > +++ b/dlls/quartz/dsoundrender.c
> > @@ -149,7 +149,6 @@ static void DSoundRender_UpdatePositions(struct
> > dsound_render *This, DWORD *seqw> 
> >   static HRESULT DSoundRender_GetWritePos(struct dsound_render *This,
> >
> > DWORD *ret_writepos, REFERENCE_TIME write_at, DWORD *pfree,
> >           DWORD *skip)
> >
> > {
> >
> > -    WAVEFORMATEX *wfx = (WAVEFORMATEX *)This->sink.pin.mt.pbFormat;
> >
> > DWORD writepos, min_writepos, playpos;
> >       REFERENCE_TIME max_lag = 50 * 10000;
> >       REFERENCE_TIME cur, writepos_t, delta_t;
> >
> > @@ -212,10 +211,8 @@ static HRESULT DSoundRender_GetWritePos(struct
> > dsound_render *This,> 
> >           *ret_writepos = (min_writepos + aheadbytes) % This->buf_size;
> >
> > }
> >
> > end:
> > -    if (playpos > *ret_writepos)
> > +    if (playpos >= *ret_writepos)
> >
> > *pfree = playpos - *ret_writepos;
> >
> > -    else if (playpos == *ret_writepos)
> > -        *pfree = This->buf_size - wfx->nBlockAlign;
> >
> > else
> >
> > *pfree = This->buf_size + playpos - *ret_writepos;
> >
> > if (time_from_pos(This, This->buf_size - *pfree) >=
> >       DSoundRenderer_Max_Fill) {> 
> > diff --git a/dlls/quartz/tests/dsoundrender.c
> > b/dlls/quartz/tests/dsoundrender.c index c33c074d93d..e591e1832d5 100644
> > --- a/dlls/quartz/tests/dsoundrender.c
> > +++ b/dlls/quartz/tests/dsoundrender.c
> > @@ -737,7 +737,7 @@ static HRESULT send_frame(IMemInputPin *sink)
> >
> > params->sink = sink;
> >       params->sample = sample;
> >       thread = CreateThread(NULL, 0, frame_thread, params, 0, NULL);
> >
> > -    ret = WaitForSingleObject(thread, 500);
> > +    ret = WaitForSingleObject(thread, 2000);
> >
> > todo_wine_if (ret) ok(!ret, "Wait failed.\n");
> >       GetExitCodeThread(thread, &ret);
> >       CloseHandle(thread);

[PATCH 2/3] quartz/dsoundrender: Don't let writepos advance past playpos.

[Zebediah Figura (she/her)]

On 4/27/21 2:20 AM, Anton Baskanov wrote:
> On вторник, 27 апреля 2021 г. 06:23:58 +07 you wrote:
> > On 4/25/21 9:51 PM, Anton Baskanov wrote:
> > > Fixes audio stuttering during video playback in multiple games (e.g.
> > > Commandos 2, Alien Nations, Dino Crisis Demo).
> >
> > Can you please provide a +quartz,+strmbase,+gstreamer log that
> > demonstrates the problem?
>
> I've attached the log of the Commandos 2 (GOG version) first intro video
> playback. The relevant part is:
>
> 0154:trace:quartz:DSoundRender_SendSampleData Wrote 75500 bytes at 17108, next
> at 4408 - (75500/88200)
> 0154:trace:quartz:SystemClockImpl_GetTime clock 002D8440, time 03FBFC20,
> returning 1569156.542.
> 0154:trace:quartz:DSoundRender_SendSampleData Wrote 12700 bytes at 4408, next
> at 17108 - (88196/12700)
>
> First, DSoundRender_GetWritePos reports 75500 free bytes, so we write 75500
> bytes to the buffer. At this point, DSoundRender_GetWritePos must report 0 free
> bytes, since the buffer is full, but it reports 88196 instead, so we write
> another 12700 bytes, overrunning playpos.
>
> Some time after that an underrun happens when dsound write cursor advances
> past our write position:
>
> 0154:fixme:quartz:DSoundRender_UpdatePositions Underrun of data occurred!
>
> To reproduce this issue there has to be enough data to fill the buffer, i.e. the
> samples must be at least 1s in length.

Thanks, I see the step I was missing.

It took me way too long to make sense of dsoundrender, but I'm finally 
convinced this patch is an improvement, if just barely...
> > > Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
> > > ---
> > >
> > > dlls/quartz/dsoundrender.c       | 5 +----
> > >    dlls/quartz/tests/dsoundrender.c | 2 +-
> > >    2 files changed, 2 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/dlls/quartz/dsoundrender.c b/dlls/quartz/dsoundrender.c
> > > index 6c6732e468a..f7f0255d4ab 100644
> > > --- a/dlls/quartz/dsoundrender.c
> > > +++ b/dlls/quartz/dsoundrender.c
> > > @@ -149,7 +149,6 @@ static void DSoundRender_UpdatePositions(struct
> > > dsound_render *This, DWORD *seqw>
> > >    static HRESULT DSoundRender_GetWritePos(struct dsound_render *This,
> > >
> > > DWORD *ret_writepos, REFERENCE_TIME write_at, DWORD *pfree,
> > >            DWORD *skip)
> > >
> > > {
> > >
> > > -    WAVEFORMATEX *wfx = (WAVEFORMATEX *)This->sink.pin.mt.pbFormat;
> > >
> > > DWORD writepos, min_writepos, playpos;
> > >        REFERENCE_TIME max_lag = 50 * 10000;
> > >        REFERENCE_TIME cur, writepos_t, delta_t;
> > >
> > > @@ -212,10 +211,8 @@ static HRESULT DSoundRender_GetWritePos(struct
> > > dsound_render *This,>
> > >            *ret_writepos = (min_writepos + aheadbytes) % This->buf_size;
> > >
> > > }
> > >
> > > end:
> > > -    if (playpos > *ret_writepos)
> > > +    if (playpos >= *ret_writepos)
> > >
> > > *pfree = playpos - *ret_writepos;
> > >
> > > -    else if (playpos == *ret_writepos)
> > > -        *pfree = This->buf_size - wfx->nBlockAlign;
> > >
> > > else
> > >
> > > *pfree = This->buf_size + playpos - *ret_writepos;
> > >
> > > if (time_from_pos(This, This->buf_size - *pfree) >=
> > >        DSoundRenderer_Max_Fill) {>
> > > diff --git a/dlls/quartz/tests/dsoundrender.c
> > > b/dlls/quartz/tests/dsoundrender.c index c33c074d93d..e591e1832d5 100644
> > > --- a/dlls/quartz/tests/dsoundrender.c
> > > +++ b/dlls/quartz/tests/dsoundrender.c
> > > @@ -737,7 +737,7 @@ static HRESULT send_frame(IMemInputPin *sink)
> > >
> > > params->sink = sink;
> > >        params->sample = sample;
> > >        thread = CreateThread(NULL, 0, frame_thread, params, 0, NULL);
> > >
> > > -    ret = WaitForSingleObject(thread, 500);
> > > +    ret = WaitForSingleObject(thread, 2000);
> > >
> > > todo_wine_if (ret) ok(!ret, "Wait failed.\n");
> > >        GetExitCodeThread(thread, &ret);
> > >        CloseHandle(thread);

[PATCH 2/3] quartz/dsoundrender: Don't let writepos advance past playpos.

[Zebediah Figura (she/her)]

Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>

[PATCH 2/3] quartz/dsoundrender: Don't let writepos advance past playpos.

[Francois Gouget]

Hi,

On Mon, 26 Apr 2021, Anton Baskanov wrote:
> Fixes audio stuttering during video playback in multiple games (e.g.
> Commandos 2, Alien Nations, Dino Crisis Demo).

This patch is causing a new failure in quartz:filtergraph (in Wine 
obviously). Could you have a look?

https://test.winehq.org/data/patterns-tb-wine.html#quartz:filtergraph


filtergraph.c:440: Test failed: GetState() failed: 40237

-- 
Francois Gouget <fgouget(a)codeweavers.com>

[PATCH 2/3] quartz/dsoundrender: Don't let writepos advance past playpos.

[Anton Baskanov]

Hi,

Looks like this failure is actually caused by a different patch (d7feceb 
"winegstreamer: Allow setting the stop position to the stream duration."). I 
haven't debugged it yet, though.

On четверг, 29 апреля 2021 г. 14:48:20 +07 Francois Gouget wrote:
> Hi,
>
> On Mon, 26 Apr 2021, Anton Baskanov wrote:
> > Fixes audio stuttering during video playback in multiple games (e.g.
> > Commandos 2, Alien Nations, Dino Crisis Demo).
>
> This patch is causing a new failure in quartz:filtergraph (in Wine
> obviously). Could you have a look?
>
> https://test.winehq.org/data/patterns-tb-wine.html#quartz:filtergraph
>
> filtergraph.c:440: Test failed: GetState() failed: 40237

[PATCH 3/3] winegstreamer: Allow setting the stop position to the stream duration.

[Anton Baskanov]

Since we no longer stop the GStreamer pipeline, it does not reset the stop position for us. Fixes a hang during intro video playback in RC Cars.

Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
---
 dlls/winegstreamer/quartz_parser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/winegstreamer/quartz_parser.c b/dlls/winegstreamer/quartz_parser.c
index fbc9fd32628..e12a0c49eb3 100644
--- a/dlls/winegstreamer/quartz_parser.c
+++ b/dlls/winegstreamer/quartz_parser.c
@@ -875,7 +875,7 @@ static HRESULT parser_init_stream(struct strmbase_filter *iface)
      * it transitions from stopped -> paused. */
 
     seeking = &filter->sources[0]->seek;
-    if (seeking->llStop && seeking->llStop != seeking->llDuration)
+    if (seeking->llStop)
         stop_flags = AM_SEEKING_AbsolutePositioning;
     unix_funcs->wg_parser_stream_seek(filter->sources[0]->wg_stream, seeking->dRate,
             seeking->llCurrent, seeking->llStop, AM_SEEKING_AbsolutePositioning, stop_flags);
-- 
2.25.1

[PATCH 3/3] winegstreamer: Allow setting the stop position to the stream duration.

[Zebediah Figura (she/her)]

Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>

[Zebediah Figura (she/her)]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>