Re: [PATCH 1/5] amstream: Call ::SetFormat in AMDirectDrawStream::CreateSample.

[Zebediah Figura]

On 9/23/20 1:54 PM, Anton Baskanov wrote:
> Signed-off-by: Anton Baskanov
>
> dlls/amstream/ddrawstream.c    | 24 ++++++++++---
>  dlls/amstream/tests/amstream.c | 66 +++++++++++++++++++++++++++++++++-
>  2 files changed, 85 insertions(+), 5 deletions(-)
>
> diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c
> index fdfe0374ab1..679da1bd2d3 100644
> --- a/dlls/amstream/ddrawstream.c
> +++ b/dlls/amstream/ddrawstream.c
> @@ -1449,14 +1449,30 @@ static HRESULT ddrawstreamsample_create(struct ddraw_stream *parent, IDirectDraw
>          }
>      }
> - if (rect)
> -
>    object->rect = *rect;
> - else if (object->surface)

Removing the "if (object->surface)" check is correct regardless of
whether it was intentional (it can't be NULL at this point), but can you
please avoid using such empty scope blocks? Either by reducing the level
of indentation or by using a helper function; either is fine.
>  {
>      DDSURFACEDESC desc = { sizeof(desc) };
>      hr = IDirectDrawSurface_GetSurfaceDesc(object->surface, &desc);
> -
>    if (hr == S_OK)
> -
>    if (FAILED(hr))
> -
>    {
> -
>        IDirectDrawStreamSample_Release(&object->IDirectDrawStreamSample_iface);
> -
>        return hr;
> -
>    }
> -
>    if (rect)
> -
>    {
> -
>        object->rect = *rect;
> -
>        desc.dwWidth = rect->right - rect->left;
> -
>        desc.dwHeight = rect->bottom - rect->top;
> -
>    }
> -
>    else
> -
>    {
>        SetRect(&object->rect, 0, 0, desc.dwWidth, desc.dwHeight);
> -
>    }
> -
>    hr = IDirectDrawMediaStream_SetFormat(&parent->IDirectDrawMediaStream_iface, &desc, NULL);
> -
>    if (FAILED(hr))
> -
>    {
> -
>        IDirectDrawStreamSample_Release(&object->IDirectDrawStreamSample_iface);
> -
>        return hr;
> -
>    }
>
> }
>
> *ddraw_stream_sample = &object->IDirectDrawStreamSample_iface;
>
> diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c
> index 1c0b36ea6c5..d2126a15db3 100644
> --- a/dlls/amstream/tests/amstream.c
> +++ b/dlls/amstream/tests/amstream.c
> @@ -6495,6 +6495,7 @@ static void get_ddrawstream_create_sample_desc_(int line, const DDSURFACEDESC *f
>  static void test_ddrawstream_create_sample(void)
>  {
>      IAMMultiMediaStream *mmstream = create_ammultimediastream();
> - DDSURFACEDESC desc2 = { sizeof(desc2) };
> IDirectDrawSurface *surface, *surface2;
> DDSURFACEDESC desc = { sizeof(desc) };
> IDirectDrawMediaStream *ddraw_stream;
>
> @@ -6591,11 +6592,74 @@ static void test_ddrawstream_create_sample(void)
>      hr = IDirectDrawMediaStream_CreateSample(ddraw_stream, surface, &rect, 0, &sample);
>      ok(hr == S_OK, "Got hr %#x.\n", hr);
> - IDirectDrawMediaStream_Release(ddraw_stream);
> ref = IDirectDrawStreamSample_Release(sample);
> ok(!ref, "Got outstanding refcount %d.\n", ref);
> ref = IDirectDrawSurface_Release(surface);
> ok(!ref, "Got outstanding refcount %d.\n", ref);
> -
> - memset(&desc, 0, sizeof(desc));
> - desc.dwSize = sizeof(desc);
> - desc.dwFlags = DDSD_WIDTH | DDSD_HEIGHT | DDSD_PIXELFORMAT | DDSD_CAPS;
> - desc.dwWidth = 444;
> - desc.dwHeight = 400;
> - desc.ddpfPixelFormat.dwSize = sizeof(DDPIXELFORMAT);
> - desc.ddpfPixelFormat.dwFlags = DDPF_RGB | DDPF_ALPHAPIXELS;
> - desc.ddpfPixelFormat.dwRGBBitCount = 32;
> - desc.ddpfPixelFormat.dwRBitMask = 0xff0000;
> - desc.ddpfPixelFormat.dwGBitMask = 0x00ff00;
> - desc.ddpfPixelFormat.dwBBitMask = 0x0000ff;
> - desc.ddpfPixelFormat.dwRGBAlphaBitMask = 0xff000000;
> - desc.ddsCaps.dwCaps = DDSCAPS_SYSTEMMEMORY | DDSCAPS_OFFSCREENPLAIN;
> - hr = IDirectDraw_CreateSurface(ddraw, &desc, &surface, NULL);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - SetRect(&rect, 111, 100, 333, 300);
> -
> - hr = IDirectDrawMediaStream_CreateSample(ddraw_stream, surface, &rect, 0, &sample);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ref = IDirectDrawStreamSample_Release(sample);
> - ok(!ref, "Got outstanding refcount %d.\n", ref);
> -
> - hr = IDirectDrawMediaStream_CreateSample(ddraw_stream, NULL, NULL, 0, &sample);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - surface2 = NULL;
> - hr = IDirectDrawStreamSample_GetSurface(sample, &surface2, &rect);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IDirectDrawSurface_GetSurfaceDesc(surface, &desc);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - hr = IDirectDrawSurface_GetSurfaceDesc(surface2, &desc2);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ok(desc2.dwWidth == 222, "Got width %u.\n", desc2.dwWidth);
> - ok(desc2.dwHeight == 200, "Got height %u.\n", desc2.dwHeight);
> - ok(memcmp(&desc2.ddpfPixelFormat, &desc.ddpfPixelFormat, sizeof(DDPIXELFORMAT)) == 0,
> -
>        "Pixel format didn't match.\n");
> -
> - ref = IDirectDrawStreamSample_Release(sample);
> - ok(!ref, "Got outstanding refcount %d.\n", ref);
> - ref = IDirectDrawSurface_Release(surface);
> - ok(!ref, "Got outstanding refcount %d.\n", ref);
> - ref = IDirectDrawSurface_Release(surface2);
> - ok(!ref, "Got outstanding refcount %d.\n", ref);
> -
> - memset(&desc, 0, sizeof(desc));
> - desc.dwSize = sizeof(desc);
> - desc.dwFlags = DDSD_WIDTH | DDSD_HEIGHT | DDSD_PIXELFORMAT | DDSD_CAPS;
> - desc.dwWidth = 444;
> - desc.dwHeight = 400;
> - desc.ddpfPixelFormat.dwSize = sizeof(DDPIXELFORMAT);
> - desc.ddpfPixelFormat.dwFlags = DDPF_RGB | DDPF_PALETTEINDEXED4;
> - desc.ddpfPixelFormat.dwRGBBitCount = 4;
> - desc.ddsCaps.dwCaps = DDSCAPS_SYSTEMMEMORY | DDSCAPS_OFFSCREENPLAIN;
> - hr = IDirectDraw_CreateSurface(ddraw, &desc, &surface, NULL);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - hr = IDirectDrawMediaStream_CreateSample(ddraw_stream, surface, NULL, 0, &sample);
> - ok(hr == DDERR_INVALIDSURFACETYPE, "Got hr %#x.\n", hr);
> -
> - IDirectDrawMediaStream_Release(ddraw_stream);
> - ref = IDirectDrawSurface_Release(surface);
> - ok(!ref, "Got outstanding refcount %d.\n", ref);
> ref = IAMMultiMediaStream_Release(mmstream);
> ok(!ref, "Got outstanding refcount %d.\n", ref);
> ref = IMediaStream_Release(stream);