[PATCH v7 2/6] wmp: Add OPEN/PLAY state change notifications
Jacek Caban
jacek at codeweavers.com
Thu Mar 29 09:11:02 CDT 2018
Hi Anton,
On 03/28/2018 05:31 PM, Anton Romanov wrote:
> diff --git a/dlls/wmp/player.c b/dlls/wmp/player.c
> index 3d0df96bd8..b3da663637 100644
> --- a/dlls/wmp/player.c
> +++ b/dlls/wmp/player.c
> @@ -20,9 +20,12 @@
>
> #include "wine/debug.h"
> #include <nserror.h>
> +#include "wmpids.h"
>
> WINE_DEFAULT_DEBUG_CHANNEL(wmp);
>
> +static void update_state(WindowsMediaPlayer *wmp, LONG type, LONG state);
How about moving implementation here to avoid forward declaration?
> +
> static inline WMPMedia *impl_from_IWMPMedia(IWMPMedia *iface)
> {
> return CONTAINING_RECORD(iface, WMPMedia, IWMPMedia_iface);
> @@ -125,14 +128,20 @@ static HRESULT WINAPI WMPPlayer4_put_URL(IWMPPlayer4 *iface, BSTR url)
> if(url == NULL) {
> return E_POINTER;
> }
> +
> hres = create_media_from_url(url, &media);
> +
> if (SUCCEEDED(hres)) {
> + update_state(This, DISPID_WMPCOREEVENT_PLAYSTATECHANGE, WMP_PLAY_STATE_TRANSITIONING);
> hres = IWMPPlayer4_put_currentMedia(iface, media);
> IWMPMedia_Release(media); /* put will addref */
> +
> + update_state(This, DISPID_WMPCOREEVENT_PLAYSTATECHANGE, WMP_PLAY_STATE_READY);
Should we report ready state if put_currentMedia fails?
>
> @@ -191,9 +200,13 @@ static HRESULT WINAPI WMPPlayer4_put_currentMedia(IWMPPlayer4 *iface, IWMPMedia
> if(pMedia == NULL) {
> return E_POINTER;
> }
> + update_state(This, DISPID_WMPCOREEVENT_OPENSTATECHANGE, WMP_OPEN_STATE_PLAYLIST_CHANGING);
> if(This->wmpmedia != NULL) {
> 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);
> +
> This->wmpmedia = pMedia;
> IWMPMedia_AddRef(This->wmpmedia);
> return S_OK;
> @@ -1388,19 +1401,31 @@ static HRESULT WINAPI WMPControls_play(IWMPControls *iface)
> CLSCTX_INPROC_SERVER,
> &IID_IGraphBuilder,
> (void **)&This->filter_graph);
> + update_state(This, DISPID_WMPCOREEVENT_OPENSTATECHANGE, WMP_OPEN_STATE_OPENING_UNKNOWN_URL);
> +
> if (SUCCEEDED(hres))
> hres = IGraphBuilder_RenderFile(This->filter_graph, media->url, NULL);
> if (SUCCEEDED(hres))
> hres = IGraphBuilder_QueryInterface(This->filter_graph, &IID_IMediaControl,
> (void**)&This->media_control);
> + update_state(This, DISPID_WMPCOREEVENT_OPENSTATECHANGE, WMP_OPEN_STATE_MEDIA_OPEN);
It's similar, is it right in case of loading error?
> diff --git a/dlls/wmp/tests/media.c b/dlls/wmp/tests/media.c
> index 6e4a0c170f..f98a60fbfd 100644
> --- a/dlls/wmp/tests/media.c
> +++ b/dlls/wmp/tests/media.c
> @@ -19,9 +19,42 @@
> #include <wmp.h>
> #include <olectl.h>
> #include <nserror.h>
> +#include <wmpids.h>
> +#include <math.h>
>
> #include "wine/test.h"
>
> +#define DEFINE_EXPECT(func) \
> + static BOOL expect_ ## func = FALSE, called_ ## func = FALSE
> +
> +#define SET_EXPECT(func) \
> + expect_ ## func = TRUE
> +
> +#define CHECK_EXPECT(func) \
> + do { \
> + ok(expect_ ##func, "unexpected call " #func "\n"); \
> + called_ ## func = TRUE; \
> + }while(0)
> +
> +#define CHECK_CALLED(func) \
> + do { \
> + ok(called_ ## func, "expected " #func "\n"); \
> + expect_ ## func = called_ ## func = FALSE; \
> + }while(0)
> +
> +#define CHECK_CALLED_OR_BROKEN(func) \
> + do { \
> + ok(called_ ## func || broken(TRUE), "expected " #func "\n"); \
> + expect_ ## func = called_ ## func = FALSE; \
> + }while(0)
> +
> +DEFINE_EXPECT(PLAYSTATE_CHANGE);
> +DEFINE_EXPECT(OPENSTATE_CHANGE);
> +
> +static BOOL open_state;
> +static BOOL play_state;
> +static HANDLE playing_event;
> +
> static const WCHAR mp3file[] = {'t','e','s','t','.','m','p','3',0};
> static inline WCHAR *load_resource(const WCHAR *name)
> {
> @@ -48,14 +81,114 @@ static inline WCHAR *load_resource(const WCHAR *name)
> return pathW;
> }
>
> +static ULONG WINAPI Dispatch_AddRef(IDispatch *iface)
> +{
> + return 2;
> +}
> +
> +static ULONG WINAPI Dispatch_Release(IDispatch *iface)
> +{
> + return 1;
> +}
> +
> +static HRESULT WINAPI Dispatch_GetTypeInfoCount(IDispatch *iface, UINT *pctinfo)
> +{
> + ok(0, "unexpected call\n");
> + return E_NOTIMPL;
> +}
> +
> +static HRESULT WINAPI Dispatch_GetTypeInfo(IDispatch *iface, UINT iTInfo, LCID lcid,
> + ITypeInfo **ppTInfo)
> +{
> + ok(0, "unexpected call\n");
> + return E_NOTIMPL;
> +}
> +
> +static HRESULT WINAPI Dispatch_GetIDsOfNames(IDispatch *iface, REFIID riid, LPOLESTR *rgszNames,
> + UINT cNames, LCID lcid, DISPID *rgDispId)
> +{
> + ok(0, "unexpected call\n");
> + return E_NOTIMPL;
> +}
> +
> +static HRESULT WINAPI WMPOCXEvents_QueryInterface(IDispatch *iface, REFIID riid, void **ppv)
> +{
> + *ppv = NULL;
> +
> + if(IsEqualGUID(&IID__WMPOCXEvents, riid) || IsEqualGUID(&IID_IDispatch, riid)) {
> + *ppv = iface;
> + return S_OK;
> + }
> +
> + ok(0, "unexpected riid %s\n", wine_dbgstr_guid(riid));
> + return E_NOINTERFACE;
> +}
> +
> +static HRESULT WINAPI WMPOCXEvents_Invoke(IDispatch *iface, DISPID dispIdMember, REFIID riid,
> + LCID lcid, WORD wFlags, DISPPARAMS *pDispParams, VARIANT *pVarResult,
> + EXCEPINFO *pExcepInfo, UINT *puArgErr)
> +{
> + switch(dispIdMember) {
> + /* Uncomment below traces to debug wmp events */
> + case DISPID_WMPCOREEVENT_OPENSTATECHANGE:
> + CHECK_EXPECT(OPENSTATE_CHANGE);
> + open_state = V_UI4(pDispParams->rgvarg);
> + if (winetest_debug > 1)
> + trace("DISPID_WMPCOREEVENT_OPENSTATECHANGE, %d\n", V_UI4(pDispParams->rgvarg));
> + break;
> + case DISPID_WMPCOREEVENT_PLAYSTATECHANGE:
> + CHECK_EXPECT(PLAYSTATE_CHANGE);
> + play_state = V_UI4(pDispParams->rgvarg);
> + if (play_state == WMP_PLAY_STATE_PLAYING) {
> + SetEvent(playing_event);
> + }
> + if (winetest_debug > 1)
> + trace("DISPID_WMPCOREEVENT_PLAYSTATECHANGE, %d\n", V_UI4(pDispParams->rgvarg));
> + break;
How about adding more specific tests? You could split CHECK_EXPECT into
actual states, so it would be like
CHECK_EXPECT(OpenStateChange_MediaChanging),
CHECK_EXPECT(PlatStateChange_Waiting) and alike.
> @@ -87,22 +232,70 @@ static void test_wmp(void)
>
> filename = SysAllocString(load_resource(mp3file));
>
> + SET_EXPECT(OPENSTATE_CHANGE);
> + SET_EXPECT(PLAYSTATE_CHANGE);
> hres = IWMPPlayer4_put_URL(player4, filename);
> ok(hres == S_OK, "IWMPPlayer4_put_URL failed: %08x\n", hres);
> + CHECK_CALLED(OPENSTATE_CHANGE);
> + CHECK_CALLED(PLAYSTATE_CHANGE);
>
> + SET_EXPECT(PLAYSTATE_CHANGE);
> + SET_EXPECT(OPENSTATE_CHANGE);
> hres = IWMPControls_play(controls);
> +
> ok(hres == S_OK, "IWMPControls_play failed: %08x\n", hres);
> + {
> + MSG msg;
> + DWORD start_time = GetTickCount();
> + DWORD dwTimeout = 5000;
> + HANDLE handles[1];
> + handles[0] = playing_event;
> + do {
> + DWORD now = GetTickCount();
> + res = MsgWaitForMultipleObjectsEx(1, handles, start_time + dwTimeout - now,
> + QS_ALLINPUT ,MWMO_ALERTABLE | MWMO_INPUTAVAILABLE);
> + if (res == WAIT_OBJECT_0 + 1) {
> + GetMessageW(&msg, 0, 0, 0);
> + if (winetest_debug > 1)
> + trace("Dispatching %d\n", msg.message);
> + TranslateMessage(&msg);
> + DispatchMessageW(&msg);
> + }
> + }
> + while (res == WAIT_OBJECT_0 + 1);
> + ok(res == WAIT_OBJECT_0 || broken(res == WAIT_TIMEOUT), "Timed out while waiting for media to become ready\n");
> + }
> + if (res == WAIT_TIMEOUT) {
> + /* This happens on Vista Ultimate 64 vms
> + * I have been unable to find out source of this behaviour */
> + win_skip("Failed to transition media to playing state.\n");
> + goto playback_skip;
> + }
> + CHECK_CALLED(OPENSTATE_CHANGE);
> + CHECK_CALLED(PLAYSTATE_CHANGE);
>
> + SET_EXPECT(OPENSTATE_CHANGE); todo_wine ok(FALSE || broken(TRUE), "WMP in wine changes open state when stopping\n");
This looks bad. If you change tests as I mentioned above, you can just
add todo_wine where appropriate.
> + SET_EXPECT(PLAYSTATE_CHANGE);
> hres = IWMPControls_stop(controls);
> ok(hres == S_OK, "IWMPControls_stop failed: %08x\n", hres);
> + CHECK_CALLED(PLAYSTATE_CHANGE);
>
> /* Already Stopped */
> hres = IWMPControls_stop(controls);
> ok(hres == NS_S_WMPCORE_COMMAND_NOT_AVAILABLE, "IWMPControls_stop is available: %08x\n", hres);
>
> + SET_EXPECT(OPENSTATE_CHANGE);
> + SET_EXPECT(PLAYSTATE_CHANGE);
> hres = IWMPControls_play(controls);
> ok(hres == S_OK, "IWMPControls_play failed: %08x\n", hres);
> + CHECK_CALLED_OR_BROKEN(OPENSTATE_CHANGE);
> + CHECK_CALLED_OR_BROKEN(PLAYSTATE_CHANGE);
>
> +playback_skip:
> + hres = IConnectionPoint_Unadvise(point, dw);
> + ok(hres == S_OK, "Unadvise failed: %08x\n", hres);
> +
> + IConnectionPoint_Release(point);
> IWMPControls_Release(controls);
> IWMPPlayer4_Release(player4);
> IOleObject_Release(oleobj);
> @@ -114,7 +307,10 @@ START_TEST(media)
> {
> CoInitialize(NULL);
>
> + playing_event = CreateEventW(NULL, FALSE, FALSE, NULL);
> test_wmp();
>
> + CloseHandle(playing_event);
> +
> CoUninitialize();
> }
> diff --git a/dlls/wmp/wmp_main.c b/dlls/wmp/wmp_main.c
> index 29b096f7fd..9a33b2762b 100644
> --- a/dlls/wmp/wmp_main.c
> +++ b/dlls/wmp/wmp_main.c
> @@ -25,6 +25,7 @@
> WINE_DEFAULT_DEBUG_CHANNEL(wmp);
>
> HINSTANCE wmp_instance;
> +DEFINE_GUID(GUID_NULL,0,0,0,0,0,0,0,0,0,0,0);
>
> static HRESULT WINAPI ClassFactory_QueryInterface(IClassFactory *iface, REFIID riid, void **ppv)
> {
> diff --git a/dlls/wmp/wmp_private.h b/dlls/wmp/wmp_private.h
> index 05ad5889e2..9e84d56ea8 100644
> --- a/dlls/wmp/wmp_private.h
> +++ b/dlls/wmp/wmp_private.h
> @@ -83,6 +83,7 @@ WMPMedia *unsafe_impl_from_IWMPMedia(IWMPMedia *iface) DECLSPEC_HIDDEN;
> HRESULT create_media_from_url(BSTR url, IWMPMedia **ppMedia) DECLSPEC_HIDDEN;
> void ConnectionPointContainer_Init(WindowsMediaPlayer *wmp) DECLSPEC_HIDDEN;
> void ConnectionPointContainer_Destroy(WindowsMediaPlayer *wmp) DECLSPEC_HIDDEN;
> +void call_sink(ConnectionPoint *This, DISPID dispid, DISPPARAMS *dispparams) DECLSPEC_HIDDEN;
>
> HRESULT WINAPI WMPFactory_CreateInstance(IClassFactory*,IUnknown*,REFIID,void**) DECLSPEC_HIDDEN;
>
> diff --git a/include/wmpids.h b/include/wmpids.h
> new file mode 100644
> index 0000000000..2792f850fa
> --- /dev/null
> +++ b/include/wmpids.h
> @@ -0,0 +1,63 @@
> +/*
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
> + */
> +
> +
> +/* play state */
> +#define WMP_PLAY_STATE_UNDEFINED 0
> +#define WMP_PLAY_STATE_STOPPED 1
> +#define WMP_PLAY_STATE_PAUSED 2
> +#define WMP_PLAY_STATE_PLAYING 3
> +#define WMP_PLAY_STATE_SCAN_FORWARD 4
> +#define WMP_PLAY_STATE_SCAN_REVERSE 5
> +#define WMP_PLAY_STATE_BUFFERING 6
> +#define WMP_PLAY_STATE_WAITING 7
> +#define WMP_PLAY_STATE_MEDIA_ENDED 8
> +#define WMP_PLAY_STATE_TRANSITIONING 9
> +#define WMP_PLAY_STATE_READY 10
> +#define WMP_PLAY_STATE_RECONNECTING 11
> +
> +/* open state */
> +#define WMP_OPEN_STATE_UNDEFINED 0
> +#define WMP_OPEN_STATE_PLAYLIST_CHANGING 1
> +#define WMP_OPEN_STATE_PLAYLIST_LOCATING 2
> +#define WMP_OPEN_STATE_PLAYLIST_CONNECTING 3
> +#define WMP_OPEN_STATE_PLAYLIST_LOADING 4
> +#define WMP_OPEN_STATE_PLAYLIST_OPENING 5
> +#define WMP_OPEN_STATE_PLAYLIST_OPEN_NO_MEDIA 6
> +#define WMP_OPEN_STATE_PLAYLIST_CHANGED 7
> +#define WMP_OPEN_STATE_MEDIA_CHANGING 8
> +#define WMP_OPEN_STATE_MEDIA_LOCATING 9
> +#define WMP_OPEN_STATE_MEDIA_CONNECTING 10
> +#define WMP_OPEN_STATE_MEDIA_LOADING 11
> +#define WMP_OPEN_STATE_MEDIA_OPENING 12
> +#define WMP_OPEN_STATE_MEDIA_OPEN 13
> +#define WMP_OPEN_STATE_BEGIN_CODEC_ACQUISITION 14
> +#define WMP_OPEN_STATE_END_CODEC_ACQUISITION 15
> +#define WMP_OPEN_STATE_BEGIN_LICENSE_ACQUISITION 16
> +#define WMP_OPEN_STATE_END_LICENSE_ACQUISITION 17
> +#define WMP_OPEN_STATE_BEGIN_INDIVIDUALIZATION 18
> +#define WMP_OPEN_STATE_END_INDIVIDUALIZATION 19
> +#define WMP_OPEN_STATE_MEDIA_WAITING 20
> +#define WMP_OPEN_STATE_OPENING_UNKNOWN_URL 21
Where do those come from? Shouldn't you use WMPOpenState and
WMPPlayState instead?
Thanks,
Jacek
More information about the wine-devel
mailing list