Re: [PATCH v11 2/4] wmp: Add seeking and duration

[Jacek Caban]

Hi Anton,

On 04/13/2018 07:55 AM, Anton Romanov wrote:
> Signed-off-by: Anton Romanov
>
> dlls/wmp/player.c      | 66 +++++++++++++++++++++++++++++++++++-------
>  dlls/wmp/tests/media.c | 33 +++++++++++++++++++++
>  dlls/wmp/wmp_private.h |  3 ++
>  3 files changed, 92 insertions(+), 10 deletions(-)
>
> diff --git a/dlls/wmp/player.c b/dlls/wmp/player.c
> index f4ce62a447..bd7e2f7184 100644
> --- a/dlls/wmp/player.c
> +++ b/dlls/wmp/player.c
> @@ -1396,8 +1396,22 @@ static HRESULT WINAPI WMPControls_Invoke(IWMPControls *iface, DISPID dispIdMembe
>  static HRESULT WINAPI WMPControls_get_isAvailable(IWMPControls *iface, BSTR bstrItem, VARIANT_BOOL *pIsAvailable)
>  {
>      WindowsMediaPlayer *This = impl_from_IWMPControls(iface);
> - FIXME("(%p)->(%s)\n", This, debugstr_w(bstrItem));
> - return E_NOTIMPL;
> - static const WCHAR currentPosition[] = {'c','u','r','r','e','n','t','P','o','s','i','t','i','o','n',0};
> - TRACE("(%p)->(%s %p)\n", This, debugstr_w(bstrItem), pIsAvailable);
> - *pIsAvailable = VARIANT_FALSE;

This is not needed, you set it anyway below.
> - if (!This->filter_graph) {
> -
>    *pIsAvailable = VARIANT_FALSE;
> - } else if (strcmpW(currentPosition, bstrItem) == 0) {
> -
>    DWORD capabilities;
> -
>    IMediaSeeking_GetCapabilities(This->media_seeking, &capabilities);
> -
>    *pIsAvailable = (capabilities & AM_SEEKING_CanSeekAbsolute) ?
> -
>        VARIANT_TRUE : VARIANT_FALSE;
> - } else {
> -
>    FIXME("%s not implemented\n", debugstr_w(bstrItem));
> -
>    return E_NOTIMPL;
> - }
> -
> - return S_OK;
>
> }
> CLEAR_CALLED(OPENSTATE, wmposMediaOpening);
>
> - hres = IWMPControls_get_isAvailable(controls, bstrcurrentPosition, &vbool);
> - ok(hres == S_OK, "IWMPControls_get_isAvailable failed: %08x\n", hres);
> - ok(vbool == VARIANT_TRUE, "unexpected value\n");
> -
> - duration = 0.0;
> - hres = IWMPControls_get_currentPosition(controls, &duration);
> - ok(hres == S_OK, "IWMPControls_get_currentPosition failed: %08x\n", hres);
> - ok((int)duration == 0, "unexpected value %f\n", duration);
> -
> - hres = IWMPControls_put_currentPosition(controls, duration);
> - ok(hres == S_OK, "IWMPControls_put_currentPosition failed: %08x\n", hres);

It would be more interesting to set currentPosition to something different.
> -
> - hres = IWMPPlayer4_get_currentMedia(player4, &media);
> - ok(hres == S_OK, "IWMPPlayer4_get_currentMedia failed: %08x\n", hres);
> - hres = IWMPMedia_get_duration(media, &duration);
> - ok(hres == S_OK, "IWMPMedia_get_duration failed: %08x\n", hres);
> - ok(
> -
>        round(duration) == 60 ||
> -
>        broken(round(duration) == 30) ||
> -
>        broken(round(duration) == 57)
> -
>        , "unexpected value: %f\n", duration);

Indention looks weird here.

Thanks,
Jacek