Re: [PATCH 4/7] wmp: Add media completion notifications

[Zebediah Figura]

On 12/03/18 23:34, Anton Romanov wrote:
> Signed-off-by: Anton Romanov
>
> dlls/wmp/player.c      | 68 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  dlls/wmp/wmp_private.h |  5 ++++
>  2 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/wmp/player.c b/dlls/wmp/player.c
> index 81ce9f2969..42ee06315a 100644
> --- a/dlls/wmp/player.c
> +++ b/dlls/wmp/player.c
> @@ -25,6 +25,7 @@
>  WINE_DEFAULT_DEBUG_CHANNEL(wmp);
>
> static void update_state(WindowsMediaPlayer *wmp, LONG type, LONG state);
> +static DWORD CALLBACK WMP_event_thread(LPVOID parm);
>
> static inline WMPMedia *impl_from_IWMPMedia(IWMPMedia *iface)
>  {
> @@ -197,18 +198,20 @@ static HRESULT WINAPI WMPPlayer4_put_currentMedia(IWMPPlayer4 *iface, IWMPMedia
>  {
>      WindowsMediaPlayer *This = impl_from_IWMPPlayer4(iface);
>      TRACE("(%p)->(%p)\n", This, pMedia);
> -
>
> if(pMedia == NULL) {
>        return E_POINTER;
>    }
>    update_state(This, DISPID_WMPCOREEVENT_OPENSTATECHANGE, WMP_OPEN_STATE_PLAYLIST_CHANGING);
>    if(This->wmpmedia != NULL) {
> -
>    IWMPControls_stop(&This->IWMPControls_iface);
>    IWMPMedia_Release(This->wmpmedia);
>
> }
>    update_state(This, DISPID_WMPCOREEVENT_OPENSTATECHANGE, WMP_OPEN_STATE_PLAYLIST_CHANGED);
>    update_state(This, DISPID_WMPCOREEVENT_OPENSTATECHANGE, WMP_OPEN_STATE_PLAYLIST_OPEN_NO_MEDIA);
> -
>
> IWMPMedia_AddRef(pMedia);
> This->wmpmedia = pMedia;
> - IWMPMedia_AddRef(This->wmpmedia);
> return S_OK;
>
> }
>
> @@ -1401,6 +1404,11 @@ static HRESULT WINAPI WMPControls_play(IWMPControls *iface)
>              hres = IGraphBuilder_QueryInterface(This->pFilterGraph, &IID_IMediaControl,
>                      (void**)&This->media_control);
>          update_state(This, DISPID_WMPCOREEVENT_OPENSTATECHANGE, WMP_OPEN_STATE_MEDIA_OPEN);
> -
>    if (SUCCEEDED(hres))
> -
>        hres = IGraphBuilder_QueryInterface(This->pFilterGraph, &IID_IMediaEvent,
> -
>                (void**)&This->media_event);
> -
>    This->stop_event = CreateEventW(NULL, FALSE, FALSE, NULL);
> -
>    This->event_thread = CreateThread(NULL, 0, WMP_event_thread, This, 0, NULL);

This->stop_event will be leaked here if it was already set; probably you
want to close it in WMPControls_stop() instead.
> HRESULT create_media_from_url(BSTR url, IWMPMedia **ppMedia)
>  {
>      WMPMedia *media = heap_alloc_zero(sizeof(WMPMedia));
> -
> - if (!media) {
> -
>    return E_OUTOFMEMORY;
> - }
> - media->IWMPMedia_iface.lpVtbl = &WMPMediaVtbl;
>    media->url = heap_strdupW(url);
>    media->ref = 1;

This belongs in the same patch where this function was added.
> @@ -1851,3 +1873,47 @@ static void update_state(WindowsMediaPlayer *wmp, LONG type, LONG state)
>      call_sink(wmp->wmpocx, type,
>              &dispparams);
>  }
> -
>
> +static DWORD CALLBACK WMP_event_thread(LPVOID parm)
> +{
> - WindowsMediaPlayer* wmp = (WindowsMediaPlayer *)parm;
> - HRESULT hr;
> - HANDLE handle[2];
> - DWORD n = 0, ret = 0;
> -
> - handle[n++] = wmp->stop_event;
> - IMediaEvent_GetEventHandle(wmp->media_event, (OAEVENT *)&handle[n++]);
> - for (;;) {
> -
>    DWORD r;
> -
> -
>    r = WaitForMultipleObjects(n, handle, FALSE, INFINITE);
> -
>    if (r == WAIT_OBJECT_0) {
> -
>        TRACE("got stop event\n");
> -
>        break;
> -
>    }
> -
>    else if (r == WAIT_OBJECT_0+1) {
> -
>        LONG event_code;
> -
>        LONG_PTR p1, p2;
> -
>        do {
> -
>            hr = IMediaEvent_GetEvent(wmp->media_event, &event_code, &p1, &p2, 0);
> -
>            if (SUCCEEDED(hr)) {
> -
>                TRACE("got event_code = 0x%02x\n", event_code);
> -
>                /* For now we only handle EC_COMPLETE */
> -
>                if (event_code == EC_COMPLETE) {
> -
>                    update_state(wmp, DISPID_WMPCOREEVENT_PLAYSTATECHANGE, WMP_PLAY_STATE_MEDIA_ENDED);
> -
>                    update_state(wmp, DISPID_WMPCOREEVENT_PLAYSTATECHANGE, WMP_PLAY_STATE_TRANSITIONING);
> -
>                    update_state(wmp, DISPID_WMPCOREEVENT_PLAYSTATECHANGE, WMP_PLAY_STATE_STOPPED);
> -
>                }
> -
> -
>                IMediaEvent_FreeEventParams(wmp->media_event, event_code, p1, p2);
> -
>            }
> -
>        } while (hr == S_OK);
> -
>    }
> -
>    else {
> -
>        TRACE("Unknown error (%d)\n", (int)r);
> -
>        break;
> -
>    }
> - }

This probably deserves an ERR more than a TRACE.
> -
> - return ret;
>
> +}
> diff --git a/dlls/wmp/wmp_private.h b/dlls/wmp/wmp_private.h
> index b2077fe77d..1ffc36c74c 100644
> --- a/dlls/wmp/wmp_private.h
> +++ b/dlls/wmp/wmp_private.h
> @@ -75,6 +75,11 @@ struct WindowsMediaPlayer {
>      /* DirectShow stuff */
>      IGraphBuilder* pFilterGraph;
>      IMediaControl* media_control;
> - IMediaEvent*  media_event;
> -
> - /* Async event notification */
> - HANDLE event_thread;
> - HANDLE stop_event;
>
> };
>
> void init_player(WindowsMediaPlayer*) DECLSPEC_HIDDEN;