[PATCH v4 0/4] MR5674: quartz: AVI decompressor fixes.
-- v4: quartz: Hold the streaming lock while calling ICDecompressEnd. quartz: Use the correct stride when calculating image size in AVIDec. quartz: Get output format from source, not sink in AVIDec. quartz/tests: Use unaligned width in AVIDec tests to expose incorrect stride calculation. https://gitlab.winehq.org/wine/wine/-/merge_requests/5674
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/quartz/tests/avidec.c | 43 +++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/dlls/quartz/tests/avidec.c b/dlls/quartz/tests/avidec.c index f28dfee7402..aa3aada3acc 100644 --- a/dlls/quartz/tests/avidec.c +++ b/dlls/quartz/tests/avidec.c @@ -85,10 +85,10 @@ static LRESULT CALLBACK vfw_driver_proc(DWORD_PTR id, HDRVR driver, UINT msg, out->biSize = sizeof(BITMAPINFOHEADER); out->biCompression = mmioFOURCC('I','4','2','0'); - out->biWidth = 32; + out->biWidth = 29; out->biHeight = 24; out->biBitCount = 12; - out->biSizeImage = 32 * 24 * 12 / 8; + out->biSizeImage = 24 * (((29 * 12 + 31) / 8) & ~3); return ICERR_OK; } @@ -159,7 +159,7 @@ static LRESULT CALLBACK vfw_driver_proc(DWORD_PTR id, HDRVR driver, UINT msg, for (i = 0; i < 200; ++i) expect[i] = i; ok(!memcmp(params->lpInput, expect, 200), "Data didn't match.\n"); - for (i = 0; i < 32 * 24 * 12 / 8; ++i) + for (i = 0; i < 24 * (((29 * 12 + 31) / 8) & ~3); ++i) output[i] = 111 - i; return ICERR_OK; @@ -876,7 +876,7 @@ static HRESULT testsink_connect(struct strmbase_sink *iface, IPin *peer, const A static HRESULT WINAPI testsink_Receive(struct strmbase_sink *iface, IMediaSample *sample) { struct testfilter *filter = impl_from_strmbase_filter(iface->pin.filter); - BYTE *data, expect[32 * 24 * 12 / 8]; + BYTE *data, expect[24 * (((29 * 12 + 31) / 8) & ~3)]; REFERENCE_TIME start, stop; LONG size, i; HRESULT hr; @@ -884,9 +884,9 @@ static HRESULT WINAPI testsink_Receive(struct strmbase_sink *iface, IMediaSample ++filter->got_sample; size = IMediaSample_GetSize(sample); - ok(size == 32 * 24 * 12 / 8, "Got size %lu.\n", size); + ok(size == 24 * (((29 * 12 + 31) / 8) & ~3), "Got size %lu.\n", size); size = IMediaSample_GetActualDataLength(sample); - ok(size == 32 * 24 * 12 / 8, "Got valid size %lu.\n", size); + ok(size == 24 * (((29 * 12 + 31) / 8) & ~3), "Got valid size %lu.\n", size); hr = IMediaSample_GetPointer(sample, &data); ok(hr == S_OK, "Got hr %#lx.\n", hr); @@ -1221,7 +1221,7 @@ static void test_connect_pin(void) { .bmiHeader.biSize = sizeof(BITMAPINFOHEADER), .bmiHeader.biCompression = test_handler, - .bmiHeader.biWidth = 32, + .bmiHeader.biWidth = 29, .bmiHeader.biHeight = 24, .bmiHeader.biBitCount = 16, }; @@ -1313,28 +1313,29 @@ static void test_connect_pin(void) const GUID *subtype; DWORD compression; WORD bpp; + BOOL todo; } expect[9] = { - {&MEDIASUBTYPE_CLJR, mmioFOURCC('C','L','J','R'), 8}, - {&MEDIASUBTYPE_UYVY, mmioFOURCC('U','Y','V','Y'), 16}, - {&MEDIASUBTYPE_YUY2, mmioFOURCC('Y','U','Y','2'), 16}, - {&MEDIASUBTYPE_RGB32, BI_RGB, 32}, - {&MEDIASUBTYPE_RGB24, BI_RGB, 24}, - {&MEDIASUBTYPE_RGB565, BI_BITFIELDS, 16}, - {&MEDIASUBTYPE_RGB555, BI_RGB, 16}, - {&MEDIASUBTYPE_RGB8, BI_RGB, 8}, - {&MEDIASUBTYPE_I420, mmioFOURCC('I','4','2','0'), 12}, + {&MEDIASUBTYPE_CLJR, mmioFOURCC('C','L','J','R'), 8, TRUE}, + {&MEDIASUBTYPE_UYVY, mmioFOURCC('U','Y','V','Y'), 16, TRUE}, + {&MEDIASUBTYPE_YUY2, mmioFOURCC('Y','U','Y','2'), 16, TRUE}, + {&MEDIASUBTYPE_RGB32, BI_RGB, 32, FALSE}, + {&MEDIASUBTYPE_RGB24, BI_RGB, 24, TRUE}, + {&MEDIASUBTYPE_RGB565, BI_BITFIELDS, 16, TRUE}, + {&MEDIASUBTYPE_RGB555, BI_RGB, 16, TRUE}, + {&MEDIASUBTYPE_RGB8, BI_RGB, 8, TRUE}, + {&MEDIASUBTYPE_I420, mmioFOURCC('I','4','2','0'), 12, FALSE}, }; VIDEOINFOHEADER expect_format = { .bmiHeader.biSize = sizeof(BITMAPINFOHEADER), - .bmiHeader.biWidth = 32, + .bmiHeader.biWidth = 29, .bmiHeader.biHeight = 24, .bmiHeader.biBitCount = expect[i].bpp, .bmiHeader.biCompression = expect[i].compression, - .bmiHeader.biSizeImage = 32 * 24 * expect[i].bpp / 8, + .bmiHeader.biSizeImage = 24 * (((29 * expect[i].bpp + 31) / 8) & ~3), }; AM_MEDIA_TYPE expect_mt = @@ -1342,7 +1343,7 @@ static void test_connect_pin(void) .majortype = MEDIATYPE_Video, .subtype = *expect[i].subtype, .bFixedSizeSamples = TRUE, - .lSampleSize = 32 * 24 * expect[i].bpp / 8, + .lSampleSize = 24 * (((29 * expect[i].bpp + 31) / 8) & ~3), .formattype = FORMAT_VideoInfo, .cbFormat = sizeof(VIDEOINFOHEADER), .pbFormat = (BYTE *)&expect_format, @@ -1350,9 +1351,9 @@ static void test_connect_pin(void) hr = IEnumMediaTypes_Next(enummt, 1, &pmt, NULL); ok(hr == S_OK, "Got hr %#lx.\n", hr); - ok(!memcmp(pmt, &expect_mt, offsetof(AM_MEDIA_TYPE, cbFormat)), + todo_wine_if(expect[i].todo) ok(!memcmp(pmt, &expect_mt, offsetof(AM_MEDIA_TYPE, cbFormat)), "%u: Media types didn't match.\n", i); - ok(!memcmp(pmt->pbFormat, &expect_format, sizeof(VIDEOINFOHEADER)), + todo_wine_if(expect[i].todo) ok(!memcmp(pmt->pbFormat, &expect_format, sizeof(VIDEOINFOHEADER)), "%u: Format blocks didn't match.\n", i); if (i == 5) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5674
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/quartz/avidec.c | 2 +- dlls/quartz/tests/avidec.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/quartz/avidec.c b/dlls/quartz/avidec.c index 8cc729748c1..f020d618fdb 100644 --- a/dlls/quartz/avidec.c +++ b/dlls/quartz/avidec.c @@ -552,7 +552,7 @@ static HRESULT avi_decompressor_init_stream(struct strmbase_filter *iface) filter->late = -1; - source_format = (VIDEOINFOHEADER *)filter->sink.pin.mt.pbFormat; + source_format = (VIDEOINFOHEADER *)filter->source.pin.mt.pbFormat; if ((res = ICDecompressBegin(filter->hvid, filter->pBihIn, &source_format->bmiHeader))) { ERR("ICDecompressBegin() failed, error %Id.\n", res); diff --git a/dlls/quartz/tests/avidec.c b/dlls/quartz/tests/avidec.c index aa3aada3acc..6cb67563bcb 100644 --- a/dlls/quartz/tests/avidec.c +++ b/dlls/quartz/tests/avidec.c @@ -119,7 +119,7 @@ static LRESULT CALLBACK vfw_driver_proc(DWORD_PTR id, HDRVR driver, UINT msg, todo_wine_if (in->biSizeImage != sink_bitmap_info.biSizeImage) ok(!memcmp(in, &sink_bitmap_info, sizeof(BITMAPINFOHEADER)), "Input types didn't match.\n"); - todo_wine ok(!memcmp(out, &source_bitmap_info, sizeof(BITMAPINFOHEADER)), + ok(!memcmp(out, &source_bitmap_info, sizeof(BITMAPINFOHEADER)), "Output types didn't match.\n"); ok(!in_begin, "Got multiple ICM_DECOMPRESS_BEGIN messages.\n"); in_begin = 1; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5674
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/quartz/avidec.c | 4 ++-- dlls/quartz/tests/avidec.c | 23 +++++++++++------------ 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/dlls/quartz/avidec.c b/dlls/quartz/avidec.c index f020d618fdb..d7f97e0380c 100644 --- a/dlls/quartz/avidec.c +++ b/dlls/quartz/avidec.c @@ -361,8 +361,8 @@ static HRESULT avi_decompressor_source_get_media_type(struct strmbase_pin *iface format->bmiHeader.biPlanes = sink_format->bmiHeader.biPlanes; format->bmiHeader.biBitCount = formats[index].bpp; format->bmiHeader.biCompression = formats[index].compression; - format->bmiHeader.biSizeImage = format->bmiHeader.biWidth - * format->bmiHeader.biHeight * formats[index].bpp / 8; + format->bmiHeader.biSizeImage = format->bmiHeader.biHeight * (((format->bmiHeader.biWidth + * formats[index].bpp + 31) / 8) & ~3); if (IsEqualGUID(formats[index].subtype, &MEDIASUBTYPE_RGB565)) { diff --git a/dlls/quartz/tests/avidec.c b/dlls/quartz/tests/avidec.c index 6cb67563bcb..ac3a9abf2b1 100644 --- a/dlls/quartz/tests/avidec.c +++ b/dlls/quartz/tests/avidec.c @@ -1313,19 +1313,18 @@ static void test_connect_pin(void) const GUID *subtype; DWORD compression; WORD bpp; - BOOL todo; } expect[9] = { - {&MEDIASUBTYPE_CLJR, mmioFOURCC('C','L','J','R'), 8, TRUE}, - {&MEDIASUBTYPE_UYVY, mmioFOURCC('U','Y','V','Y'), 16, TRUE}, - {&MEDIASUBTYPE_YUY2, mmioFOURCC('Y','U','Y','2'), 16, TRUE}, - {&MEDIASUBTYPE_RGB32, BI_RGB, 32, FALSE}, - {&MEDIASUBTYPE_RGB24, BI_RGB, 24, TRUE}, - {&MEDIASUBTYPE_RGB565, BI_BITFIELDS, 16, TRUE}, - {&MEDIASUBTYPE_RGB555, BI_RGB, 16, TRUE}, - {&MEDIASUBTYPE_RGB8, BI_RGB, 8, TRUE}, - {&MEDIASUBTYPE_I420, mmioFOURCC('I','4','2','0'), 12, FALSE}, + {&MEDIASUBTYPE_CLJR, mmioFOURCC('C','L','J','R'), 8}, + {&MEDIASUBTYPE_UYVY, mmioFOURCC('U','Y','V','Y'), 16}, + {&MEDIASUBTYPE_YUY2, mmioFOURCC('Y','U','Y','2'), 16}, + {&MEDIASUBTYPE_RGB32, BI_RGB, 32}, + {&MEDIASUBTYPE_RGB24, BI_RGB, 24}, + {&MEDIASUBTYPE_RGB565, BI_BITFIELDS, 16}, + {&MEDIASUBTYPE_RGB555, BI_RGB, 16}, + {&MEDIASUBTYPE_RGB8, BI_RGB, 8}, + {&MEDIASUBTYPE_I420, mmioFOURCC('I','4','2','0'), 12}, }; VIDEOINFOHEADER expect_format = @@ -1351,9 +1350,9 @@ static void test_connect_pin(void) hr = IEnumMediaTypes_Next(enummt, 1, &pmt, NULL); ok(hr == S_OK, "Got hr %#lx.\n", hr); - todo_wine_if(expect[i].todo) ok(!memcmp(pmt, &expect_mt, offsetof(AM_MEDIA_TYPE, cbFormat)), + ok(!memcmp(pmt, &expect_mt, offsetof(AM_MEDIA_TYPE, cbFormat)), "%u: Media types didn't match.\n", i); - todo_wine_if(expect[i].todo) ok(!memcmp(pmt->pbFormat, &expect_format, sizeof(VIDEOINFOHEADER)), + ok(!memcmp(pmt->pbFormat, &expect_format, sizeof(VIDEOINFOHEADER)), "%u: Format blocks didn't match.\n", i); if (i == 5) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5674
From: Anton Baskanov <baskanov(a)gmail.com> Otherwise, the streaming thread might try to access the decompressor while it's being destroyed. --- dlls/quartz/avidec.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/dlls/quartz/avidec.c b/dlls/quartz/avidec.c index d7f97e0380c..f494efdf1e8 100644 --- a/dlls/quartz/avidec.c +++ b/dlls/quartz/avidec.c @@ -573,10 +573,16 @@ static HRESULT avi_decompressor_cleanup_stream(struct strmbase_filter *iface) if (!filter->source.pin.peer) return S_OK; - if (filter->hvid && (res = ICDecompressEnd(filter->hvid))) + if (filter->hvid) { - ERR("ICDecompressEnd() failed, error %Id.\n", res); - return E_FAIL; + EnterCriticalSection(&filter->filter.stream_cs); + res = ICDecompressEnd(filter->hvid); + LeaveCriticalSection(&filter->filter.stream_cs); + if (res) + { + ERR("ICDecompressEnd() failed, error %Id.\n", res); + return E_FAIL; + } } IMemAllocator_Decommit(filter->source.pAllocator); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5674
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=145752 Your paranoid android. === debian11b (build log) === 009c:err:winediag:gnutls_process_attach failed to load libgnutls, no support for encryption
v4: - Fix indentation in the last commit. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5674#note_71101
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5674
participants (4)
-
Anton Baskanov -
Anton Baskanov (＠baskanov)
-
Elizabeth Figura (＠zfigura)
-
Marvin