[PATCH] strmbase: Attempt connection using source pin's current media type.

Zebediah Figura z.figura12 at gmail.com
Thu Aug 27 10:04:06 CDT 2020


On 8/26/20 10:29 PM, Jeff Smith wrote:
> Signed-off-by: Jeff Smith <whydoubt at gmail.com>
> ---
> If no media type is specified, IPin::Connect() currently attempts to
> connect pins using any accepted media type.  With this patch, it first
> attempts to connect using the source pin's current media type.

More specifically, it attempts to connect pins using the media types
returned, in order, from IPin::EnumMediaTypes(). Which leads to the
question: does calling IAMStreamConfig::SetFormat() affect the types
enumerated? I wrote a brief test, attached, which passes for me on
Windows 10 with an Anivia W5, showing that the correct solution here is
to always return the current caps *first* from source_get_media_type().

> 
>  dlls/qcap/tests/videocapture.c | 64 ++++++++++++++++++++++++++++++++--
>  dlls/strmbase/pin.c            | 27 ++++++++++++++
>  2 files changed, 88 insertions(+), 3 deletions(-)
> 
> diff --git a/dlls/qcap/tests/videocapture.c b/dlls/qcap/tests/videocapture.c
> index 80d4899410..7938b48376 100644
> --- a/dlls/qcap/tests/videocapture.c
> +++ b/dlls/qcap/tests/videocapture.c
> @@ -174,7 +174,51 @@ static void test_stream_config(IPin *pin)
>      IAMStreamConfig_Release(stream_config);
>  }
>  
> -static void test_capture(IBaseFilter *filter)
> +static void test_connect(IPin *source_pin, IPin *sink_pin)
> +{
> +    IEnumMediaTypes *enum_media_types;
> +    IAMStreamConfig *stream_config;
> +    AM_MEDIA_TYPE mt, *pmt;
> +    HRESULT hr;
> +
> +    hr = IPin_QueryInterface(source_pin, &IID_IAMStreamConfig, (void **)&stream_config);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> +    hr = IPin_EnumMediaTypes(source_pin, &enum_media_types);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> +    while (IEnumMediaTypes_Next(enum_media_types, 1, &pmt, NULL) == S_OK)
> +    {
> +        hr = IAMStreamConfig_SetFormat(stream_config, pmt);
> +        ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> +        hr = IPin_Connect(source_pin, sink_pin, NULL);
> +        if (hr == S_OK)
> +        {
> +            hr = IPin_ConnectionMediaType(source_pin, &mt);
> +            ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
> +            ok(IsEqualGUID(&pmt->majortype, &mt.majortype), "majortype did not match.\n");
> +            ok(IsEqualGUID(&pmt->subtype, &mt.subtype), "subtype did not match.\n");
> +            ok(IsEqualGUID(&pmt->formattype, &mt.formattype), "formattype did not match.\n");
> +            ok(memcmp(pmt->pbFormat, mt.pbFormat, mt.cbFormat) == 0, "format details did not match.\n");

In general, note that we have a compare_media_types() helper for this,
which you can find in e.g. quartz/avisplit.c.

> +
> +            FreeMediaType(&mt);
> +
> +            hr = IPin_Disconnect(source_pin);
> +            ok(hr == S_OK, "Got hr %#x.\n", hr);
> +            hr = IPin_Disconnect(sink_pin);
> +            ok(hr == S_OK, "Got hr %#x.\n", hr);
> +        }
> +
> +        CoTaskMemFree(pmt);
> +    }
> +
> +    IEnumMediaTypes_Release(enum_media_types);
> +    IAMStreamConfig_Release(stream_config);
> +}
> +
> +static void test_capture(IBaseFilter *filter, IPin *sink_pin)
>  {
>      IEnumPins *enum_pins;
>      IPin *pin;
> @@ -209,6 +253,10 @@ static void test_capture(IBaseFilter *filter)
>              check_interface(pin, &IID_IAMVideoCompression, FALSE);
>              check_interface(pin, &IID_IAMVideoProcAmp, FALSE);
>              check_interface(pin, &IID_IPersistPropertyBag, FALSE);
> +
> +            /* Placed after check_inferface tests, as it can affect the
> +             * result for IID_IAMDroppedFrames on Windows. */
> +            test_connect(pin, sink_pin);

This should be part of the main loop alongside the call to
test_capture(), not part of it. test_capture() should also really be
renamed to test_interfaces().

>          }
>          IPin_Release(pin);
>      }
> @@ -255,7 +303,8 @@ START_TEST(videocapture)
>  {
>      ICreateDevEnum *dev_enum;
>      IEnumMoniker *class_enum;
> -    IBaseFilter *filter;
> +    IBaseFilter *filter, *null_renderer;
> +    IPin *null_renderer_pin;
>      IMoniker *moniker;
>      WCHAR *name;
>      HRESULT hr;
> @@ -277,6 +326,13 @@ START_TEST(videocapture)
>      }
>      ok(hr == S_OK, "Got hr=%#x.\n", hr);
>  
> +    hr = CoCreateInstance(&CLSID_NullRenderer, NULL, CLSCTX_INPROC,
> +            &IID_IBaseFilter, (void **)&null_renderer);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);

