Re: [PATCH 3/7] qedit: Implement IMediaDet::GetSampleGrabber.

[Zebediah Figura]

On 10/19/20 11:48 AM, Gabriel Ivăncescu wrote:
> Signed-off-by: Gabriel Ivăncescu
>
> dlls/qedit/mediadet.c       |  13 +++--
>  dlls/qedit/tests/mediadet.c | 101 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 110 insertions(+), 4 deletions(-)
>
> diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
> index 11498bd..6afae37 100644
> --- a/dlls/qedit/mediadet.c
> +++ b/dlls/qedit/mediadet.c
> @@ -761,9 +761,16 @@ static HRESULT WINAPI MediaDet_get_StreamMediaType(IMediaDet* iface,
>  static HRESULT WINAPI MediaDet_GetSampleGrabber(IMediaDet* iface,
>                                                  ISampleGrabber **ppVal)
>  {
> - MediaDetImpl *This = impl_from_IMediaDet(iface);
> - FIXME("(%p)->(%p): not implemented!\n", This, ppVal);
> - return E_NOTIMPL;
> - MediaDetImpl *detector = impl_from_IMediaDet(iface);
> -
> - TRACE("(%p)->(%p)\n", detector, ppVal);
> -
> - if (!detector->grabber)
> -
>    return E_NOINTERFACE;
> -
> - *ppVal = detector->grabber;
> - ISampleGrabber_AddRef(*ppVal);
> - return S_OK;
>
> }
>
> static HRESULT WINAPI MediaDet_get_FrameRate(IMediaDet* iface, double *pVal)
> diff --git a/dlls/qedit/tests/mediadet.c b/dlls/qedit/tests/mediadet.c
> index afad173..412d574 100644
> --- a/dlls/qedit/tests/mediadet.c
> +++ b/dlls/qedit/tests/mediadet.c
> @@ -627,6 +627,39 @@ static BOOL unpack_avi_file(int id, WCHAR name[MAX_PATH])
>      return ret && written == size;
>  }
>
> +static HRESULT get_first_pin(IBaseFilter *filter, PIN_DIRECTION pin_dir, IPin **out)
> +{
> - IEnumPins *pins;
> - HRESULT hr;
> - IPin *pin;
> -
> - *out = NULL;
> - if (FAILED(hr = IBaseFilter_EnumPins(filter, &pins)))
> -
>    return hr;
> -
> - while (IEnumPins_Next(pins, 1, &pin, NULL) == S_OK)
> - {
> -
>    PIN_DIRECTION dir;
> -
> -
>    hr = IPin_QueryDirection(pin, &dir);
> -
>    if (FAILED(hr))
> -
>    {
> -
>        IPin_Release(pin);
> -
>        IEnumPins_Release(pins);
> -
>        return hr;
> -
>    }
> -
>    if (dir == pin_dir)
> -
>    {
> -
>        *out = pin;
> -
>        IEnumPins_Release(pins);
> -
>        return S_OK;
> -
>    }
> -
>    IPin_Release(pin);
> - }
> - IEnumPins_Release(pins);
> - return E_NOTIMPL;
>
> +}
> -
>
> static BOOL init_tests(void)
>  {
>      return unpack_avi_file(TEST_AVI_RES, test_avi_filename)
> @@ -1318,14 +1351,23 @@ static void test_bitmap_grab_mode(void)
>          &TIME_FORMAT_BYTE,
>          &TIME_FORMAT_MEDIA_TIME
>      };
> - char *buf = malloc(640 * 480 * 3);
> struct testfilter testfilter;
> - FILTER_INFO filter_info;
> - IReferenceClock *clock;
> - IBaseFilter *filter;
> IMediaDet *detector;
> - ISampleGrabber *sg;
> - FILTER_STATE state;
> - PIN_INFO pin_info;
> AM_MEDIA_TYPE mt;
> - IMediaFilter *mf;
> - LONG count, size;
> - IPin *pin, *pin2;
> double duration;
> IUnknown *unk;
> unsigned i;
> HRESULT hr;
> - LONG count;
> ULONG ref;
> GUID guid;
> BSTR str;
>
> @@ -1336,6 +1378,8 @@ static void test_bitmap_grab_mode(void)
>  hr = IMediaDet_EnterBitmapGrabMode(detector, 0.0);
>  ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
> -
>
> hr = IMediaDet_GetSampleGrabber(detector, &sg);
> -
>
> ok(hr == E_NOINTERFACE, "Got hr %#x.\n", hr);
>
> /* EnterBitmapGrabMode only seeks once, and if SeekTime is non-negative */
> testfilter_init(&testfilter);
>
> @@ -1429,10 +1473,64 @@ static void test_bitmap_grab_mode(void)
>      hr = IMediaDet_put_CurrentStream(detector, 0);
>      ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
> - /* Check the SampleGrabber */
> - hr = IMediaDet_GetSampleGrabber(detector, &sg);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - hr = ISampleGrabber_GetConnectedMediaType(sg, &mt);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ok(IsEqualGUID(&mt.majortype, &MEDIATYPE_Video), "Got major type %s.\n", debugstr_guid(&mt.majortype));
> - ok(IsEqualGUID(&mt.subtype, &MEDIASUBTYPE_RGB24), "Got sub type %s.\n", debugstr_guid(&mt.subtype));
> - ok(IsEqualGUID(&mt.formattype, &FORMAT_VideoInfo), "Got format type %s.\n", debugstr_guid(&mt.formattype));
> - FreeMediaType(&mt);
> -
> - hr = ISampleGrabber_GetCurrentBuffer(sg, &size, NULL);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ok(size == 640 * 480 * 3, "Got size %d.\n", size);
> - hr = ISampleGrabber_GetCurrentBuffer(sg, &size, (LONG*)buf);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ok(size == 640 * 480 * 3, "Got size %d.\n", size);
> - hr = ISampleGrabber_QueryInterface(sg, &IID_IBaseFilter, (void**)&filter);
> - ok(hr == S_OK, "QueryInterface for IID_IBaseFilter failed: %08x\n", hr);
> - ISampleGrabber_Release(sg);
> -
> - hr = IBaseFilter_QueryFilterInfo(filter, &filter_info);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ok(!wcscmp(filter_info.achName, L"BitBucket"), "Got name %s.\n", debugstr_w(filter_info.achName));
> - IFilterGraph_Release(filter_info.pGraph);
> - hr = get_first_pin(filter, PINDIR_OUTPUT, &pin);

