Re: [PATCH 3/5] amstream: Reject incompatible media types in AMDirectDrawStream::ReceiveConnection.

[Zebediah Figura]

On 8/29/20 8:51 AM, Anton Baskanov wrote:
> Signed-off-by: Anton Baskanov
>
> dlls/amstream/ddrawstream.c    |  41 +++++++++++-
>  dlls/amstream/tests/amstream.c | 110 +++++++++++++++++++++++++++++++++
>  2 files changed, 150 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c
> index a713b80be45..dcb36d0028f 100644
> --- a/dlls/amstream/ddrawstream.c
> +++ b/dlls/amstream/ddrawstream.c
> @@ -53,6 +53,44 @@ struct ddraw_stream
>  static HRESULT ddrawstreamsample_create(struct ddraw_stream *parent, IDirectDrawSurface *surface,
>      const RECT *rect, IDirectDrawStreamSample **ddraw_stream_sample);
>
> +static BOOL is_media_type_compatible(const AM_MEDIA_TYPE *media_type, const DDSURFACEDESC *format)
> +{
> - const VIDEOINFOHEADER *video_info = (const VIDEOINFOHEADER *)media_type->pbFormat;
> -
> - if ((format->dwFlags & DDSD_WIDTH) && video_info->bmiHeader.biWidth != format->dwWidth)
> -
>    return FALSE;
> -
> - if ((format->dwFlags & DDSD_HEIGHT) && abs(video_info->bmiHeader.biHeight) != format->dwHeight)
> -
>    return FALSE;
> -
> - if (format->dwFlags & DDSD_PIXELFORMAT)
> - {
> -
>    const GUID *subtype = &GUID_NULL;
> -
>    switch (format->ddpfPixelFormat.u1.dwRGBBitCount)
> -
>    {
> -
>    case 8:
> -
>        subtype = &MEDIASUBTYPE_RGB8;
> -
>        break;
> -
>    case 16:
> -
>        if (format->ddpfPixelFormat.u3.dwGBitMask == 0x7e0)
> -
>            subtype = &MEDIASUBTYPE_RGB565;
> -
>        else
> -
>            subtype = &MEDIASUBTYPE_RGB555;
> -
>        break;
> -
>    case 24:
> -
>        subtype = &MEDIASUBTYPE_RGB24;
> -
>        break;
> -
>    case 32:
> -
>        subtype = &MEDIASUBTYPE_RGB32;
> -
>        break;
> -
>    }
> -
>    if (!IsEqualGUID(&media_type->subtype, subtype))
> -
>        return FALSE;
> - }

I wonder if there's a nice way to combine the pixel format translation
with the validity check from 2/5, maybe along the lines of a helper
function that checks both.
> -
> - return TRUE;
>
> +}
> -
>
> static inline struct ddraw_stream *impl_from_IAMMediaStream(IAMMediaStream *iface)
>  {
>      return CONTAINING_RECORD(iface, struct ddraw_stream, IAMMediaStream_iface);
> @@ -750,7 +788,8 @@ static HRESULT WINAPI ddraw_sink_ReceiveConnection(IPin *iface, IPin *peer, cons
>                  && !IsEqualGUID(&mt->subtype, &MEDIASUBTYPE_RGB32)
>                  && !IsEqualGUID(&mt->subtype, &MEDIASUBTYPE_RGB555)
>                  && !IsEqualGUID(&mt->subtype, &MEDIASUBTYPE_RGB565))
> -
>        || !IsEqualGUID(&mt->formattype, &FORMAT_VideoInfo))
> -
>        || !IsEqualGUID(&mt->formattype, &FORMAT_VideoInfo)
> -
>        || !is_media_type_compatible(mt, &stream->format))
> {
>        LeaveCriticalSection(&stream->cs);
>        return VFW_E_TYPE_NOT_ACCEPTED;
>
> diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c
> index 2ce72d4f1c1..fe1cca4036c 100644
> --- a/dlls/amstream/tests/amstream.c
> +++ b/dlls/amstream/tests/amstream.c
> @@ -3440,7 +3440,9 @@ static void test_ddrawstream_receive_connection(void)
>      IDirectDrawMediaStream *ddraw_stream;
>      IAMMultiMediaStream *mmstream;
>      struct testfilter source;
> - DDSURFACEDESC format;
> IMediaStream *stream;
> - VIDEOINFO video_info;
> AM_MEDIA_TYPE mt;
> HRESULT hr;
> ULONG ref;
>
> @@ -3507,6 +3509,114 @@ static void test_ddrawstream_receive_connection(void)
>          }
>      }
> - format = rgb8_format;
> - format.dwFlags = DDSD_WIDTH | DDSD_HEIGHT;
> - format.dwWidth = 333;
> - format.dwHeight = 444;
> - hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &format, NULL);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - video_info = rgb555_video_info;
> - video_info.bmiHeader.biWidth = 333;
> - video_info.bmiHeader.biHeight = 444;
> - mt = rgb555_mt;
> - mt.pbFormat = (BYTE *)&video_info;
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &mt);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - hr = IPin_Disconnect(pin);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - video_info = rgb32_video_info;
> - video_info.bmiHeader.biWidth = 333;
> - video_info.bmiHeader.biHeight = -444;
> - mt = rgb32_mt;
> - mt.pbFormat = (BYTE *)&video_info;
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &mt);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - hr = IPin_Disconnect(pin);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - video_info = rgb32_video_info;
> - video_info.bmiHeader.biWidth = 332;
> - video_info.bmiHeader.biHeight = 444;
> - mt = rgb32_mt;
> - mt.pbFormat = (BYTE *)&video_info;
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &mt);
> - ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#x.\n", hr);
> -
> - video_info = rgb32_video_info;
> - video_info.bmiHeader.biWidth = 333;
> - video_info.bmiHeader.biHeight = 443;
> - mt = rgb32_mt;
> - mt.pbFormat = (BYTE *)&video_info;
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &mt);
> - ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#x.\n", hr);
> -
> - hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &rgb8_format, NULL);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &rgb555_mt);
> - ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#x.\n", hr);
> -
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &rgb8_mt);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - hr = IPin_Disconnect(pin);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &rgb555_format, NULL);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &rgb565_mt);
> - ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#x.\n", hr);
> -
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &rgb555_mt);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - hr = IPin_Disconnect(pin);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &rgb565_format, NULL);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &rgb24_mt);
> - ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#x.\n", hr);
> -
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &rgb565_mt);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - hr = IPin_Disconnect(pin);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &rgb24_format, NULL);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &rgb32_mt);
> - ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#x.\n", hr);
> -
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &rgb24_mt);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - hr = IPin_Disconnect(pin);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &rgb32_format, NULL);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &rgb8_mt);
> - ok(hr == VFW_E_TYPE_NOT_ACCEPTED, "Got hr %#x.\n", hr);
> -
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &rgb32_mt);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - hr = IPin_Disconnect(pin);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - format = rgb8_format;
> - format.dwFlags = 0;
> - hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &format, NULL);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IPin_ReceiveConnection(pin, &source.source.pin.IPin_iface, &rgb565_mt);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - hr = IPin_Disconnect(pin);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ref = IAMMultiMediaStream_Release(mmstream);
>    ok(!ref, "Got outstanding refcount %d.\n", ref);
>    IPin_Release(pin);