On 4/17/20 7:24 AM, Gabriel Ivăncescu wrote:
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@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.
The AVI splitter does expose IMediaPosition on Windows (see test_interfaces() in quartz:avisplit), so it's not really testing anything here unless you mix components.
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.
If Windows doesn't bother with IMediaPosition, then yes, it would be easier for us not to bother with it either.
I don't know about other Windows versions than XP; some (newer) ones don't even have qedit.
Just Windows Server 2003, as far as I'm aware.
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.
In that case, can we instead borrow compare_double() from msvcrt:string?
hr = IMediaDet_Release(pM); ok(hr == 0, "IMediaDet_Release returned: %x\n", hr);