[PATCH 0/16] MR4046: windows.media.mediacontrol: Implement some IMusicDisplayProperties and IMusicDisplayProperties2 properties.

[Mohamad Al-Jaf (＠maljaf)]

Called by Roon.

-- 
This merge request has too many patches to be relayed via email.
Please visit the URL below to see the contents of the merge request.
https://gitlab.winehq.org/wine/wine/-/merge_requests/4046

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.mediacontrol/tests/mediacontrol.c:
> hr = ISystemMediaTransportControls_get_DisplayUpdater( media_control_statics, &display_updater );
>      ok( hr == S_OK, "got hr %#lx.\n", hr );
>
> +    hr = ISystemMediaTransportControlsDisplayUpdater_put_Type( display_updater, -1 );
> +    todo_wine ok( hr == E_INVALIDARG, "got hr %#lx.\n", hr );
> +    hr = ISystemMediaTransportControlsDisplayUpdater_put_Type( display_updater, 4 );
> +    todo_wine ok( hr == E_INVALIDARG, "got hr %#lx.\n", hr );
> +    hr = ISystemMediaTransportControlsDisplayUpdater_put_Type( display_updater, 1 );
> +    todo_wine ok( hr == S_OK, "got hr %#lx.\n", hr );
> +
> +    playback_type = -1;
> +    hr = ISystemMediaTransportControlsDisplayUpdater_get_Type( display_updater, &playback_type );
> +    todo_wine ok( hr == S_OK, "got hr %#lx.\n", hr );
> +    todo_wine ok( playback_type == 1, "got playback_type %d.\n", playback_type );

Would be better to use MediaPlaybackType values here, except for the invalid ones.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4046#note_48269

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.mediacontrol/display.c:
> static HRESULT WINAPI music_properties_get_Title( IMusicDisplayProperties *iface, HSTRING *value )
>  {
> -    FIXME( "iface %p, value %p stub!\n", iface, value );
> -    return E_NOTIMPL;
> +    struct music_properties *impl = impl_from_IMusicDisplayProperties( iface );
> +    TRACE( "iface %p, value %p\n", iface, value );
> +    return WindowsDuplicateString( impl->title, value );
>  }
>
> static HRESULT WINAPI music_properties_put_Title( IMusicDisplayProperties *iface, HSTRING value )
>  {
> -    FIXME( "iface %p, value %s stub!\n", iface, debugstr_hstring(value) );
> -    return E_NOTIMPL;
> +    struct music_properties *impl = impl_from_IMusicDisplayProperties( iface );
> +    TRACE( "iface %p, value %p\n", iface, value );
> +    return WindowsDuplicateString( value, &impl->title );
> You need to release impl->title first if it was previously set.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4046#note_48270

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.mediacontrol/display.c:
> static HRESULT WINAPI music_properties_get_Artist( IMusicDisplayProperties *iface, HSTRING *value )
>  {
> -    FIXME( "iface %p, value %p stub!\n", iface, value );
> -    return E_NOTIMPL;
> +    struct music_properties *impl = impl_from_IMusicDisplayProperties( iface );
> +    TRACE( "iface %p, value %p\n", iface, value );
> +    return WindowsDuplicateString( impl->artist, value );
>  }
>
> static HRESULT WINAPI music_properties_put_Artist( IMusicDisplayProperties *iface, HSTRING value )
>  {
> -    FIXME( "iface %p, value %s stub!\n", iface, debugstr_hstring(value) );
> -    return E_NOTIMPL;
> +    struct music_properties *impl = impl_from_IMusicDisplayProperties( iface );
> +    TRACE( "iface %p, value %p\n", iface, value );
> +    return WindowsDuplicateString( value, &impl->artist );
> Same here, you need to call `WindowsDeleteString` first.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4046#note_48271

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.mediacontrol/display.c:
> static HRESULT STDMETHODCALLTYPE music_properties2_get_AlbumTitle( IMusicDisplayProperties2 *iface, HSTRING *value )
>  {
> -    FIXME( "iface %p, value %p stub\n", iface, value );
> -    return E_NOTIMPL;
> +    struct music_properties *impl = impl_from_IMusicDisplayProperties2( iface );
> +    TRACE( "iface %p, value %p\n", iface, value );
> +    return WindowsDuplicateString( impl->album_title, value );
>  }
>
> static HRESULT STDMETHODCALLTYPE music_properties2_put_AlbumTitle( IMusicDisplayProperties2 *iface, HSTRING value )
>  {
> -    FIXME( "iface %p, value %s stub\n", iface, debugstr_hstring( value ) );
> -    return E_NOTIMPL;
> +    struct music_properties *impl = impl_from_IMusicDisplayProperties2( iface );
> +    TRACE( "iface %p, value %p\n", iface, value );
> +    return WindowsDuplicateString( value, &impl->album_title );
> Ditto.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4046#note_48272

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.mediacontrol/tests/mediacontrol.c:
> ok( ref == 1, "got ref %ld.\n", ref );
>      ref = ISystemMediaTransportControlsDisplayUpdater_Release( display_updater );
>      ok( ref == 1, "got ref %ld.\n", ref );
> -    ref = ISystemMediaTransportControls_Release( media_control_statics );
> -    ok( ref == 1 || broken(ref == 3) /* Win10 1507 */ || broken(ref == 2) /* Win10 1607 */, "got ref %ld.\n", ref );
> +    ISystemMediaTransportControls_Release( media_control_statics );

Maybe that could be first in the MR, so that it gets fixed as soon as possible.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4046#note_48273

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.mediacontrol/display.c:
> +    display_updater_Update,
> +};
> +
> +HRESULT media_control_create_display_updater( ISystemMediaTransportControls *iface, ISystemMediaTransportControlsDisplayUpdater **value )
> +{
> +    struct display_updater *impl;
> +
> +    if (!(impl = calloc( 1, sizeof(*impl) ))) return E_OUTOFMEMORY;
> +
> +    impl->ISystemMediaTransportControlsDisplayUpdater_iface.lpVtbl = &display_updater_vtbl;
> +    impl->ref = 2;
> +
> +    *value = &impl->ISystemMediaTransportControlsDisplayUpdater_iface;
> +    TRACE( "created ISystemMediaTransportControlsDisplayUpdater %p.\n", *value );
> +    return S_OK;
> +}
> I'm not sure it deserves a dedicated file, as it seems to be pretty much related to the `SystemMediaTransportControls` class, and the constructor could simply be inlined into `ISystemMediaTransportControls_get_DisplayUpdater` the same way you later do for `music_properties` and other subclases.

If you still prefer to keep it separate, I would name the file `display_updater.c`, and the constructor `display_updater_create`, and the iface parameter seems to be unused and should be removed.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4046#note_48274