[PATCH v2 0/3] MR10595: amstream: Reject filter when top-down image is not accepted.
On Windows, when the ddraw media stream receives a `ReceiveConnection` request and the media type has a positive height value (i.e. a bottom-up image), it will call `AcceptQuery` on the peers Pin with the sign of the height flipped (i.e. it checks if the peer can produce a top-down image). It will reject the `ReceiveConnection` attempt with `VFW_E_TYPE_NOT_ACCEPTED` if the `AcceptQuery` returns `S_FALSE`. This MR also modifies the return code of `IPin::Disconnect` when not already connected. MS documentation says it should return `S_FALSE` but the implementation actually returns `S_OK`. -- v2: amstream: Always return S_OK on disconnect. amstream: Reject filter when top-down image is not accepted. https://gitlab.winehq.org/wine/wine/-/merge_requests/10595
From: Brendan McGrath <bmcgrath@codeweavers.com> On Windows, when the ddraw media stream receives a ReceiveConnection request and the media type has a positive height value (i.e. a bottom-up image), it will call AcceptQuery on the peers Pin with the sign of the height flipped (i.e. it checks if the peer can produce a top-down image). It will reject the ReceiveConnection attempt with VFW_E_TYPE_NOT_ACCEPTED if the AcceptQuery returns S_FALSE. --- dlls/amstream/tests/amstream.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index 8da987a6367..35f4b72cbe3 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -4083,11 +4083,29 @@ static void test_ddrawstream_receive_connection(void) hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &format, NULL); ok(hr == S_OK, "Got hr %#lx.\n", hr); + /* Return S_FALSE from QueryAccept */ + source.query_accept_hr = S_FALSE; hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &rgb32_mt); ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IPin_Disconnect(pin); ok(hr == S_OK, "Got hr %#lx.\n", hr); + CopyMediaType(&mt, &rgb32_mt); + ((VIDEOINFO*)mt.pbFormat)->bmiHeader.biHeight = -rgb32_video_info.bmiHeader.biHeight; + hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &mt); + todo_wine + ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#lx.\n", hr); + hr = IPin_Disconnect(pin); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + /* Return S_OK from QueryAccept */ + source.query_accept_hr = S_OK; + hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &mt); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IPin_Disconnect(pin); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + FreeMediaType(&mt); + format = rgb8_format; format.dwFlags = DDSD_HEIGHT; format.dwWidth = 333; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10595
From: Brendan McGrath <bmcgrath@codeweavers.com> --- dlls/amstream/ddrawstream.c | 16 ++++++++++++++++ dlls/amstream/tests/amstream.c | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c index 29e17f54542..7186519382b 100644 --- a/dlls/amstream/ddrawstream.c +++ b/dlls/amstream/ddrawstream.c @@ -1060,6 +1060,7 @@ static HRESULT WINAPI ddraw_sink_ReceiveConnection(IPin *iface, IPin *peer, cons DWORD width; DWORD height; DDPIXELFORMAT pf = {sizeof(DDPIXELFORMAT)}; + HRESULT hr; TRACE("stream %p, peer %p, mt %p.\n", stream, peer, mt); strmbase_dump_media_type(mt); @@ -1137,6 +1138,21 @@ static HRESULT WINAPI ddraw_sink_ReceiveConnection(IPin *iface, IPin *peer, cons return VFW_E_INVALID_DIRECTION; } + if (video_info->bmiHeader.biHeight > 0) + { + AM_MEDIA_TYPE top_down_mt; + CopyMediaType(&top_down_mt, mt); + ((VIDEOINFOHEADER*)top_down_mt.pbFormat)->bmiHeader.biHeight = -height; + hr = IPin_QueryAccept(peer, &top_down_mt); + FreeMediaType(&top_down_mt); + if (hr != S_OK) + { + TRACE("Rejecting filter that can't produce top-down images.\n"); + LeaveCriticalSection(&stream->cs); + return VFW_E_TYPE_NOT_ACCEPTED; + } + } + CopyMediaType(&stream->mt, mt); IPin_AddRef(stream->peer = peer); diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index 35f4b72cbe3..74771ea2a13 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -4093,9 +4093,9 @@ static void test_ddrawstream_receive_connection(void) CopyMediaType(&mt, &rgb32_mt); ((VIDEOINFO*)mt.pbFormat)->bmiHeader.biHeight = -rgb32_video_info.bmiHeader.biHeight; hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &mt); - todo_wine ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#lx.\n", hr); hr = IPin_Disconnect(pin); + todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); /* Return S_OK from QueryAccept */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10595
From: Brendan McGrath <bmcgrath@codeweavers.com> The MS documentation for IPin::Disconnect says to return S_FALSE when not connected, but the MS implementation returns S_OK --- dlls/amstream/ddrawstream.c | 4 +++- dlls/amstream/tests/amstream.c | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c index 7186519382b..846814b8fd4 100644 --- a/dlls/amstream/ddrawstream.c +++ b/dlls/amstream/ddrawstream.c @@ -1177,7 +1177,9 @@ static HRESULT WINAPI ddraw_sink_Disconnect(IPin *iface) if (!stream->peer) { LeaveCriticalSection(&stream->cs); - return S_FALSE; + /* The MS documentation for IPin::Disconnect says to return S_FALSE + * when not connected, but the MS implementation returns S_OK */ + return S_OK; } IPin_Release(stream->peer); diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index 74771ea2a13..2f01a2b7688 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -4095,7 +4095,6 @@ static void test_ddrawstream_receive_connection(void) hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &mt); ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#lx.\n", hr); hr = IPin_Disconnect(pin); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); /* Return S_OK from QueryAccept */ @@ -9481,7 +9480,7 @@ static void test_ddrawstream_set_format_dynamic(void) hr = IGraphBuilder_Disconnect(graph, &source.source.pin.IPin_iface); ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IGraphBuilder_Disconnect(graph, pin); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(hr == S_OK, "Got hr %#lx.\n", hr); ref = IAMMultiMediaStream_Release(mmstream); ok(!ref, "Got outstanding refcount %ld.\n", ref); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10595
v2: - Flip the sign of the height on the media type used in `QueryAccept -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10595#note_135409
I wouldn't even bother with the comment in 3/3. Rules in dshow are very much guidelines. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10595#note_135599
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10595
participants (3)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Elizabeth Figura (＠zfigura)