Same here; you can replace this with FindPin(L"Out").
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - IBaseFilter_Release(filter);
> -
> - hr = IPin_ConnectedTo(pin, &pin2);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - hr = IPin_QueryPinInfo(pin2, &pin_info);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ok(pin_info.pFilter != NULL, "Got NULL filter.\n");
> - IPin_Release(pin2);
> - IPin_Release(pin);
> -
> - hr = IBaseFilter_QueryFilterInfo(pin_info.pFilter, &filter_info);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ok(!wcscmp(filter_info.achName, L"NullRenderer"), "Got name %s.\n", debugstr_w(filter_info.achName));
> - hr = IFilterGraph_QueryInterface(filter_info.pGraph, &IID_IMediaFilter, (void**)&mf);
> - ok(hr == S_OK, "QueryInterface for IID_IMediaFilter failed: %08x\n", hr);
> - IFilterGraph_Release(filter_info.pGraph);
> - IBaseFilter_Release(pin_info.pFilter);
> -
> - hr = IMediaFilter_GetState(mf, INFINITE, &state);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ok(state == State_Paused, "Got state %d.\n", state);
> - hr = IMediaFilter_GetSyncSource(mf, &clock);
> - ok(SUCCEEDED(hr), "Got hr %#x.\n", hr);
> - ok(clock == NULL, "Got non-NULL clock.\n");
> - IMediaFilter_Release(mf);
> - /* Changing filter resets bitmap grab mode */
>    testfilter.bitmap_grab_mode = FALSE;
>    hr = IMediaDet_put_Filter(detector, &testfilter.filter.IUnknown_inner);
>    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);
> ok(hr == S_OK, "Got hr %#x.\n", hr);
> ok(count == 1, "Got %d streams.\n", count);
>
> @@ -1441,6 +1539,7 @@ static void test_bitmap_grab_mode(void)
>      ok(!ref, "Got outstanding refcount %d.\n", ref);
>      ref = IBaseFilter_Release(&testfilter.filter.IBaseFilter_iface);
>      ok(!ref, "Got outstanding refcount %d.\n", ref);
> - free(buf);

I think it's clearer to free things closer to when they're returned,
unless you end up using them later in the function.
> }
>
> START_TEST(mediadet)

Can these tests be moved to before patch 2/7?