[PATCH v2 13/13] qedit: Implement MediaDet_get_StreamLength.
Gabriel Ivăncescu
gabrielopcode at gmail.com
Fri Apr 17 07:24:33 CDT 2020
Hi Zeb,
On 16/04/2020 19:52, Zebediah Figura wrote:
> On 4/16/20 10:25 AM, Gabriel Ivăncescu wrote:
>> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
>> ---
>> dlls/qedit/mediadet.c | 39 +++++++++++++++++++++++++++++++++++--
>> dlls/qedit/tests/mediadet.c | 19 ++++++++++++++++++
>> 2 files changed, 56 insertions(+), 2 deletions(-)
>>
>
> I think adding the IMediaPosition and IMediaSeeking interfaces to
> testfilter should be part of this patch rather than 12/13.
>
Right. Will do (same as some of the other emails I won't reply to, no
point flooding the mailing list).
>> diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
>> index 9903238..4990dbe 100644
>> --- a/dlls/qedit/mediadet.c
>> +++ b/dlls/qedit/mediadet.c
>> @@ -569,8 +569,43 @@ static HRESULT WINAPI MediaDet_get_StreamTypeB(IMediaDet* iface, BSTR *pVal)
>> static HRESULT WINAPI MediaDet_get_StreamLength(IMediaDet* iface, double *pVal)
>> {
>> MediaDetImpl *This = impl_from_IMediaDet(iface);
>> - FIXME("(%p): stub!\n", This);
>> - return VFW_E_INVALIDMEDIATYPE;
>> + IMediaSeeking *seeking;
>> + IMediaPosition *pos;
>> + HRESULT hr;
>> +
>> + TRACE("(%p)\n", This);
>> +
>> + if (!pVal)
>> + return E_POINTER;
>> +
>> + if (!This->cur_pin)
>> + return E_INVALIDARG;
>> +
>> + hr = IPin_QueryInterface(This->cur_pin, &IID_IMediaPosition, (void **) &pos);
>> + if (SUCCEEDED(hr))
>> + {
>> + hr = IMediaPosition_get_Duration(pos, pVal);
>> + IMediaPosition_Release(pos);
>> + return hr;
>> + }
>> +
>> + hr = IPin_QueryInterface(This->cur_pin, &IID_IMediaSeeking, (void **) &seeking);
>> + if (SUCCEEDED(hr))
>> + {
>> + LONGLONG duration;
>> +
>> + hr = IMediaSeeking_GetDuration(seeking, &duration);
>> + if (SUCCEEDED(hr))
>> + {
>> + hr = IMediaSeeking_ConvertTimeFormat(seeking, &duration, &TIME_FORMAT_MEDIA_TIME, duration, NULL);
>> + if (SUCCEEDED(hr))
>> + *pVal = (REFTIME)duration / 10000000;
>> + }
>> + IMediaSeeking_Release(seeking);
>> + return hr;
>> + }
>
> You're not really testing this, and for that matter, IMediaSeeking is
> essentially unused in your tests.
>
It's tested for the AVI source, since those don't expose IMediaPosition.
It also seems to be tested on Windows XP because it doesn't query
IMediaPosition.
Now, I can probably just drop the IMediaPosition entirely, if you want
to be perfectly consistent with Windows, although I considered it more
of an implementation detail. The reason I used it first, is because it
has a method to retrieve the duration in the format we want, without
having to resort to conversions.
I don't know about other Windows versions than XP; some (newer) ones
don't even have qedit.
Should I drop IMediaPosition?
>> +
>> + return hr;
>> }
>>
>> static HRESULT WINAPI MediaDet_get_Filename(IMediaDet* iface, BSTR *pVal)
>> diff --git a/dlls/qedit/tests/mediadet.c b/dlls/qedit/tests/mediadet.c
>> index 10127cf..14c99e8 100644
>> --- a/dlls/qedit/tests/mediadet.c
>> +++ b/dlls/qedit/tests/mediadet.c
>> @@ -1060,6 +1060,7 @@ static void test_mediadet(void)
>> GUID guid;
>> BSTR bstr;
>> AM_MEDIA_TYPE mt;
>> + double duration;
>> IUnknown *unk;
>> double fps;
>> int flags;
>> @@ -1122,6 +1123,12 @@ static void test_mediadet(void)
>> hr = IMediaDet_get_StreamTypeB(pM, NULL);
>> ok(hr == E_INVALIDARG, "IMediaDet_get_StreamTypeB failed: %08x\n", hr);
>>
>> + hr = IMediaDet_get_StreamLength(pM, &duration);
>> + ok(hr == E_INVALIDARG, "IMediaDet_get_StreamLength failed: %08x\n", hr);
>> +
>> + hr = IMediaDet_get_StreamLength(pM, NULL);
>> + ok(hr == E_POINTER, "IMediaDet_get_StreamLength failed: %08x\n", hr);
>> +
>> hr = IMediaDet_get_Filter(pM, NULL);
>> ok(hr == E_POINTER, "IMediaDet_get_Filter failed: %08x\n", hr);
>>
>> @@ -1202,6 +1209,10 @@ static void test_mediadet(void)
>> ok(!wcscmp(bstr, L"{73646976-0000-0010-8000-00AA00389B71}"), "Wrong GUID %s\n", wine_dbgstr_w(bstr));
>> SysFreeString(bstr);
>>
>> + hr = IMediaDet_get_StreamLength(pM, &duration);
>> + ok(hr == S_OK, "IMediaDet_get_StreamLength failed: %08x\n", hr);
>> + ok(duration >= 0.1 && duration < 0.10000001, "Wrong duration %.17g\n", duration);
>
> %.16e.
>
> I'm also not sure that I particularly care about adding these tests; as
> mentioned in 12/13, I'd rather see the AVI parts go away entirely.
>
I'd rather we keep the AVI parts since they test a different thing (more
of that in the other patch reply).
>> +
>> /* Even before get_OutputStreams. */
>> hr = IMediaDet_put_CurrentStream(pM, 1);
>> ok(hr == E_INVALIDARG, "IMediaDet_put_CurrentStream failed: %08x\n", hr);
>> @@ -1264,6 +1275,10 @@ static void test_mediadet(void)
>> ok(!wcscmp(bstr, L"{73646976-0000-0010-8000-00AA00389B71}"), "Wrong GUID %s\n", wine_dbgstr_w(bstr));
>> SysFreeString(bstr);
>>
>> + hr = IMediaDet_get_StreamLength(pM, &duration);
>> + ok(hr == S_OK, "IMediaDet_get_StreamLength failed: %08x\n", hr);
>> + ok(duration >= 0.1 && duration < 0.10000001, "Wrong duration %.17g\n", duration);
>> +
>> hr = IMediaDet_get_FrameRate(pM, NULL);
>> ok(hr == E_POINTER, "IMediaDet_get_FrameRate failed: %08x\n", hr);
>>
>> @@ -1392,6 +1407,10 @@ static void test_mediadet(void)
>> ok(hr == S_OK, "IMediaDet_get_OutputStreams failed: %08x\n", hr);
>> ok(nstrms == 1, "IMediaDet_get_OutputStreams: nstrms is %i\n", nstrms);
>>
>> + hr = IMediaDet_get_StreamLength(pM, &duration);
>> + ok(hr == S_OK, "IMediaDet_get_StreamLength failed: %08x\n", hr);
>> + ok(duration >= 4.2 && duration < 4.20000001, "Wrong duration %.17g\n", duration);
>> +
>
> Surely we can at least compare this to 4.2 exactly? That's the double
> that we return.
>
Well, Windows XP reports something like 4.2000000000002 (not the exact
amount of zeros), because it uses IMediaSeeking and does the conversion
which probably loses some accuracy. So we have to keep it, I guess.
>> hr = IMediaDet_Release(pM);
>> ok(hr == 0, "IMediaDet_Release returned: %x\n", hr);
>>
>>
>
More information about the wine-devel
mailing list