[PATCH 0/4] MR4906: mf,mfplat: Clang warning fixes.

[PATCH 1/4] mf: Avoid implicit enum to int pointer casts.

[Jacek Caban]

From: Jacek Caban

---
 dlls/mf/topology_loader.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c
index 6b6d39d76d1..672cb88be78 100644
--- a/dlls/mf/topology_loader.c
+++ b/dlls/mf/topology_loader.c
@@ -379,8 +379,8 @@ static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_ME
 {
     IMFMediaTypeHandler *down_handler;
     IMFMediaType *down_type = NULL;
-    MF_CONNECT_METHOD method;
     MF_TOPOLOGY_TYPE type;
+    UINT32 method;
     DWORD flags;
     HRESULT hr;

@@ -458,7 +458,7 @@ static HRESULT topology_branch_foreach_up_types(IMFTopology *topology, MF_CONNEC
 static HRESULT topology_branch_connect(IMFTopology *topology, MF_CONNECT_METHOD method_mask,
         struct topology_branch *branch, BOOL enumerate_source_types)
 {
-    MF_CONNECT_METHOD method;
+    UINT32 method;
     HRESULT hr;

TRACE("topology %p, method_mask %#x, branch %s.\n", topology, method_mask, debugstr_topology_branch(branch));

[PATCH 2/4] mfplat: Avoid implicit cast in IMFAttributes_GetUINT32 call.

[Jacek Caban]

From: Jacek Caban

---
 dlls/mfplat/sample.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/mfplat/sample.c b/dlls/mfplat/sample.c
index a50f81c93d4..4d75924b939 100644
--- a/dlls/mfplat/sample.c
+++ b/dlls/mfplat/sample.c
@@ -1423,7 +1423,7 @@ static HRESULT sample_allocator_initialize(struct sample_allocator *allocator, u
     unsigned int i, value;
     GUID major, subtype;
     UINT64 frame_size;
-    D3D11_USAGE usage;
+    UINT32 usage;
     HRESULT hr;

if (FAILED(hr = IMFMediaType_GetMajorType(media_type, &major)))

[PATCH 3/4] mfplat/tests: Use MF_ATTRIBUTE_TYPE type in IMFMediaType_GetItemType call.

[Jacek Caban]

From: Jacek Caban

---
 dlls/mfplat/tests/mfplat.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c
index 7fab1598181..9964b0957bc 100644
--- a/dlls/mfplat/tests/mfplat.c
+++ b/dlls/mfplat/tests/mfplat.c
@@ -5069,7 +5069,8 @@ static void test_attributes_serialization(void)
 static void test_wrapped_media_type(void)
 {
     IMFMediaType *mediatype, *mediatype2;
-    UINT32 count, type;
+    MF_ATTRIBUTE_TYPE type;
+    UINT32 count;
     HRESULT hr;
     GUID guid;

[PATCH 4/4] mfplat: Avoid implicit enum pointer casts in MFCreateMFVideoFormatFromMFMediaType.

[Jacek Caban]

From: Jacek Caban

---
 dlls/mfplat/mediatype.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/dlls/mfplat/mediatype.c b/dlls/mfplat/mediatype.c
index 2ba967c6719..532272b6d2a 100644
--- a/dlls/mfplat/mediatype.c
+++ b/dlls/mfplat/mediatype.c
@@ -3253,13 +3253,20 @@ HRESULT WINAPI MFCreateMFVideoFormatFromMFMediaType(IMFMediaType *media_type, MF
     media_type_get_ratio(media_type, &MF_MT_FRAME_RATE, &format->videoInfo.FramesPerSecond.Numerator,
             &format->videoInfo.FramesPerSecond.Denominator);

-    IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_CHROMA_SITING, &format->videoInfo.SourceChromaSubsampling);
-    IMFMediaType_GetUINT32(media_type, &MF_MT_INTERLACE_MODE, &format->videoInfo.InterlaceMode);
-    IMFMediaType_GetUINT32(media_type, &MF_MT_TRANSFER_FUNCTION, &format->videoInfo.TransferFunction);
-    IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_PRIMARIES, &format->videoInfo.ColorPrimaries);
-    IMFMediaType_GetUINT32(media_type, &MF_MT_YUV_MATRIX, &format->videoInfo.TransferMatrix);
-    IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_LIGHTING, &format->videoInfo.SourceLighting);
-    IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_NOMINAL_RANGE, &format->videoInfo.NominalRange);
+    if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_CHROMA_SITING, &value)))
+        format->videoInfo.SourceChromaSubsampling = value;
+    if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_INTERLACE_MODE, &value)))
+        format->videoInfo.InterlaceMode = value;
+    if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_TRANSFER_FUNCTION, &value)))
+        format->videoInfo.TransferFunction = value;
+    if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_PRIMARIES, &value)))
+        format->videoInfo.ColorPrimaries = value;
+    if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_YUV_MATRIX, &value)))
+        format->videoInfo.TransferMatrix = value;
+    if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_LIGHTING, &value)))
+        format->videoInfo.SourceLighting = value;
+    if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_NOMINAL_RANGE, &value)))
+        format->videoInfo.NominalRange = value;
     IMFMediaType_GetBlob(media_type, &MF_MT_GEOMETRIC_APERTURE, (UINT8 *)&format->videoInfo.GeometricAperture,
            sizeof(format->videoInfo.GeometricAperture), NULL);
     IMFMediaType_GetBlob(media_type, &MF_MT_MINIMUM_DISPLAY_APERTURE, (UINT8 *)&format->videoInfo.MinimumDisplayAperture,

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/mfplat/mediatype.c:
> - IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_LIGHTING, &format->videoInfo.SourceLighting);
> - IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_NOMINAL_RANGE, &format->videoInfo.NominalRange);
> - if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_CHROMA_SITING, &value)))
> -
>    format->videoInfo.SourceChromaSubsampling = value;
> - if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_INTERLACE_MODE, &value)))
> -
>    format->videoInfo.InterlaceMode = value;
> - if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_TRANSFER_FUNCTION, &value)))
> -
>    format->videoInfo.TransferFunction = value;
> - if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_PRIMARIES, &value)))
> -
>    format->videoInfo.ColorPrimaries = value;
> - if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_YUV_MATRIX, &value)))
> -
>    format->videoInfo.TransferMatrix = value;
> - if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_LIGHTING, &value)))
> -
>    format->videoInfo.SourceLighting = value;
> - if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_VIDEO_NOMINAL_RANGE, &value)))
> -
>    format->videoInfo.NominalRange = value;

@rbernon , what's your opinion on this? It would be nice to have a helper to hide these checks, but I don't see how.

[Rémi Bernon (＠rbernon)]

On Mon Jan 22 15:29:16 2024 +0000, Nikolay Sivov wrote:
> @rbernon , what's your opinion on this? It would be nice to have a
> helper to hide these checks, but I don't see how.

Does clang also warn on enum implicit conversion on assign or just pointer types?

What about some helper like:

```
#define MEDIA_TYPE_GET(m, t, a, v) \
  do { \
    t __val;
    if (SUCCEEDED(IMFMediaType_Get ## t((m), (a), &__val))) \
      *(v) = __val; \
  } while (0)
```

Which could also trace a WARN or ERR at the same time, which could help catching missing attributes if that feels useful.

Could also add a default value argument, and / or some less generic helper with UINT32 hardcoded. That could be used for ULONG too.