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

[Mohamad Al-Jaf (＠maljaf)]

Called by Roon.

--
  v2: windows.media.mediacontrol: Return success in ISystemMediaTransportControlsDisplayUpdater::Update().
      windows.media.mediacontrol: Implement IMusicDisplayProperties2::get/put_AlbumTitle().
      windows.media.mediacontrol/tests: Add IMusicDisplayProperties2::get/put_AlbumTitle() tests.
      windows.media.mediacontrol: Add IMusicDisplayProperties2 stub interface.
      include: Add IMusicDisplayProperties2 interface definition.
      windows.media.mediacontrol: Implement IMusicDisplayProperties::get/put_Artist().
      windows.media.mediacontrol/tests: Add IMusicDisplayProperties::get/put_Artist() tests.
      windows.media.mediacontrol: Implement IMusicDisplayProperties::get/put_Title().
      windows.media.mediacontrol/tests: Add IMusicDisplayProperties::get/put_Title() tests.
      windows.media.mediacontrol: Implement ISystemMediaTransportControlsDisplayUpdater::get_MusicProperties().
      include: Add IMusicDisplayProperties interface definition.
      windows.media.mediacontrol: Implement ISystemMediaTransportControlsDisplayUpdater::get/put_Type().
      windows.media.mediacontrol/tests: Add ISystemMediaTransportControlsDisplayUpdater::get/put_Type() tests.
      windows.media.mediacontrol: Implement ISystemMediaTransportControls::get_DisplayUpdater().
      include: Add ISystemMediaTransportControlsDisplayUpdater interface definition.
      windows.media.mediacontrol/tests: Remove flakey ref count test.
 
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

[Mohamad Al-Jaf (＠maljaf)]

On Thu Oct 12 06:17:01 2023 +0000, Mohamad Al-Jaf wrote:
> changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/4046/diffs?diff_id=75689&start_sha=f3c6276795504e481dd0508a03ca44afd3813e9b#72635bff85b2bd07432306b4de6e43d9507bbaa1_421_0)
> Yeah, it is related to the same class. I added it to organize the music/video/image properties in one place, but it's not necessary.

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

[Mohamad Al-Jaf (＠maljaf)]

On Thu Oct 12 06:16:59 2023 +0000, Mohamad Al-Jaf wrote:
> changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/4046/diffs?diff_id=75689&start_sha=f3c6276795504e481dd0508a03ca44afd3813e9b#72635bff85b2bd07432306b4de6e43d9507bbaa1_123_0)
> I thought `WindowsDuplicateString` would handle the memory management for this.

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

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.mediacontrol/main.c:
> +{
> +    FIXME( "iface %p, value %p stub!\n", iface, value );
> +    return E_NOTIMPL;
> +}
> +
> +static HRESULT WINAPI display_updater_get_MusicProperties( ISystemMediaTransportControlsDisplayUpdater *iface, IMusicDisplayProperties **value )
> +{
> +    struct music_properties *impl;
> +
> +    TRACE( "iface %p, value %p\n", iface, value );
> +
> +    if (!(impl = calloc( 1, sizeof(*impl) ))) return E_OUTOFMEMORY;
> +
> +    impl->IMusicDisplayProperties_iface.lpVtbl = &music_properties_vtbl;
> +    impl->IMusicDisplayProperties2_iface.lpVtbl = &music_properties2_vtbl;
> +    impl->ref = 2;
> I missed it previously but I don't think you should initialize ref count to 2. If the tests show that kind of refcount (although I don't think testing this is very useful), then it's very likely that the objects shouldn't be created on demand but rather be created upfront and owned by their parent until it is destroyed.

It is probably also fine to still create them on-demand, but in all cases initialize ref to 1, so that there's no leak when an application release their only reference.

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

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/windows.media.mediacontrol/main.c:
> static HRESULT WINAPI media_control_get_DisplayUpdater( ISystemMediaTransportControls *iface, ISystemMediaTransportControlsDisplayUpdater **value )
>  {
> +    struct display_updater *impl;
> +
>      FIXME( "iface %p, value %p semi-stub!\n", iface, value );
> -    return media_control_create_display_updater( iface, value );
> +
> +    if (!(impl = calloc( 1, sizeof(*impl) ))) return E_OUTOFMEMORY;
> +
> +    impl->ISystemMediaTransportControlsDisplayUpdater_iface.lpVtbl = &display_updater_vtbl;
> +    impl->ref = 2;

Same here.

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