[4/5] [v2] strmbase: Add validation checks when updating source rectangle.
Andrew Eikum
aeikum at codeweavers.com
Mon Nov 28 09:54:17 CST 2016
Signed-off-by: Andrew Eikum <aeikum at codeweavers.com>
On Fri, Nov 25, 2016 at 01:10:05AM +0900, Akihiro Sagawa wrote:
>
> In this version,
> - Use IsRectEmpty().
> - Reorder conditional statements to clear the case.
> - Rebased and remove todos.
>
> Signed-off-by: Akihiro Sagawa <sagawa.aki at gmail.com>
> ---
> dlls/quartz/tests/filtergraph.c | 24 ++++++------
> dlls/strmbase/video.c | 85 +++++++++++++++++++++++++++++++----------
> 2 files changed, 76 insertions(+), 33 deletions(-)
>
> diff --git a/dlls/quartz/tests/filtergraph.c b/dlls/quartz/tests/filtergraph.c
> index 93fb2a0..1b72008 100644
> --- a/dlls/quartz/tests/filtergraph.c
> +++ b/dlls/quartz/tests/filtergraph.c
> @@ -85,33 +85,33 @@ static void test_basic_video(void)
> ok(height == video_height, "expected %d, got %d\n", video_height, height);
>
> hr = IBasicVideo_SetSourcePosition(pbv, 0, 0, 0, 0);
> - todo_wine ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> hr = IBasicVideo_SetSourcePosition(pbv, 0, 0, video_width*2, video_height*2);
> - todo_wine ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> hr = IBasicVideo_put_SourceTop(pbv, -1);
> - todo_wine ok(hr==E_INVALIDARG, "IBasicVideo_put_SourceTop returned: %x\n", hr);
> + ok(hr==E_INVALIDARG, "IBasicVideo_put_SourceTop returned: %x\n", hr);
> hr = IBasicVideo_put_SourceTop(pbv, 0);
> ok(hr==S_OK, "Cannot put source top returned: %x\n", hr);
> hr = IBasicVideo_put_SourceTop(pbv, 1);
> - todo_wine ok(hr==E_INVALIDARG, "IBasicVideo_put_SourceTop returned: %x\n", hr);
> + ok(hr==E_INVALIDARG, "IBasicVideo_put_SourceTop returned: %x\n", hr);
>
> hr = IBasicVideo_SetSourcePosition(pbv, video_width, 0, video_width, video_height);
> - todo_wine ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> hr = IBasicVideo_SetSourcePosition(pbv, 0, video_height, video_width, video_height);
> - todo_wine ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> hr = IBasicVideo_SetSourcePosition(pbv, -1, 0, video_width, video_height);
> - todo_wine ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> hr = IBasicVideo_SetSourcePosition(pbv, 0, -1, video_width, video_height);
> - todo_wine ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> hr = IBasicVideo_SetSourcePosition(pbv, video_width/2, video_height/2, video_width, video_height);
> - todo_wine ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> hr = IBasicVideo_SetSourcePosition(pbv, video_width/2, video_height/2, video_width, video_height);
> - todo_wine ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
>
> hr = IBasicVideo_SetSourcePosition(pbv, 0, 0, video_width, video_height+1);
> - todo_wine ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> hr = IBasicVideo_SetSourcePosition(pbv, 0, 0, video_width+1, video_height);
> - todo_wine ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
> + ok(hr==E_INVALIDARG, "IBasicVideo_SetSourcePosition returned: %x\n", hr);
>
> hr = IBasicVideo_SetSourcePosition(pbv, video_width/2, video_height/2, video_width/3+1, video_height/3+1);
> ok(hr==S_OK, "Cannot set source position returned: %x\n", hr);
> diff --git a/dlls/strmbase/video.c b/dlls/strmbase/video.c
> index ce7d26e..3f4d207 100644
> --- a/dlls/strmbase/video.c
> +++ b/dlls/strmbase/video.c
> @@ -53,6 +53,25 @@ HRESULT WINAPI BaseControlVideo_Destroy(BaseControlVideo *pControlVideo)
> return BaseDispatch_Destroy(&pControlVideo->baseDispatch);
> }
>
> +static HRESULT BaseControlVideoImpl_CheckSourceRect(BaseControlVideo *This, RECT *pSourceRect)
> +{
> + LONG VideoWidth, VideoHeight;
> + HRESULT hr;
> +
> + if (IsRectEmpty(pSourceRect))
> + return E_INVALIDARG;
> +
> + hr = BaseControlVideoImpl_GetVideoSize((IBasicVideo *)This, &VideoWidth, &VideoHeight);
> + if (FAILED(hr))
> + return hr;
> +
> + if (pSourceRect->top < 0 || pSourceRect->left < 0 ||
> + pSourceRect->bottom > VideoHeight || pSourceRect->right > VideoWidth)
> + return E_INVALIDARG;
> +
> + return S_OK;
> +}
> +
> HRESULT WINAPI BaseControlVideoImpl_GetTypeInfoCount(IBasicVideo *iface, UINT *pctinfo)
> {
> BaseControlVideo *This = impl_from_IBasicVideo(iface);
> @@ -175,14 +194,20 @@ HRESULT WINAPI BaseControlVideoImpl_put_SourceLeft(IBasicVideo *iface, LONG Sour
> {
> RECT SourceRect;
> BaseControlVideo *This = impl_from_IBasicVideo(iface);
> + HRESULT hr;
>
> TRACE("(%p/%p)->(%d)\n", This, iface, SourceLeft);
> - This->pFuncsTable->pfnGetSourceRect(This, &SourceRect);
> - SourceRect.right = (SourceRect.right - SourceRect.left) + SourceLeft;
> - SourceRect.left = SourceLeft;
> - This->pFuncsTable->pfnSetSourceRect(This, &SourceRect);
> + hr = This->pFuncsTable->pfnGetSourceRect(This, &SourceRect);
> + if (SUCCEEDED(hr))
> + {
> + SourceRect.right = (SourceRect.right - SourceRect.left) + SourceLeft;
> + SourceRect.left = SourceLeft;
> + hr = BaseControlVideoImpl_CheckSourceRect(This, &SourceRect);
> + }
> + if (SUCCEEDED(hr))
> + hr = This->pFuncsTable->pfnSetSourceRect(This, &SourceRect);
>
> - return S_OK;
> + return hr;
> }
>
> HRESULT WINAPI BaseControlVideoImpl_get_SourceLeft(IBasicVideo *iface, LONG *pSourceLeft)
> @@ -203,13 +228,19 @@ HRESULT WINAPI BaseControlVideoImpl_put_SourceWidth(IBasicVideo *iface, LONG Sou
> {
> RECT SourceRect;
> BaseControlVideo *This = impl_from_IBasicVideo(iface);
> + HRESULT hr;
>
> TRACE("(%p/%p)->(%d)\n", This, iface, SourceWidth);
> - This->pFuncsTable->pfnGetSourceRect(This, &SourceRect);
> - SourceRect.right = SourceRect.left + SourceWidth;
> - This->pFuncsTable->pfnSetSourceRect(This, &SourceRect);
> + hr = This->pFuncsTable->pfnGetSourceRect(This, &SourceRect);
> + if (SUCCEEDED(hr))
> + {
> + SourceRect.right = SourceRect.left + SourceWidth;
> + hr = BaseControlVideoImpl_CheckSourceRect(This, &SourceRect);
> + }
> + if (SUCCEEDED(hr))
> + hr = This->pFuncsTable->pfnSetSourceRect(This, &SourceRect);
>
> - return S_OK;
> + return hr;
> }
>
> HRESULT WINAPI BaseControlVideoImpl_get_SourceWidth(IBasicVideo *iface, LONG *pSourceWidth)
> @@ -230,14 +261,20 @@ HRESULT WINAPI BaseControlVideoImpl_put_SourceTop(IBasicVideo *iface, LONG Sourc
> {
> RECT SourceRect;
> BaseControlVideo *This = impl_from_IBasicVideo(iface);
> + HRESULT hr;
>
> TRACE("(%p/%p)->(%d)\n", This, iface, SourceTop);
> - This->pFuncsTable->pfnGetSourceRect(This, &SourceRect);
> - SourceRect.bottom = (SourceRect.bottom - SourceRect.top) + SourceTop;
> - SourceRect.top = SourceTop;
> - This->pFuncsTable->pfnSetSourceRect(This, &SourceRect);
> + hr = This->pFuncsTable->pfnGetSourceRect(This, &SourceRect);
> + if (SUCCEEDED(hr))
> + {
> + SourceRect.bottom = (SourceRect.bottom - SourceRect.top) + SourceTop;
> + SourceRect.top = SourceTop;
> + hr = BaseControlVideoImpl_CheckSourceRect(This, &SourceRect);
> + }
> + if (SUCCEEDED(hr))
> + hr = This->pFuncsTable->pfnSetSourceRect(This, &SourceRect);
>
> - return S_OK;
> + return hr;
> }
>
> HRESULT WINAPI BaseControlVideoImpl_get_SourceTop(IBasicVideo *iface, LONG *pSourceTop)
> @@ -258,13 +295,19 @@ HRESULT WINAPI BaseControlVideoImpl_put_SourceHeight(IBasicVideo *iface, LONG So
> {
> RECT SourceRect;
> BaseControlVideo *This = impl_from_IBasicVideo(iface);
> + HRESULT hr;
>
> TRACE("(%p/%p)->(%d)\n", This, iface, SourceHeight);
> - This->pFuncsTable->pfnGetSourceRect(This, &SourceRect);
> - SourceRect.bottom = SourceRect.top + SourceHeight;
> - This->pFuncsTable->pfnSetSourceRect(This, &SourceRect);
> + hr = This->pFuncsTable->pfnGetSourceRect(This, &SourceRect);
> + if (SUCCEEDED(hr))
> + {
> + SourceRect.bottom = SourceRect.top + SourceHeight;
> + hr = BaseControlVideoImpl_CheckSourceRect(This, &SourceRect);
> + }
> + if (SUCCEEDED(hr))
> + hr = This->pFuncsTable->pfnSetSourceRect(This, &SourceRect);
>
> - return S_OK;
> + return hr;
> }
>
> HRESULT WINAPI BaseControlVideoImpl_get_SourceHeight(IBasicVideo *iface, LONG *pSourceHeight)
> @@ -400,9 +443,9 @@ HRESULT WINAPI BaseControlVideoImpl_SetSourcePosition(IBasicVideo *iface, LONG L
> TRACE("(%p/%p)->(%d, %d, %d, %d)\n", This, iface, Left, Top, Width, Height);
>
> SetRect(&SourceRect, Left, Top, Left + Width, Top + Height);
> - This->pFuncsTable->pfnSetSourceRect(This, &SourceRect);
> -
> - return S_OK;
> + if (FAILED(BaseControlVideoImpl_CheckSourceRect(This, &SourceRect)))
> + return E_INVALIDARG;
> + return This->pFuncsTable->pfnSetSourceRect(This, &SourceRect);
> }
>
> HRESULT WINAPI BaseControlVideoImpl_GetSourcePosition(IBasicVideo *iface, LONG *pLeft, LONG *pTop, LONG *pWidth, LONG *pHeight)
>
More information about the wine-patches
mailing list