On 10/1/20 12:57 PM, Anton Baskanov wrote:
On Thursday, 1 October 2020 00:06:48 +07 you wrote:
On 9/29/20 2:09 PM, Anton Baskanov wrote:
Signed-off-by: Anton Baskanov baskanov@gmail.com
dlls/amstream/ddrawstream.c | 36 ++++++++-- dlls/amstream/tests/amstream.c | 128 ++++++++++++++++++++++++++++++++- 2 files changed, 157 insertions(+), 7 deletions(-)
diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c index 4bd1aa230b4..d940bf88cdc 100644 --- a/dlls/amstream/ddrawstream.c +++ b/dlls/amstream/ddrawstream.c @@ -59,6 +59,7 @@ struct ddraw_stream
struct format format; FILTER_STATE state; BOOL eos;
BOOL flushing;
CONDITION_VARIABLE update_queued_cv; struct list update_queue;
};
@@ -1142,7 +1143,7 @@ static HRESULT WINAPI ddraw_sink_EndOfStream(IPin *iface)> EnterCriticalSection(&stream->cs);
- if (stream->eos)
if (stream->eos || stream->flushing)
{
LeaveCriticalSection(&stream->cs); return E_FAIL;@@ -1159,14 +1160,34 @@ static HRESULT WINAPI ddraw_sink_EndOfStream(IPin *iface)> static HRESULT WINAPI ddraw_sink_BeginFlush(IPin *iface) {
- FIXME("iface %p, stub!\n", iface);
- return E_NOTIMPL;
- struct ddraw_stream *stream = impl_from_IPin(iface);
- TRACE("stream %p.\n", stream);
- EnterCriticalSection(&stream->cs);
- stream->flushing = TRUE;
- stream->eos = FALSE;
- WakeConditionVariable(&stream->update_queued_cv);
- LeaveCriticalSection(&stream->cs);
- return S_OK;
}
static HRESULT WINAPI ddraw_sink_EndFlush(IPin *iface) {
- FIXME("iface %p, stub!\n", iface);
- return E_NOTIMPL;
- struct ddraw_stream *stream = impl_from_IPin(iface);
- TRACE("stream %p.\n", stream);
- EnterCriticalSection(&stream->cs);
- stream->flushing = FALSE;
- LeaveCriticalSection(&stream->cs);
- return S_OK;
}
This implementation worries me a bit (and, admittedly, is one of the drawbacks of using a condition variable), because it's vulnerable to an ABA problem. In particular, it's possible for a main thread to call BeginFlush() and EndFlush() in quick succession, while a thread sleeping in Receive() doesn't wake up until after EndFlush() and hence goes back to sleep. I don't know if Windows actually guarantees a wakeup here, but unless it demonstrably doesn't, we should probably make sure the queue is actually empty in either BeginFlush() or EndFlush(). I think we can use the same CV for that.
Fixed this with another CV and a boolean flag, which is kind of ugly, but I couldn't think of a better solution.
Hmm.
On the one hand, I don't think the flag is particularly ugly, but.
On the other hand, I'm a little bothered now by that statement of mine; I'm not actually sure it's necessary. [In my defense, DirectShow synchronization is hard, and extremely underspecified.] I remember dealing with a similar problem when trying to improve synchronization in the base renderer a while ago, and I came to the conclusion that a patch like this wasn't necessary.
In particular, I think the actual requirements for flushing are that a filter cannot send old samples after EndFlush(), or allow them to be rendered. In practice, if a filter doesn't buffer, it can "pass the buck" up to the upstream filter. Put another way, if filters don't buffer, the only filter that actually needs to wait for the streaming thread to finish is the parser filter, and the parser filter basically always needs to wait *anyway*.
But, just in case, I tested, and surprisingly enough, native amstream's Receive() won't be unblocked if BeginFlush() and EndFlush() are called in quick succession, but if I add a small Sleep() between those, it will be unblocked. Quite surprising for several reasons, but I think that seals the case.
So I'd be perfectly happy to sign off on the original version of this patch, rebased on the other changes ;-)
(Incidentally, patches 2 and 4 of this series look fine.)
static HRESULT WINAPI ddraw_sink_NewSegment(IPin *iface, REFERENCE_TIME start, REFERENCE_TIME stop, double rate)> @@ -1299,6 +1320,11 @@ static HRESULT WINAPI ddraw_meminput_Receive(IMemInputPin *iface, IMediaSample *> LeaveCriticalSection(&stream->cs); return S_OK;
}
if (stream->flushing){LeaveCriticalSection(&stream->cs);return S_FALSE;} if (!list_empty(&stream->update_queue)) { struct ddraw_sample *sample = LIST_ENTRY(list_head(&stream->update_queue), struct ddraw_sample, entry);>diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index 19461d921f6..653c184f0b4 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -4005,6 +4005,7 @@ static IPin *ammediastream_pin;
static IMemInputPin *ammediastream_mem_input_pin; static IMediaSample *ammediastream_media_sample; static DWORD ammediastream_sleep_time;
+static HRESULT ammediastream_expected_hr;
static DWORD CALLBACK ammediastream_end_of_stream(void *param) {
@@ -4012,7 +4013,7 @@ static DWORD CALLBACK ammediastream_end_of_stream(void *param)> Sleep(ammediastream_sleep_time); hr = IPin_EndOfStream(ammediastream_pin);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
ok(hr == ammediastream_expected_hr, "Got hr %#x.\n", hr);
return 0;
}
@@ -4023,7 +4024,7 @@ static DWORD CALLBACK ammediastream_receive(void *param)> Sleep(ammediastream_sleep_time); hr = IMemInputPin_Receive(ammediastream_mem_input_pin, ammediastream_media_sample);>
- ok(hr == S_OK, "Got hr %#x.\n", hr);
ok(hr == ammediastream_expected_hr, "Got hr %#x.\n", hr);
return 0;
}
@@ -4197,6 +4198,7 @@ static void test_audiostreamsample_update(void)
ammediastream_mem_input_pin = mem_input_pin; ammediastream_media_sample = media_sample1; ammediastream_sleep_time = 100;
ammediastream_expected_hr = S_OK;
thread = CreateThread(NULL, 0, ammediastream_receive, NULL, 0, NULL);
hr = IAudioStreamSample_Update(stream_sample, 0, NULL, NULL, 0);
@@ -4216,6 +4218,7 @@ static void test_audiostreamsample_update(void)
ammediastream_pin = pin; ammediastream_sleep_time = 100;
ammediastream_expected_hr = S_OK;
thread = CreateThread(NULL, 0, ammediastream_end_of_stream, NULL, 0, NULL);
hr = IAudioStreamSample_Update(stream_sample, 0, NULL, NULL, 0);
@@ -5157,6 +5160,7 @@ static void test_ddrawstream_receive(void)
ammediastream_mem_input_pin = source.source.pMemInputPin; ammediastream_media_sample = sample; ammediastream_sleep_time = 0;
ammediastream_expected_hr = S_OK;
thread = CreateThread(NULL, 0, ammediastream_receive, NULL, 0, NULL);
ok(WaitForSingleObject(thread, 100) == WAIT_TIMEOUT, "Receive returned prematurely.\n");>
@@ -5196,6 +5200,121 @@ static void test_ddrawstream_receive(void)
ok(!ref, "Got outstanding refcount %d.\n", ref);}
+static void test_ddrawstream_begin_flush_end_flush(void) +{
- IAMMultiMediaStream *mmstream = create_ammultimediastream();
- IDirectDrawStreamSample *stream_sample;
- IDirectDrawMediaStream *ddraw_stream;
- IMediaSample *media_sample;
- IMediaFilter *media_filter;
- struct testfilter source;
- IGraphBuilder *graph;
- IMediaStream *stream;
- VIDEOINFO video_info;
- AM_MEDIA_TYPE mt;
- HANDLE thread;
- HRESULT hr;
- ULONG ref;
- IPin *pin;
- hr = IAMMultiMediaStream_Initialize(mmstream, STREAMTYPE_READ, 0,
NULL); + ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL,
&MSPID_PrimaryVideo, 0, &stream); + ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IMediaStream_QueryInterface(stream, &IID_IDirectDrawMediaStream,
(void **)&ddraw_stream); + ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IMediaStream_QueryInterface(stream, &IID_IPin, (void **)&pin);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IAMMultiMediaStream_GetFilterGraph(mmstream, &graph);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- ok(graph != NULL, "Expected non-NULL graph.\n");
- hr = IGraphBuilder_QueryInterface(graph, &IID_IMediaFilter, (void
**)&media_filter); + ok(hr == S_OK, "Got hr %#x.\n", hr);
- testfilter_init(&source);
- hr = IGraphBuilder_AddFilter(graph, &source.filter.IBaseFilter_iface,
NULL); + ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IMediaFilter_SetSyncSource(media_filter, NULL);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- video_info = rgb32_video_info;
- video_info.bmiHeader.biWidth = 3;
- video_info.bmiHeader.biHeight = 1;
- mt = rgb32_mt;
- mt.pbFormat = (BYTE *)&video_info;
- hr = IGraphBuilder_ConnectDirect(graph,
&source.source.pin.IPin_iface, pin, &mt); + ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IDirectDrawMediaStream_CreateSample(ddraw_stream, NULL, NULL, 0,
&stream_sample); + 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 = BaseOutputPinImpl_GetDeliveryBuffer(&source.source,
&media_sample, NULL, NULL, 0); + ok(hr == S_OK, "Got hr %#x.\n", hr);
- ammediastream_mem_input_pin = source.source.pMemInputPin;
- ammediastream_media_sample = media_sample;
- ammediastream_sleep_time = 0;
- ammediastream_expected_hr = S_FALSE;
- thread = CreateThread(NULL, 0, ammediastream_receive, NULL, 0, NULL);
- hr = IPin_BeginFlush(pin);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- ok(!WaitForSingleObject(thread, 2000), "Wait timed out.\n");
- CloseHandle(thread);
- ref = IMediaSample_Release(media_sample);
- ok(!ref, "Got outstanding refcount %d.\n", ref);
- hr = BaseOutputPinImpl_GetDeliveryBuffer(&source.source,
&media_sample, NULL, NULL, 0); + ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IMemInputPin_Receive(source.source.pMemInputPin, media_sample);
- ok(hr == S_FALSE, "Got hr %#x.\n", hr);
- ref = IMediaSample_Release(media_sample);
- ok(!ref, "Got outstanding refcount %d.\n", ref);
- hr = IDirectDrawStreamSample_Update(stream_sample, SSUPDATE_ASYNC,
NULL, NULL, 0); + ok(hr == MS_S_PENDING, "Got hr %#x.\n", hr);
- hr = IPin_EndOfStream(pin);
- ok(hr == E_FAIL, "Got hr %#x.\n", hr);
- hr = IPin_EndFlush(pin);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = BaseOutputPinImpl_GetDeliveryBuffer(&source.source,
&media_sample, NULL, NULL, 0); + ok(hr == S_OK, "Got hr %#x.\n", hr);
- 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 = IPin_EndOfStream(pin);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IAMMultiMediaStream_SetState(mmstream, STREAMSTATE_STOP);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- IGraphBuilder_Disconnect(graph, pin);
- IGraphBuilder_Disconnect(graph, &source.source.pin.IPin_iface);
- ref = IDirectDrawStreamSample_Release(stream_sample);
- ok(!ref, "Got outstanding refcount %d.\n", ref);
- ref = IAMMultiMediaStream_Release(mmstream);
- ok(!ref, "Got outstanding refcount %d.\n", ref);
- IMediaFilter_Release(media_filter);
- ref = IGraphBuilder_Release(graph);
- ok(!ref, "Got outstanding refcount %d.\n", ref);
- IPin_Release(pin);
- IDirectDrawMediaStream_Release(ddraw_stream);
- ref = IMediaStream_Release(stream);
- ok(!ref, "Got outstanding refcount %d.\n", ref);
+}
static void check_ammediastream_join_am_multi_media_stream(const CLSID *clsid) {
IAMMultiMediaStream *mmstream = create_ammultimediastream();@@ -7062,6 +7181,7 @@ static void test_ddrawstreamsample_update(void)
ammediastream_mem_input_pin = mem_input_pin; ammediastream_media_sample = media_sample; ammediastream_sleep_time = 0;
ammediastream_expected_hr = S_OK;
thread = CreateThread(NULL, 0, ammediastream_receive, NULL, 0, NULL);
Sleep(100);
@@ -7109,6 +7229,7 @@ static void test_ddrawstreamsample_update(void)
ammediastream_mem_input_pin = mem_input_pin; ammediastream_media_sample = media_sample; ammediastream_sleep_time = 0;
ammediastream_expected_hr = S_OK;
thread = CreateThread(NULL, 0, ammediastream_receive, NULL, 0, NULL);
Sleep(100);
@@ -7161,6 +7282,7 @@ static void test_ddrawstreamsample_update(void)
ammediastream_mem_input_pin = mem_input_pin; ammediastream_media_sample = media_sample; ammediastream_sleep_time = 100;
ammediastream_expected_hr = S_OK;
thread = CreateThread(NULL, 0, ammediastream_receive, NULL, 0, NULL);
hr = IDirectDrawStreamSample_Update(stream_sample, 0, NULL, NULL, 0);
@@ -7183,6 +7305,7 @@ static void test_ddrawstreamsample_update(void)
ammediastream_pin = pin; ammediastream_sleep_time = 100;
ammediastream_expected_hr = S_OK;
thread = CreateThread(NULL, 0, ammediastream_end_of_stream, NULL, 0, NULL);
hr = IDirectDrawStreamSample_Update(stream_sample, 0, NULL, NULL, 0);
@@ -7702,6 +7825,7 @@ START_TEST(amstream)
test_ddrawstream_get_format(); test_ddrawstream_set_format(); test_ddrawstream_receive();
test_ddrawstream_begin_flush_end_flush();
test_ddrawstreamsample_get_media_stream(); test_ddrawstreamsample_update();