[PATCH 3/3] quartz/tests: Add tests for when IMediaSeeking on a filter is queried by the filter graph.

[Gabriel Ivăncescu]

On 06/04/2020 20:09, Zebediah Figura wrote:
> On 4/6/20 8:07 AM, Gabriel Ivăncescu wrote:
>> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
>> ---
>>   dlls/quartz/tests/filtergraph.c | 45 +++++++++++++++++++++++++++++++++
>>   1 file changed, 45 insertions(+)
>>
>> diff --git a/dlls/quartz/tests/filtergraph.c b/dlls/quartz/tests/filtergraph.c
>> index 9988dca..2c787c4 100644
>> --- a/dlls/quartz/tests/filtergraph.c
>> +++ b/dlls/quartz/tests/filtergraph.c
>> @@ -26,6 +26,35 @@
>>   #include "wine/heap.h"
>>   #include "wine/test.h"
>>   
>> +#define DEFINE_EXPECT(func) \
>> +    static BOOL expect_ ## func = FALSE, called_ ## func = FALSE
>> +
>> +#define SET_EXPECT(func) \
>> +    do { called_ ## func = FALSE; expect_ ## func = TRUE; } while(0)
>> +
>> +#define CHECK_EXPECT2(func) \
>> +    do { \
>> +        ok(expect_ ##func, "unexpected call " #func "\n"); \
>> +        called_ ## func = TRUE; \
>> +    }while(0)
>> +
>> +#define CHECK_EXPECT(func) \
>> +    do { \
>> +        CHECK_EXPECT2(func); \
>> +        expect_ ## func = FALSE; \
>> +    }while(0)
>> +
>> +#define CHECK_CALLED(func) \
>> +    do { \
>> +        ok(called_ ## func, "expected " #func "\n"); \
>> +        expect_ ## func = called_ ## func = FALSE; \
>> +    }while(0)
>> +
>> +#define CLEAR_CALLED(func) \
>> +    expect_ ## func = called_ ## func = FALSE
>> +
>> +DEFINE_EXPECT(QI_IMediaSeeking);
>> +
>>   static const GUID testguid = {0xabbccdde};
>>   
>>   typedef struct TestFilterImpl
>> @@ -1273,6 +1302,7 @@ static HRESULT WINAPI testfilter_QueryInterface(IBaseFilter *iface, REFIID iid,
>>       }
>>       else if (IsEqualGUID(iid, &IID_IMediaSeeking) && filter->IMediaSeeking_iface.lpVtbl)
>>       {
>> +        CHECK_EXPECT2(QI_IMediaSeeking);
>>           *out = &filter->IMediaSeeking_iface;
>>       }
>>       else if (IsEqualGUID(iid, &IID_IReferenceClock) && filter->IReferenceClock_iface.lpVtbl)
>> @@ -3345,8 +3375,10 @@ todo_wine
>>   static HRESULT check_ec_complete(IFilterGraph2 *graph, IBaseFilter *filter)
>>   {
>>       IMediaEventSink *eventsink;
>> +    BOOL has_seeking = FALSE;
>>       LONG_PTR param1, param2;
>>       IMediaControl *control;
>> +    IMediaSeeking *seeking;
>>       IMediaEvent *eventsrc;
>>       HRESULT hr, ret_hr;
>>       LONG code;
>> @@ -3355,7 +3387,17 @@ static HRESULT check_ec_complete(IFilterGraph2 *graph, IBaseFilter *filter)
>>       IFilterGraph2_QueryInterface(graph, &IID_IMediaEvent, (void **)&eventsrc);
>>       IFilterGraph2_QueryInterface(graph, &IID_IMediaEventSink, (void **)&eventsink);
>>   
>> +    SET_EXPECT(QI_IMediaSeeking);
>> +    if (SUCCEEDED(IBaseFilter_QueryInterface(filter, &IID_IMediaSeeking, (void **)&seeking)))
>> +    {
>> +        IMediaSeeking_Release(seeking);
>> +        has_seeking = TRUE;
>> +    }
>> +    CLEAR_CALLED(QI_IMediaSeeking);
>> +
>> +    if (has_seeking) SET_EXPECT(QI_IMediaSeeking);
>>       IMediaControl_Run(control);
>> +    if (has_seeking) CHECK_CALLED(QI_IMediaSeeking);
>>   
>>       hr = IMediaEvent_GetEvent(eventsrc, &code, &param1, &param2, 0);
>>       ok(hr == E_ABORT, "Got hr %#x.\n", hr);
>> @@ -3766,9 +3808,12 @@ static void test_graph_seeking(void)
>>   
>>       filter1.seek_caps = AM_SEEKING_CanDoSegments | AM_SEEKING_CanGetCurrentPos;
>>       filter2.seek_caps = AM_SEEKING_CanDoSegments | AM_SEEKING_CanGetDuration;
>> +
>> +    SET_EXPECT(QI_IMediaSeeking);
>>       hr = IMediaSeeking_GetCapabilities(seeking, &caps);
>>       ok(hr == S_OK, "Got hr %#x.\n", hr);
>>       ok(caps == AM_SEEKING_CanDoSegments, "Got caps %#x.\n", caps);
>> +    CHECK_CALLED(QI_IMediaSeeking);
>>   
>>       caps = AM_SEEKING_CanDoSegments | AM_SEEKING_CanGetCurrentPos;
>>       hr = IMediaSeeking_CheckCapabilities(seeking, &caps);
>>
> 
> 
> Does "The Legend of Heroes" actually depend on IMediaSeeking being
> queried at a certain time, or is this just a way of testing that it's
> not queried every time we do a seeking call?
> 
> If the latter, I think it would be better to test the part that actually
> matters, i.e. that IMediaSeeking::Release() is only called once, and
> only when the graph is torn down. That way you don't need patch 2/3.
> 
> (As an aside, I've always felt sort of lukewarm about the CHECK_EXPECT
> framework; I think a simple global variable such as is used in the
> qasf:dmowrapper tests is more transparent and flexible.)
> 

Hi Zeb,

You're right, it's only on the Release, I just wanted to make it closer 
to Windows, but it seems it might be too much of an implementation detail.

For the tests, I don't think calling Release itself multiple times is 
the real problem, but only when the ref count becomes 0, so I'll need to 
add a separate ref count just for testing the IMediaSeeking.

Thanks,
Gabriel