[PATCH 0/3] MR4609: winegstreamer: Some tiny fixes

[Alfred Agrell (＠Alcaro)]

Just some tiny fixes I found while creating !4449. No functional changes intended.

Wine's code review process, and commit history, is a lot more comprehensive than anything else I've seen. This means I can't just write code, submit patch, and call it a day; but it also means the codebase is clean and easy to work with, so fixing bugs and missing pieces is relatively straightforward. While this does increase the total time needed on my side (especially splitting commits), it's easy (though somewhat boring) work, so in total, it's a win. And splitting makes it easier for the maintainers - you have more to do than me, so if it saves your time at the expense of mine, I have no right to complain.

Thank you for your patience with my prior MRs. 8.2/10 would contribute again (I've seen a few other games with broken videos that I suspect have still-unfixed causes)

[PATCH 1/3] winegstreamer: Don't use VIDEOINFO for non-RGB data.

[Alfred Agrell]

From: Alfred Agrell

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

diff --git a/dlls/winegstreamer/quartz_parser.c b/dlls/winegstreamer/quartz_parser.c
index fdcea1017a9..44e13d05c2a 100644
--- a/dlls/winegstreamer/quartz_parser.c
+++ b/dlls/winegstreamer/quartz_parser.c
@@ -594,7 +594,7 @@ static bool amt_from_wg_format_video(AM_MEDIA_TYPE *mt, const struct wg_format *

static bool amt_from_wg_format_video_cinepak(AM_MEDIA_TYPE *mt, const struct wg_format *format)
 {
-    VIDEOINFO *video_format;
+    VIDEOINFOHEADER *video_format;
     uint32_t frame_time;

if (!(video_format = CoTaskMemAlloc(sizeof(*video_format))))
@@ -605,7 +605,7 @@ static bool amt_from_wg_format_video_cinepak(AM_MEDIA_TYPE *mt, const struct wg_
     mt->bTemporalCompression = TRUE;
     mt->lSampleSize = 1;
     mt->formattype = FORMAT_VideoInfo;
-    mt->cbFormat = sizeof(VIDEOINFOHEADER);
+    mt->cbFormat = sizeof(*video_format);
     mt->pbFormat = (BYTE *)video_format;

memset(video_format, 0, sizeof(*video_format));

[PATCH 2/3] winegstreamer: Fix a memory leak in stream_props_GetMediaType.

[Alfred Agrell]

From: Alfred Agrell

---
 dlls/winegstreamer/wm_reader.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c
index 4751cc9da2e..b63cdbf3346 100644
--- a/dlls/winegstreamer/wm_reader.c
+++ b/dlls/winegstreamer/wm_reader.c
@@ -565,6 +565,7 @@ static HRESULT WINAPI stream_props_GetMediaType(IWMMediaProps *iface, WM_MEDIA_T
     memcpy(mt, &stream_mt, sizeof(*mt));
     memcpy(mt + 1, stream_mt.pbFormat, stream_mt.cbFormat);
     mt->pbFormat = (BYTE *)(mt + 1);
+    FreeMediaType(&stream_mt);
     return S_OK;
 }

[PATCH 3/3] winegstreamer: Delete duplicate WMT_ON check.

[Alfred Agrell]

From: Alfred Agrell

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

diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c
index b63cdbf3346..ebdf812db9c 100644
--- a/dlls/winegstreamer/wm_reader.c
+++ b/dlls/winegstreamer/wm_reader.c
@@ -2461,7 +2461,7 @@ static HRESULT WINAPI reader_SetStreamsSelected(IWMSyncReader2 *iface,
             TRACE("Disabling stream %u.\n", stream_numbers[i]);
             wg_parser_stream_disable(stream->wg_stream);
         }
-        else if (selections[i] == WMT_ON)
+        else
         {
             if (selections[i] != WMT_ON)
                 FIXME("Ignoring selection %#x for stream %u; treating as enabled.\n",

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/winegstreamer/wm_reader.c:
>      return E_OUTOFMEMORY;
>
>  *size = sizeof(stream_mt) + stream_mt.cbFormat;
>  if (!mt)

Let's return early when !mt is true and call FreeMediaType(&stream_mt) when req_size < *size so that stream_mt is not leaked on these branches as well.

[Alfred Agrell (＠Alcaro)]

On Wed Dec  6 01:41:59 2023 +0000, Zhiyi Zhang wrote:
> Let's return early when !mt is true and call FreeMediaType(&stream_mt)
> when req_size < *size so that stream_mt is not leaked on these branches
> as well.

~~but then the fix isn't tiny anymore~~

Can't return any earlier, need to set *size.

But yes, good catch. I'm not a huge fan of amt_from_wg_format allocating at all, but trying to refactor that is out of scope for this MR.