We don't particularly want to depend on this being available in tests.
qedit is missing on one of the Windows server versions. Instead we want
to use a strmbase filter; see e.g. struct testfilter in quartz:avisplit
for an example. This also lets us test details of connection.

> +
> +    hr = IBaseFilter_FindPin(null_renderer, L"In", &null_renderer_pin);
> +    ok(hr == S_OK, "Got hr %#x.\n", hr);
> +
>      while (IEnumMoniker_Next(class_enum, 1, &moniker, NULL) == S_OK)
>      {
>          hr = IMoniker_GetDisplayName(moniker, NULL, NULL, &name);
> @@ -287,7 +343,7 @@ START_TEST(videocapture)
>          hr = IMoniker_BindToObject(moniker, NULL, NULL, &IID_IBaseFilter, (void**)&filter);
>          if (hr == S_OK)
>          {
> -            test_capture(filter);
> +            test_capture(filter, null_renderer_pin);
>              test_misc_flags(filter);
>              ref = IBaseFilter_Release(filter);
>              ok(!ref, "Got outstanding refcount %d.\n", ref);
> @@ -298,6 +354,8 @@ START_TEST(videocapture)
>          IMoniker_Release(moniker);
>      }
>  
> +    IPin_Release(null_renderer_pin);
> +    IBaseFilter_Release(null_renderer);
>      ICreateDevEnum_Release(dev_enum);
>      IEnumMoniker_Release(class_enum);
>      CoUninitialize();
> diff --git a/dlls/strmbase/pin.c b/dlls/strmbase/pin.c
> index e5017c2ff9..35aa6bc77d 100644
> --- a/dlls/strmbase/pin.c
> +++ b/dlls/strmbase/pin.c
> @@ -510,6 +510,33 @@ static HRESULT WINAPI source_Connect(IPin *iface, IPin *peer, const AM_MEDIA_TYP
>          return VFW_E_NOT_STOPPED;
>      }
>  
> +    if (!mt)
> +    {
> +        IAMStreamConfig *stream_config = NULL;
> +
> +        if (pin->pFuncsTable->base.pin_query_interface)
> +            pin->pFuncsTable->base.pin_query_interface(&pin->pin, &IID_IAMStreamConfig,
> +                    (void **)&stream_config);
> +
> +        if (stream_config)
> +        {
> +            AM_MEDIA_TYPE *pmt = NULL;
> +            IAMStreamConfig_GetFormat(stream_config, &pmt);
> +            if (pmt)
> +            {
> +                if (pin->pFuncsTable->pfnAttemptConnection(pin, peer, pmt) == S_OK)
> +                {
> +                    LeaveCriticalSection(&pin->pin.filter->csFilter);
> +                    DeleteMediaType(pmt);
> +                    IAMStreamConfig_Release(stream_config);
> +                    return S_OK;
> +                }
> +                DeleteMediaType(pmt);
> +            }
> +            IAMStreamConfig_Release(stream_config);
> +        }
> +    }
> +
>      /* We don't check the subtype here. The rationale (as given by the DirectX
>       * documentation) is that the format type is supposed to provide at least
>       * as much information as the subtype. */
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: videocapture-types.diff
Type: text/x-patch
Size: 3798 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200827/05be3856/attachment-0001.bin>
-------------- 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/20200827/05be3856/attachment-0001.sig>


More information about the wine-devel mailing list