[PATCH] amstream: Implement SSUPDATE_CONTINUOUS flag in IDirectDrawStreamSample::Update.
Signed-off-by: Anton Baskanov <baskanov(a)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; }; 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); + + 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); + + 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); -- 2.17.1
On 10/8/20 2:02 PM, Anton Baskanov wrote:
Signed-off-by: Anton Baskanov <baskanov(a)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?
On Friday, 9 October 2020 02:53:24 +07 you wrote:
On 10/8/20 2:02 PM, Anton Baskanov wrote:
Signed-off-by: Anton Baskanov <baskanov(a)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()?
It would require additional state anyway, because ::CompletionStatus with COMPSTAT_NOUPDATEOK returns different values depending on whether the sample was updated at least once or not (S_OK vs MS_S_NOUPDATE).
};
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.
This should be correct as-is, added a test for it.
+ + 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?]
Done.
+ + 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?
Looking at the return values of ::CompletionStatus, I think they both do.
* 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?
No, actually only EndOfStream does this. Added tests for it.
* What about calling Update() again, with the same flags, or different ones?
This has no effect, returns MS_E_BUSY.
* 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.
Added tests for this and it looks like COMPSTAT_WAIT takes the sample out of continuous state immediately and then waits for completion like with the normal update.
Also, can we test briefly that Receive() does update the ddraw surface every time?
Done.
On 10/9/20 3:55 PM, Anton Baskanov wrote:
On Friday, 9 October 2020 02:53:24 +07 you wrote:
On 10/8/20 2:02 PM, Anton Baskanov wrote:
Signed-off-by: Anton Baskanov <baskanov(a)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()?
It would require additional state anyway, because ::CompletionStatus with COMPSTAT_NOUPDATEOK returns different values depending on whether the sample was updated at least once or not (S_OK vs MS_S_NOUPDATE).
Hmm, I see what you mean. In that case, the way you have it currently is probably best.
};
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.
This should be correct as-is, added a test for it.
+ + 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?]
Done.
+ + 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?
Looking at the return values of ::CompletionStatus, I think they both do.
I'm not sure I want to trust the return value to tell me that. Fortunately, the MS_E_BUSY result from my third question below plus the tests you have currently should prove it another way.
* 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?
No, actually only EndOfStream does this. Added tests for it.
* What about calling Update() again, with the same flags, or different ones?
This has no effect, returns MS_E_BUSY.
* 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.
Added tests for this and it looks like COMPSTAT_WAIT takes the sample out of continuous state immediately and then waits for completion like with the normal update.
Also, can we test briefly that Receive() does update the ddraw surface every time?
Done.
participants (2)
-
Anton Baskanov -
Zebediah Figura