[PATCH v3 0/1] MR8937: windows.media.playback.backgroundmediaplayer: Implement IBackgroundMediaPlayerStatics::get_Current().
-- v3: windows.media.playback.backgroundmediaplayer: Implement IBackgroundMediaPlayerStatics::get_Current(). https://gitlab.winehq.org/wine/wine/-/merge_requests/8937
From: Mohamad Al-Jaf <mohamadaljaf(a)gmail.com> --- .../main.c | 62 ++++++++++++++++++- .../private.h | 1 + .../tests/playback.c | 10 +++ 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/dlls/windows.media.playback.backgroundmediaplayer/main.c b/dlls/windows.media.playback.backgroundmediaplayer/main.c index 66382520957..3bf534ab867 100644 --- a/dlls/windows.media.playback.backgroundmediaplayer/main.c +++ b/dlls/windows.media.playback.backgroundmediaplayer/main.c @@ -24,11 +24,22 @@ WINE_DEFAULT_DEBUG_CHANNEL(playback); +static CRITICAL_SECTION media_player_cs; +static CRITICAL_SECTION_DEBUG media_player_cs_debug = +{ + 0, 0, &media_player_cs, + { &media_player_cs_debug.ProcessLocksList, &media_player_cs_debug.ProcessLocksList }, + 0, 0, { (DWORD_PTR)(__FILE__ ": media_player_cs") } +}; +static CRITICAL_SECTION media_player_cs = { &media_player_cs_debug, -1, 0, 0, 0, 0 }; + struct background_media_player_statics { IActivationFactory IActivationFactory_iface; IBackgroundMediaPlayerStatics IBackgroundMediaPlayerStatics_iface; LONG ref; + + IMediaPlayer *media_player; }; static inline struct background_media_player_statics *impl_from_IActivationFactory( IActivationFactory *iface ) @@ -76,7 +87,20 @@ static ULONG WINAPI factory_Release( IActivationFactory *iface ) { struct background_media_player_statics *impl = impl_from_IActivationFactory( iface ); ULONG ref = InterlockedDecrement( &impl->ref ); + TRACE( "iface %p decreasing refcount to %lu.\n", iface, ref ); + + if (!ref) + { + EnterCriticalSection( &media_player_cs ); + if (!impl->ref && impl->media_player) + { + IMediaPlayer_Release( impl->media_player ); + impl->media_player = NULL; + } + LeaveCriticalSection( &media_player_cs ); + } + return ref; } @@ -120,10 +144,44 @@ static const struct IActivationFactoryVtbl factory_vtbl = DEFINE_IINSPECTABLE( background_media_player_statics, IBackgroundMediaPlayerStatics, struct background_media_player_statics, IActivationFactory_iface ) +static HRESULT get_media_player( IMediaPlayer **media_player ) +{ + static const WCHAR *media_player_name = L"Windows.Media.Playback.MediaPlayer"; + IInspectable *inspectable = NULL; + HSTRING str = NULL; + HRESULT hr; + + if (FAILED(hr = WindowsCreateString( media_player_name, wcslen( media_player_name ), &str ))) return hr; + if (SUCCEEDED(hr = RoActivateInstance( str, &inspectable ))) + { + hr = IInspectable_QueryInterface( inspectable, &IID_IMediaPlayer, (void **)media_player ); + IInspectable_Release( inspectable ); + } + + WindowsDeleteString( str ); + return hr; +} + static HRESULT WINAPI background_media_player_statics_get_Current( IBackgroundMediaPlayerStatics *iface, IMediaPlayer **player ) { - FIXME( "iface %p, player %p stub!\n", iface, player ); - return E_NOTIMPL; + struct background_media_player_statics *impl = impl_from_IBackgroundMediaPlayerStatics( iface ); + HRESULT hr; + + TRACE( "iface %p, player %p\n", iface, player ); + + EnterCriticalSection( &media_player_cs ); + + if (!impl->media_player && FAILED(hr = get_media_player( &impl->media_player ))) + { + *player = NULL; + LeaveCriticalSection( &media_player_cs ); + return hr; + } + + *player = impl->media_player; + IMediaPlayer_AddRef( *player ); + LeaveCriticalSection( &media_player_cs ); + return S_OK; } static HRESULT WINAPI background_media_player_statics_add_MessageReceivedFromBackground( IBackgroundMediaPlayerStatics *iface, diff --git a/dlls/windows.media.playback.backgroundmediaplayer/private.h b/dlls/windows.media.playback.backgroundmediaplayer/private.h index 63115ff2a6f..345937f80ee 100644 --- a/dlls/windows.media.playback.backgroundmediaplayer/private.h +++ b/dlls/windows.media.playback.backgroundmediaplayer/private.h @@ -28,6 +28,7 @@ #include "winstring.h" #include "activation.h" +#include "roapi.h" #define WIDL_using_Windows_Foundation #define WIDL_using_Windows_Foundation_Collections diff --git a/dlls/windows.media.playback.backgroundmediaplayer/tests/playback.c b/dlls/windows.media.playback.backgroundmediaplayer/tests/playback.c index a99c4080e1b..593355d67f3 100644 --- a/dlls/windows.media.playback.backgroundmediaplayer/tests/playback.c +++ b/dlls/windows.media.playback.backgroundmediaplayer/tests/playback.c @@ -50,6 +50,8 @@ static void test_BackgroundMediaPlayer_Statics(void) static const WCHAR *background_media_player_statics_name = L"Windows.Media.Playback.BackgroundMediaPlayer"; IBackgroundMediaPlayerStatics *background_media_player_statics = (void *)0xdeadbeef; IActivationFactory *factory = (void *)0xdeadbeef; + IMediaPlayer *media_player2 = (void *)0xdeadbeef; + IMediaPlayer *media_player = (void *)0xdeadbeef; HSTRING str; HRESULT hr; LONG ref; @@ -73,6 +75,14 @@ static void test_BackgroundMediaPlayer_Statics(void) hr = IActivationFactory_QueryInterface( factory, &IID_IBackgroundMediaPlayerStatics, (void **)&background_media_player_statics ); ok( hr == S_OK, "got hr %#lx.\n", hr ); + hr = IBackgroundMediaPlayerStatics_get_Current( background_media_player_statics, &media_player ); + ok( hr == S_OK, "got hr %#lx.\n", hr ); + hr = IBackgroundMediaPlayerStatics_get_Current( background_media_player_statics, &media_player2 ); + ok( hr == S_OK, "got hr %#lx.\n", hr ); + ok( media_player == media_player2, "got media_player %p, media_player2 %p.\n", media_player, media_player2 ); + IMediaPlayer_Release( media_player2 ); + IMediaPlayer_Release( media_player ); + ref = IBackgroundMediaPlayerStatics_Release( background_media_player_statics ); ok( ref == 2, "got ref %ld.\n", ref ); ref = IActivationFactory_Release( factory ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8937
On Thu Sep 11 03:53:29 2025 +0000, Rémi Bernon wrote:
```suggestion:-10+0 HRESULT hr; if (FAILED(hr = WindowsCreateString( media_player_name, wcslen( media_player_name ), &str ))) return hr; if (SUCCEEDED(hr = RoActivateInstance( str, &inspectable ))) { hr = IInspectable_QueryInterface( inspectable, &IID_IMediaPlayer, (void **)media_player ); IInspectable_Release( inspectable ); } WindowsDeleteString( str ); return hr; ``` Otherwise you'd call WindowsDeleteString even if WindowsCreateString failed (no big deal but still).
Yeah, that's why I initialized the HSTRING to NULL. Though, this looks better style-wise, thanks. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8937#note_115639
On Thu Sep 11 03:53:33 2025 +0000, Rémi Bernon wrote:
This works because you also guard the factory QueryInterface, but I would say it's not the canonical way to do it. Guarding the factory AddRef shouldn't be necessary, and instead you should probably check the ref count again within the critical section: ```suggestion:-6+0 EnterCriticalSection( &media_player_cs ); if (!impl->ref && impl->media_player) { IMediaPlayer_Release( impl->media_player ); impl->media_player = NULL; } LeaveCriticalSection( &media_player_cs ); ``` I see, thanks.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8937#note_115640
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8937
participants (3)
-
Mohamad Al-Jaf -
Mohamad Al-Jaf (＠maljaf)
-
Rémi Bernon