[PATCH] amstream: Implement SSUPDATE_CONTINUOUS flag in IDirectDrawStreamSample::Update.
Zebediah Figura
zfigura at codeweavers.com
Thu Oct 8 14:53:24 CDT 2020
On 10/8/20 2:02 PM, Anton Baskanov wrote:
> Signed-off-by: Anton Baskanov <baskanov at gmail.com>
> ---
> dlls/amstream/ddrawstream.c | 36 ++++++++++++++---------
> dlls/amstream/tests/amstream.c | 53 ++++++++++++++++++++++++++++++++++
> 2 files changed, 75 insertions(+), 14 deletions(-)
>
> diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c
> index 5e6e1555d7c..0e5e54bd80d 100644
> --- a/dlls/amstream/ddrawstream.c
> +++ b/dlls/amstream/ddrawstream.c
> @@ -75,10 +75,12 @@ struct ddraw_sample
> RECT rect;
> STREAM_TIME start_time;
> STREAM_TIME end_time;
> + BOOL continuous_update;
> CONDITION_VARIABLE update_cv;
>
> struct list entry;
> HRESULT update_hr;
> + BOOL busy;
As far as I can tell, this is identical to "update_hr == MS_S_PENDING",
except that you set update_hr to MS_S_NOUPDATE in the continuous case
instead, right?
Would it be simpler, then, to keep most of the code handling update_hr
as it is, and instead handle COMPSTAT_NOUPDATEOK/MS_S_NOUPDATE specially
in ::CompletionStatus()?
> };
>
> static HRESULT ddrawstreamsample_create(struct ddraw_stream *parent, IDirectDrawSurface *surface,
> @@ -86,6 +88,7 @@ static HRESULT ddrawstreamsample_create(struct ddraw_stream *parent, IDirectDraw
>
> static void remove_queued_update(struct ddraw_sample *sample)
> {
> + sample->busy = FALSE;
> list_remove(&sample->entry);
> WakeConditionVariable(&sample->update_cv);
> }
> @@ -1348,7 +1351,16 @@ static HRESULT WINAPI ddraw_meminput_Receive(IMemInputPin *iface, IMediaSample *
> sample->update_hr = process_update(sample, top_down_stride, top_down_pointer,
> start_stream_time, end_stream_time);
>
> - remove_queued_update(sample);
> + if (sample->continuous_update && SUCCEEDED(sample->update_hr))
> + {
> + list_remove(&sample->entry);
> + list_add_tail(&sample->parent->update_queue, &sample->entry);
> + }
> + else
> + {
> + remove_queued_update(sample);
> + }
> +
> LeaveCriticalSection(&stream->cs);
> return S_OK;
> }
> @@ -1540,12 +1552,6 @@ static HRESULT WINAPI ddraw_sample_Update(IDirectDrawStreamSample *iface,
> return E_NOTIMPL;
> }
>
> - if (flags & ~SSUPDATE_ASYNC)
> - {
> - FIXME("Unsupported flags %#x.\n", flags);
> - return E_NOTIMPL;
> - }
> -
> EnterCriticalSection(&sample->parent->cs);
>
> if (sample->parent->state != State_Running)
> @@ -1559,13 +1565,16 @@ static HRESULT WINAPI ddraw_sample_Update(IDirectDrawStreamSample *iface,
> LeaveCriticalSection(&sample->parent->cs);
> return MS_S_ENDOFSTREAM;
> }
> - if (MS_S_PENDING == sample->update_hr)
> + if (sample->busy)
> {
> LeaveCriticalSection(&sample->parent->cs);
> return MS_E_BUSY;
> }
>
> - sample->update_hr = MS_S_PENDING;
> + sample->continuous_update = (flags & SSUPDATE_ASYNC) && (flags & SSUPDATE_CONTINUOUS);
This implies that specifying SSUPDATE_CONTINUOUS without SSUPDATE_ASYNC
is equivalent to zero flags, which seems surprising. Is that correct?
It may not exactly be easy to test, if it's not correct,
SSUPDATE_CONTINUOUS without SSUPDATE_ASYNC ends up doing a continuous
update but blocking (until EOS?), but if this line is correct as-is,
then I'd like to see a test for that.
> +
> + sample->update_hr = MS_S_NOUPDATE;
> + sample->busy = TRUE;
> list_add_tail(&sample->parent->update_queue, &sample->entry);
> WakeConditionVariable(&sample->parent->update_queued_cv);
>
> @@ -1575,7 +1584,7 @@ static HRESULT WINAPI ddraw_sample_Update(IDirectDrawStreamSample *iface,
> return MS_S_PENDING;
> }
>
> - while (sample->update_hr == MS_S_PENDING)
> + while (sample->busy)
> SleepConditionVariableCS(&sample->update_cv, &sample->parent->cs, INFINITE);
>
> LeaveCriticalSection(&sample->parent->cs);
> @@ -1592,18 +1601,17 @@ static HRESULT WINAPI ddraw_sample_CompletionStatus(IDirectDrawStreamSample *ifa
>
> EnterCriticalSection(&sample->parent->cs);
>
> - if (sample->update_hr == MS_S_PENDING)
> + if (sample->busy)
> {
> if (flags & (COMPSTAT_NOUPDATEOK | COMPSTAT_ABORT))
> {
> - sample->update_hr = MS_S_NOUPDATE;
> remove_queued_update(sample);
> }
> else if (flags & COMPSTAT_WAIT)
> {
> DWORD start_time = GetTickCount();
> DWORD elapsed = 0;
> - while (sample->update_hr == MS_S_PENDING && elapsed < milliseconds)
> + while (sample->busy && elapsed < milliseconds)
> {
> DWORD sleep_time = milliseconds - elapsed;
> if (!SleepConditionVariableCS(&sample->update_cv, &sample->parent->cs, sleep_time))
> @@ -1613,7 +1621,7 @@ static HRESULT WINAPI ddraw_sample_CompletionStatus(IDirectDrawStreamSample *ifa
> }
> }
>
> - hr = sample->update_hr;
> + hr = sample->busy ? MS_S_PENDING : sample->update_hr;
>
> LeaveCriticalSection(&sample->parent->cs);
>
> diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c
> index 9dda5882a4f..6e6f7483b49 100644
> --- a/dlls/amstream/tests/amstream.c
> +++ b/dlls/amstream/tests/amstream.c
> @@ -7665,6 +7665,59 @@ static void test_ddrawstreamsample_completion_status(void)
> hr = IAMMultiMediaStream_SetState(mmstream, STREAMSTATE_RUN);
> ok(hr == S_OK, "Got hr %#x.\n", hr);
>
> + hr = IDirectDrawStreamSample_Update(stream_sample1, SSUPDATE_ASYNC | SSUPDATE_CONTINUOUS, NULL, NULL, 0);
> + ok(hr == MS_S_PENDING, "Got hr %#x.\n", hr);
> +
> + hr = IDirectDrawStreamSample_CompletionStatus(stream_sample1, COMPSTAT_NOUPDATEOK, 0);
> + ok(hr == MS_S_NOUPDATE, "Got hr %#x.\n", hr);
> +
> + hr = IDirectDrawStreamSample_Update(stream_sample1, SSUPDATE_ASYNC | SSUPDATE_CONTINUOUS, NULL, NULL, 0);
> + ok(hr == MS_S_PENDING, "Got hr %#x.\n", hr);
> +
> + hr = IDirectDrawStreamSample_Update(stream_sample2, SSUPDATE_ASYNC, NULL, NULL, 0);
> + ok(hr == MS_S_PENDING, "Got hr %#x.\n", hr);
> +
> + hr = IDirectDrawStreamSample_CompletionStatus(stream_sample1, 0, 0);
> + ok(hr == MS_S_PENDING, "Got hr %#x.\n", hr);
> +
> + media_sample = ammediastream_allocate_sample(&source, test_data, sizeof(test_data));
> + hr = IMemInputPin_Receive(source.source.pMemInputPin, media_sample);
> + ok(hr == S_OK, "Got hr %#x.\n", hr);
> + ref = IMediaSample_Release(media_sample);
> + ok(!ref, "Got outstanding refcount %d.\n", ref);
> +
> + hr = IDirectDrawStreamSample_CompletionStatus(stream_sample1, 0, 0);
> + ok(hr == MS_S_PENDING, "Got hr %#x.\n", hr);
Can we check that sample2 isn't satisfied here? [Or is native not
consistent?]
> +
> + media_sample = ammediastream_allocate_sample(&source, test_data, sizeof(test_data));
> + hr = IMemInputPin_Receive(source.source.pMemInputPin, media_sample);
> + ok(hr == S_OK, "Got hr %#x.\n", hr);
> + ref = IMediaSample_Release(media_sample);
> + ok(!ref, "Got outstanding refcount %d.\n", ref);
> +
> + hr = IDirectDrawStreamSample_CompletionStatus(stream_sample1, 0, 0);
> + ok(hr == MS_S_PENDING, "Got hr %#x.\n", hr);
> +
> + hr = IDirectDrawStreamSample_CompletionStatus(stream_sample2, 0, 0);
> + ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> + hr = IDirectDrawStreamSample_CompletionStatus(stream_sample1, COMPSTAT_NOUPDATEOK, 0);
> + ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> + hr = IDirectDrawStreamSample_Update(stream_sample1, SSUPDATE_ASYNC | SSUPDATE_CONTINUOUS, NULL, NULL, 0);
> + ok(hr == MS_S_PENDING, "Got hr %#x.\n", hr);
> +
> + hr = IPin_EndOfStream(pin);
> + ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> + hr = IDirectDrawStreamSample_CompletionStatus(stream_sample1, 0, 0);
> + ok(hr == MS_S_ENDOFSTREAM, "Got hr %#x.\n", hr);
> +
> + hr = IAMMultiMediaStream_SetState(mmstream, STREAMSTATE_STOP);
> + ok(hr == S_OK, "Got hr %#x.\n", hr);
> + hr = IAMMultiMediaStream_SetState(mmstream, STREAMSTATE_RUN);
> + ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> hr = IDirectDrawStreamSample_Update(stream_sample1, SSUPDATE_ASYNC, NULL, NULL, 0);
> ok(hr == MS_S_PENDING, "Got hr %#x.\n", hr);
>
>
The DirectX documentation is rather vague about what "continuous" means,
or the behaviour surrounding this flag, and even after these tests I'm
still left with a lot of questions. In particular:
* CompletionStatus() with no flags evidently doesn't take the sample out
of continuous state, but does COMPSTAT_NOUPDATEOK? What about
COMPSTAT_ABORT?
* IPin::EndOfStream() takes the sample out of continuous state, as far
as I think it's possible to tell. The same applies to IPin::BeginFlush()
and IAMMediaStream::SetState(State_Stopped), right?
* What about calling Update() again, with the same flags, or different ones?
* COMPSTAT_WAIT blocks until the sample is taken out of continuous
state, right? This doesn't necessarily need to be tested if the answer
is "yes", since it'll be more than a little annoying; I just want to
make sure.
Also, can we test briefly that Receive() does update the ddraw surface
every time?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201008/1e18f6ef/attachment.sig>
More information about the wine-devel
mailing list