[PATCH 5/7] qedit: Implement IMediaDet::GetBitmapBits.

Zebediah Figura zfigura at codeweavers.com
Tue Oct 20 12:27:58 CDT 2020


On 10/19/20 11:48 AM, Gabriel Ivăncescu wrote:
> The stretching algorithm that Windows uses does not match any mode on
> StretchBlt or StretchDIBits, which is why the actual algorithm used by
> these APIs was implemented.
> 
> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
> ---
> 
> I've literally tested every mode (on SetStretchBltMode) and operation mode
> and none of them were correct on Windows, even if Wine's implementation is
> not perfect. So clearly Windows doesn't use gdi32 to do the scaling here. It
> was quite a pain to discover the exact algorithm used, but this passes all
> tests on all Windows versions on the testbot.

I appreciate the attention to detail, and the effort put into
reverse-engineering this algorithm. However, pixel-perfect accuracy is
not really a goal of Wine, especially not when we're relying on
libraries like GStreamer to do decoding and format conversions, and in
this case I don't think that it's worth writing or maintaining a new
stretching algorithm. I would instead just use StretchDIBits() with the
closest stretch blit mode.

> 
>  dlls/qedit/mediadet.c       | 191 +++++++++++++++++++++++++++++++++++-
>  dlls/qedit/tests/mediadet.c |  57 ++++++-----
>  2 files changed, 222 insertions(+), 26 deletions(-)
> 
> diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
> index 6afae37..f874095 100644
> --- a/dlls/qedit/mediadet.c
> +++ b/dlls/qedit/mediadet.c
> @@ -324,6 +324,60 @@ done:
>      return hr;
>  }
>  
> +static void stretch_line(BYTE *dst, ULONG dst_width, BYTE *src, ULONG src_width)
> +{
> +    ULONG ratio, rem, drift, i = dst_width;
> +
> +    if (dst_width < src_width)
> +    {
> +        ratio = src_width / dst_width;
> +        rem   = src_width % dst_width;
> +        drift = 0;
> +        while (i--)
> +        {
> +            dst[0] = src[0];
> +            dst[1] = src[1];
> +            dst[2] = src[2];
> +            dst += 3;
> +            src += ratio * 3;
> +            if (drift < rem)
> +            {
> +                src += 3;
> +                drift += dst_width;
> +            }
> +            drift -= rem;
> +        }
> +    }
> +    else if (dst_width > src_width)
> +    {
> +        drift = dst_width - 1;
> +        while (i--)
> +        {
> +            dst[0] = src[0];
> +            dst[1] = src[1];
> +            dst[2] = src[2];
> +            dst += 3;
> +            if (drift < src_width)
> +            {
> +                drift += dst_width;
> +                src += 3;
> +            }
> +            drift -= src_width;
> +        }
> +    }
> +    else
> +    {
> +        memcpy(dst, src, dst_width * 3);
> +        dst += dst_width * 3;
> +    }
> +
> +    /* Fill the stride padding with zeros */
> +    i = (dst_width * 3) % 4;
> +    if (i)
> +        for (i = 4 - i; i--;)
> +            *dst++ = 0;
> +}
> +
>  /* MediaDet inner IUnknown */
>  static HRESULT WINAPI MediaDet_inner_QueryInterface(IUnknown *iface, REFIID riid, void **ppv)
>  {
> @@ -727,10 +781,139 @@ static HRESULT WINAPI MediaDet_GetBitmapBits(IMediaDet* iface,
>                                               LONG *pBufferSize, char *pBuffer,
>                                               LONG Width, LONG Height)
>  {
> -    MediaDetImpl *This = impl_from_IMediaDet(iface);
> -    FIXME("(%p)->(%f %p %p %d %d): not implemented!\n", This, StreamTime, pBufferSize, pBuffer,
> -          Width, Height);
> -    return E_NOTIMPL;
> +    MediaDetImpl *detector = impl_from_IMediaDet(iface);
> +    LONG ratio, rem, stride = (Width * 3 + 3) & ~3;
> +    LONG src_x, src_y, src_stride, size, buf_size;
> +    ULONG src_width, src_height, drift, i;
> +    BITMAPINFOHEADER hdr = { 0 };
> +    BYTE *buf, *dup, *dst, *src;
> +    VIDEOINFOHEADER *info;
> +    AM_MEDIA_TYPE mt;
> +    HRESULT hr;
> +
> +    TRACE("(%p)->(%f %p %p %d %d)\n", detector, StreamTime, pBufferSize, pBuffer, Width, Height);
> +
> +    if (!pBuffer && !pBufferSize) return E_POINTER;
> +    if (Width < 0 || Height < 0) return E_INVALIDARG;
> +    if (!pBuffer)
> +    {
> +        *pBufferSize = sizeof(BITMAPINFOHEADER) + stride * Height;
> +        return S_OK;
> +    }
> +    if (StreamTime < 0.0) return E_INVALIDARG;
> +    if (pBufferSize)
> +    {
> +        if (*pBufferSize < 0)
> +            return E_INVALIDARG;
> +        if (*pBufferSize < sizeof(BITMAPINFOHEADER) + stride * Height)
> +            return E_OUTOFMEMORY;
> +    }
> +
> +    if (detector->grabber)
> +        hr = seek_source(detector, StreamTime);
> +    else
> +        hr = IMediaDet_EnterBitmapGrabMode(&detector->IMediaDet_iface, StreamTime);
> +    if (FAILED(hr))
> +        return hr;
> +
> +    hr = ISampleGrabber_GetConnectedMediaType(detector->grabber, &mt);
> +    info = (VIDEOINFOHEADER*)mt.pbFormat;
> +    if (FAILED(hr) ||
> +        !IsEqualGUID(&mt.majortype, &MEDIATYPE_Video) ||
> +        !IsEqualGUID(&mt.subtype, &MEDIASUBTYPE_RGB24) ||
> +        !IsEqualGUID(&mt.formattype, &FORMAT_VideoInfo) ||
> +        mt.cbFormat != sizeof(VIDEOINFOHEADER) ||
> +        info->bmiHeader.biSize != sizeof(BITMAPINFOHEADER) ||
> +        info->bmiHeader.biWidth <= 0 ||
> +        info->bmiHeader.biHeight == 0 ||
> +        info->bmiHeader.biPlanes != 1 ||
> +        info->bmiHeader.biBitCount != 24 ||
> +        info->bmiHeader.biCompression != BI_RGB)
> +        return VFW_E_INVALID_MEDIA_TYPE;

This may leak "mt.pbFormat". Moreover, can some of these conditions even
happen? And are the others worth checking for?

> +
> +    src_x = src_y = 0;
> +    src_width = info->bmiHeader.biWidth;
> +    src_height = abs(info->bmiHeader.biHeight);
> +    src_stride = (src_width * 3 + 3) & ~3;
> +    buf_size = src_stride * src_height;
> +    if (!IsRectEmpty(&info->rcTarget))
> +    {
> +        src_x = max(info->rcTarget.left, 0);
> +        src_y = max(info->rcTarget.top, 0);
> +        src_width = min(info->rcTarget.right - src_x, src_width);
> +        src_height = min(info->rcTarget.bottom - src_y, src_height);
> +    }
> +    if (info->bmiHeader.biHeight < 0)
> +    {
> +        src_y += info->bmiHeader.biHeight + 1;
> +        src_stride = -src_stride;
> +    }
> +    FreeMediaType(&mt);
> +
> +    if (!(buf = HeapAlloc(GetProcessHeap(), 0, buf_size)))
> +        return E_OUTOFMEMORY;
> +    size = buf_size;
> +    hr = ISampleGrabber_GetCurrentBuffer(detector->grabber, &size, (LONG*)buf);
> +    if (FAILED(hr)) goto err;
> +    if (size != buf_size)
> +    {
> +        hr = E_UNEXPECTED;
> +        goto err;
> +    }
> +
> +    hdr.biSize = sizeof(BITMAPINFOHEADER);
> +    hdr.biWidth = Width;
> +    hdr.biHeight = Height;
> +    hdr.biPlanes = 1;
> +    hdr.biBitCount = 24;
> +    hdr.biCompression = BI_RGB;
> +    memcpy(pBuffer, &hdr, sizeof(BITMAPINFOHEADER));
> +
> +    /* Copy and potentially stretch the image (differently than StretchBlt) */
> +    dst = (BYTE*)pBuffer + sizeof(BITMAPINFOHEADER);
> +    src = buf + src_x * 3 + src_y * src_stride;
> +    i = Height;
> +    if (Height < src_height)
> +    {
> +        ratio = src_height / Height;
> +        rem   = src_height % Height;
> +        drift = 0;
> +        while (i--)
> +        {
> +            stretch_line(dst, Width, src, src_width);
> +            dst += stride;
> +            src += ratio * src_stride;
> +            if (drift < rem)
> +            {
> +                src += src_stride;
> +                drift += Height;
> +            }
> +            drift -= rem;
> +        }
> +    }
> +    else
> +    {
> +        drift = Height - 1;
> +        while (i--)
> +        {
> +            stretch_line(dst, Width, src, src_width);
> +            dup = dst;
> +            dst += stride;
> +            while (drift >= src_height && i--)
> +            {
> +                memcpy(dst, dup, stride);
> +                dst += stride;
> +                drift -= src_height;
> +            }
> +            drift += Height - src_height;
> +            src += src_stride;
> +        }
> +    }
> +
> +    hr = S_OK;
> +err:
> +    HeapFree(GetProcessHeap(), 0, buf);
> +    return hr;
>  }
>  
>  static HRESULT WINAPI MediaDet_WriteBitmapBits(IMediaDet* iface,
> diff --git a/dlls/qedit/tests/mediadet.c b/dlls/qedit/tests/mediadet.c
> index 98372b6..e00226a 100644
> --- a/dlls/qedit/tests/mediadet.c
> +++ b/dlls/qedit/tests/mediadet.c
> @@ -1531,10 +1531,23 @@ static void test_bitmap_grab_mode(void)
>      ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
>      size = sizeof(BITMAPINFOHEADER) + 640 * 480 * 3;
>      hr = IMediaDet_GetBitmapBits(detector, 0.0, &size, buf, 640, 480);
> -    todo_wine ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
> +    ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
>      hr = IMediaDet_GetSampleGrabber(detector, &sg);
>      ok(hr == E_NOINTERFACE, "Got hr %#x.\n", hr);
>  
> +    /* Lines are rounded up to the bitmap stride */
> +    hr = IMediaDet_GetBitmapBits(detector, 0.0, &size, NULL, 640, 480);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +    ok(size == sizeof(BITMAPINFOHEADER) + 640 * 480 * 3, "Got size %d.\n", size);
> +    hr = IMediaDet_GetBitmapBits(detector, 0.0, &size, NULL, 640, 0);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +    ok(size == sizeof(BITMAPINFOHEADER), "Got size %d.\n", size);
> +    hr = IMediaDet_GetBitmapBits(detector, -1.0, &size, NULL, 151, 131);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +    ok(size == sizeof(BITMAPINFOHEADER) + ((151 * 3 + 3) & ~3) * 131, "Got size %d.\n", size);
> +    hr = IMediaDet_GetBitmapBits(detector, 0.0, &size, NULL, -59, -79);
> +    ok(hr == E_INVALIDARG || broken(hr == S_OK) /* WinXP */, "Got hr %#x.\n", hr);
> +
>      /* EnterBitmapGrabMode only seeks once, and if SeekTime is non-negative */
>      testfilter_init(&testfilter);
>      testfilter.bitmap_grab_mode = TRUE;
> @@ -1681,39 +1694,39 @@ static void test_bitmap_grab_mode(void)
>  
>      /* despite what MSDN states, size must be properly supplied on newer Windows versions */
>      hr = IMediaDet_GetBitmapBits(detector, 0.0, NULL, NULL, 640, 480);
> -    todo_wine ok(hr == E_POINTER, "Got hr %#x.\n", hr);
> +    ok(hr == E_POINTER, "Got hr %#x.\n", hr);
>      size = -1;
>      hr = IMediaDet_GetBitmapBits(detector, 0.0, &size, buf, 640, 480);
> -    todo_wine ok(hr == E_INVALIDARG || broken(hr == S_OK) /* WinXP */, "Got hr %#x.\n", hr);
> +    ok(hr == E_INVALIDARG || broken(hr == S_OK) /* WinXP */, "Got hr %#x.\n", hr);
>      size = 640 * 480 * 3;
>      hr = IMediaDet_GetBitmapBits(detector, 0.0, &size, buf, 640, 480);
> -    todo_wine ok(hr == E_OUTOFMEMORY || broken(hr == S_OK) /* WinXP */, "Got hr %#x.\n", hr);
> +    ok(hr == E_OUTOFMEMORY || broken(hr == S_OK) /* WinXP */, "Got hr %#x.\n", hr);
>      ok(size == 640 * 480 * 3, "Got size %d.\n", size);
>      size = sizeof(BITMAPINFOHEADER) + 640 * 480 * 3;
>      hr = IMediaDet_GetBitmapBits(detector, -1.0, &size, buf, 640, 480);
> -    todo_wine ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
> +    ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
>      hr = IMediaDet_GetBitmapBits(detector, 2.5, &size, buf, 640, 480);
> -    todo_wine ok(hr == S_OK, "Got hr %#x.\n", hr);
> -    todo_wine check_bitmap(buf, 640, 480, 2.5);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +    check_bitmap(buf, 640, 480, 2.5);
>  
>      /* GetBitmapBits/WriteBitmapBits can scale the image */
>      size = sizeof(BITMAPINFOHEADER) + 960 * 720 * 3;
>      hr = IMediaDet_GetBitmapBits(detector, 1.5, &size, buf, 131, 151);
> -    todo_wine ok(hr == S_OK, "Got hr %#x.\n", hr);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
>      ok(size == sizeof(BITMAPINFOHEADER) + 960 * 720 * 3, "Got size %d.\n", size);
> -    todo_wine check_bitmap(buf, 131, 151, 1.5);
> +    check_bitmap(buf, 131, 151, 1.5);
>      hr = IMediaDet_GetBitmapBits(detector, 4.0, NULL, buf, 503, 79);
> -    todo_wine ok(hr == S_OK, "Got hr %#x.\n", hr);
> -    todo_wine check_bitmap(buf, 503, 79, 4.0);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +    check_bitmap(buf, 503, 79, 4.0);
>      hr = IMediaDet_GetBitmapBits(detector, 1.52, NULL, buf, 139, 487);
> -    todo_wine ok(hr == S_OK, "Got hr %#x.\n", hr);
> -    todo_wine check_bitmap(buf, 139, 487, 1.52);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +    check_bitmap(buf, 139, 487, 1.52);
>      hr = IMediaDet_GetBitmapBits(detector, 2.12, NULL, buf, 640, 641);
> -    todo_wine ok(hr == S_OK, "Got hr %#x.\n", hr);
> -    todo_wine check_bitmap(buf, 640, 641, 2.12);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +    check_bitmap(buf, 640, 641, 2.12);
>      hr = IMediaDet_GetBitmapBits(detector, 3.25, NULL, buf, 960, 720);
> -    todo_wine ok(hr == S_OK, "Got hr %#x.\n", hr);
> -    todo_wine check_bitmap(buf, 960, 720, 3.25);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +    check_bitmap(buf, 960, 720, 3.25);
>  
>      /* Changing filter resets bitmap grab mode */
>      testfilter.bitmap_grab_mode = FALSE;
> @@ -1728,7 +1741,7 @@ static void test_bitmap_grab_mode(void)
>      /* GetBitmapBits enables it only if it retrieves the image */
>      testfilter.bitmap_grab_mode = TRUE;
>      hr = IMediaDet_GetBitmapBits(detector, 1.25, &size, NULL, 640, 480);
> -    todo_wine ok(hr == S_OK, "Got hr %#x.\n", hr);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
>      hr = IMediaDet_GetSampleGrabber(detector, &sg);
>      ok(hr == E_NOINTERFACE, "Got hr %#x.\n", hr);
>      hr = IMediaDet_get_OutputStreams(detector, &count);
> @@ -1736,12 +1749,12 @@ static void test_bitmap_grab_mode(void)
>      ok(count == 1, "Got %d streams.\n", count);
>  
>      hr = IMediaDet_GetBitmapBits(detector, 1.25, NULL, buf, 640, 480);
> -    todo_wine ok(hr == S_OK, "Got hr %#x.\n", hr);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
>      hr = IMediaDet_GetSampleGrabber(detector, &sg);
> -    todo_wine ok(hr == S_OK, "Got hr %#x.\n", hr);
> -    if (SUCCEEDED(hr)) ISampleGrabber_Release(sg);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +    ISampleGrabber_Release(sg);
>      hr = IMediaDet_get_OutputStreams(detector, &count);
> -    todo_wine ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
> +    ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
>  
>      ref = IMediaDet_Release(detector);
>      ok(!ref, "Got outstanding refcount %d.\n", ref);
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201020/4b1d4b9f/attachment-0001.sig>


More information about the wine-devel mailing list