November 2021 - Wine-Devel - list.winehq.org

[PATCH v2 2/5] winegstreamer: Implement INSSBuffer::GetMaxLength().

by Zebediah Figura

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/winegstreamer/wm_reader.c | 8 ++++++-- dlls/wmvcore/tests/wmvcore.c | 16 ++++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c index 2de0610165d..2c0f89e29a9 100644 --- a/dlls/winegstreamer/wm_reader.c +++ b/dlls/winegstreamer/wm_reader.c @@ -237,8 +237,12 @@ static HRESULT WINAPI buffer_SetLength(INSSBuffer *iface, DWORD size) static HRESULT WINAPI buffer_GetMaxLength(INSSBuffer *iface, DWORD *size) { - FIXME("iface %p, size %p, stub!\n", iface, size); - return E_NOTIMPL; + struct buffer *buffer = impl_from_INSSBuffer(iface); + + TRACE("buffer %p, size %p.\n", buffer, size); + + *size = buffer->size; + return S_OK; } static HRESULT WINAPI buffer_GetBuffer(INSSBuffer *iface, BYTE **data) diff --git a/dlls/wmvcore/tests/wmvcore.c b/dlls/wmvcore/tests/wmvcore.c index 685dd92e27e..0347195ba10 100644 --- a/dlls/wmvcore/tests/wmvcore.c +++ b/dlls/wmvcore/tests/wmvcore.c @@ -591,7 +591,7 @@ static void test_reader_attributes(IWMProfile *profile) static void test_sync_reader_streaming(void) { - DWORD size, flags, output_number, expect_output_number; + DWORD size, capacity, flags, output_number, expect_output_number; const WCHAR *filename = load_resource(L"test.wmv"); WORD stream_numbers[2], stream_number; IWMStreamConfig *config, *config2; @@ -682,6 +682,10 @@ static void test_sync_reader_streaming(void) ok(hr == S_OK, "Got hr %#x.\n", hr); ok(data2 == data, "Data pointers didn't match.\n"); + hr = INSSBuffer_GetMaxLength(sample, &capacity); + ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(size <= capacity, "Size %u exceeds capacity %u.\n", size, capacity); + ref = INSSBuffer_Release(sample); ok(!ref, "Got outstanding refcount %d.\n", ref); @@ -747,6 +751,10 @@ static void test_sync_reader_streaming(void) ok(hr == S_OK, "Got hr %#x.\n", hr); ok(data2 == data, "Data pointers didn't match.\n"); + hr = INSSBuffer_GetMaxLength(sample, &capacity); + ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(size <= capacity, "Size %u exceeds capacity %u.\n", size, capacity); + ref = INSSBuffer_Release(sample); ok(!ref, "Got outstanding refcount %d.\n", ref); } @@ -1193,9 +1201,9 @@ static HRESULT WINAPI callback_OnSample(IWMReaderCallback *iface, DWORD output, QWORD time, QWORD duration, DWORD flags, INSSBuffer *sample, void *context) { struct callback *callback = impl_from_IWMReaderCallback(iface); + DWORD size, capacity; BYTE *data, *data2; HRESULT hr; - DWORD size; if (winetest_debug > 1) trace("%u: %04x: IWMReaderCallback::OnSample(output %u, time %I64u, duration %I64u, flags %#x)\n", @@ -1210,6 +1218,10 @@ static HRESULT WINAPI callback_OnSample(IWMReaderCallback *iface, DWORD output, ok(hr == S_OK, "Got hr %#x.\n", hr); ok(data2 == data, "Data pointers didn't match.\n"); + hr = INSSBuffer_GetMaxLength(sample, &capacity); + ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(size <= capacity, "Size %u exceeds capacity %u.\n", size, capacity); + ok(callback->got_started > 0, "Got %u WMT_STARTED callbacks.\n", callback->got_started); ok(!callback->got_eof, "Got %u WMT_EOF callbacks.\n", callback->got_eof); ++callback->got_sample; -- 2.33.0

Nov. 8, 2021

[PATCH v2 1/5] winegstreamer: Implement INSSBuffer::GetBuffer().

by Zebediah Figura

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/winegstreamer/wm_reader.c | 8 ++++++-- dlls/wmvcore/tests/wmvcore.c | 18 ++++++++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c index 12d1f04c270..2de0610165d 100644 --- a/dlls/winegstreamer/wm_reader.c +++ b/dlls/winegstreamer/wm_reader.c @@ -243,8 +243,12 @@ static HRESULT WINAPI buffer_GetMaxLength(INSSBuffer *iface, DWORD *size) static HRESULT WINAPI buffer_GetBuffer(INSSBuffer *iface, BYTE **data) { - FIXME("iface %p, data %p, stub!\n", iface, data); - return E_NOTIMPL; + struct buffer *buffer = impl_from_INSSBuffer(iface); + + TRACE("buffer %p, data %p.\n", buffer, data); + + *data = buffer->data; + return S_OK; } static HRESULT WINAPI buffer_GetBufferAndLength(INSSBuffer *iface, BYTE **data, DWORD *size) diff --git a/dlls/wmvcore/tests/wmvcore.c b/dlls/wmvcore/tests/wmvcore.c index 0e7c6380c92..685dd92e27e 100644 --- a/dlls/wmvcore/tests/wmvcore.c +++ b/dlls/wmvcore/tests/wmvcore.c @@ -602,9 +602,9 @@ static void test_sync_reader_streaming(void) IWMProfile *profile; QWORD pts, duration; INSSBuffer *sample; + BYTE *data, *data2; HANDLE file; HRESULT hr; - BYTE *data; BOOL ret; file = CreateFileW(filename, GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, 0); @@ -677,6 +677,11 @@ static void test_sync_reader_streaming(void) { hr = INSSBuffer_GetBufferAndLength(sample, &data, &size); ok(hr == S_OK, "Got hr %#x.\n", hr); + + hr = INSSBuffer_GetBuffer(sample, &data2); + ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(data2 == data, "Data pointers didn't match.\n"); + ref = INSSBuffer_Release(sample); ok(!ref, "Got outstanding refcount %d.\n", ref); @@ -737,6 +742,11 @@ static void test_sync_reader_streaming(void) { hr = INSSBuffer_GetBufferAndLength(sample, &data, &size); ok(hr == S_OK, "Got hr %#x.\n", hr); + + hr = INSSBuffer_GetBuffer(sample, &data2); + ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(data2 == data, "Data pointers didn't match.\n"); + ref = INSSBuffer_Release(sample); ok(!ref, "Got outstanding refcount %d.\n", ref); } @@ -1183,9 +1193,9 @@ static HRESULT WINAPI callback_OnSample(IWMReaderCallback *iface, DWORD output, QWORD time, QWORD duration, DWORD flags, INSSBuffer *sample, void *context) { struct callback *callback = impl_from_IWMReaderCallback(iface); + BYTE *data, *data2; HRESULT hr; DWORD size; - BYTE *data; if (winetest_debug > 1) trace("%u: %04x: IWMReaderCallback::OnSample(output %u, time %I64u, duration %I64u, flags %#x)\n", @@ -1196,6 +1206,10 @@ static HRESULT WINAPI callback_OnSample(IWMReaderCallback *iface, DWORD output, hr = INSSBuffer_GetBufferAndLength(sample, &data, &size); ok(hr == S_OK, "Got hr %#x.\n", hr); + hr = INSSBuffer_GetBuffer(sample, &data2); + ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(data2 == data, "Data pointers didn't match.\n"); + ok(callback->got_started > 0, "Got %u WMT_STARTED callbacks.\n", callback->got_started); ok(!callback->got_eof, "Got %u WMT_EOF callbacks.\n", callback->got_eof); ++callback->got_sample; -- 2.33.0

Nov. 8, 2021

Re: [PATCH 2/2] winegstreamer: Allow some audio resampling in the default pipeline.

by Zebediah Figura

On 11/8/21 11:41 AM, Rémi Bernon wrote: > On 11/8/21 17:36, Rémi Bernon wrote: >> On 10/27/21 22:34, Zebediah Figura (she/her) wrote: >>> On 10/27/21 10:25, Rémi Bernon wrote: >>>> Planet Coaster requests an output format with 44100 rate for user >>>> provided music, which may not match what the files are decoded to. >>>> >>>> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> >>> >>> This one looks good, although it'd be nice for this description to be >>> in the code itself. >>> >>> It could also be split into front and backend parts. >>> >> >> So it ends up being a little bit more complicated than that, and I don't >> think hardcoding a list of supported audio format is the right way to do >> it. In order to support all possible user music formats, we would have >> to hardcode all possible variations of media types. >> >> As far as I could see (and with tests from >> https://source.winehq.org/patches/data/219024) native streams only >> enumerate their native media types. Then, more media types are supported >> by the IMFSourceReader, but probably using a dynamically allocated >> decoder / converter MF transforms, or using MF topology elements which I >> believe are doing more advanced logic than what >> src_reader_SetCurrentMediaType does. >> >> Doing it this way would mean instantiating an audio_converter MF >> transform for instance, and would then make the audioresampler element >> useless. This seems to be the direction existing code is generally >> going, but it kind of defeat the idea of using GStreamer and its >> dynamically created pipelines. >> >> Another way I can see to make this dynamic, using an always present >> audioresampler element, would be instead to change the way we match >> IMFSourceReader stream media types, by delegating the type matching to >> winegstreamer, and GStreamer through gst_pad_query_caps calls. I'm not >> sure how we can do that, maybe with a custom IMFMediaTypeHandler, but >> it's not supposed to do this. > > Actually it should be possible to do something not too ugly, with an > audioresampler element, allowing audio conversion by accepting media > types directly in source_reader_set_compatible_media_type, as MSDN seems > to suggest [1] is done since Win8 (and as the test confirm). > > [1] > https://docs.microsoft.com/en-us/windows/win32/api/mfreadwrite/nf-mfreadwri… > I'm not entirely sure I understand what you're proposing? The patch as-is looks fine and is perfectly in line with what we do already. We basically have the frontend (mfplat, quartz, wmvcore) request a specific media type from the backend, and give the whole responsibility of deciding *what* media type to the frontend. In terms of mfplat separating its demuxers and decoders (really quartz and wmvcore do this too, but the design of mfplat makes this fact harder to ignore) I've been thus far inclined to try to keep demuxing within the same GStreamer pipeline as much as we can. In particular I'm worried about the latency of marshalling buffers between two or three threads (after all, we already have latency problems even on high-end processors), plus the added code complexity is not insignificant.

Nov. 8, 2021

Re: [PATCH v3 4/4] wined3d: Try to allocate new Vulkan BOs from the client thread for DISCARD maps.

by Zebediah Figura

On 11/8/21 1:35 PM, Henri Verbeet wrote: >>>> +void wined3d_buffer_set_bo(struct wined3d_buffer *buffer, struct wined3d_context *context, struct wined3d_bo *bo) >>>> +{ >>>> + TRACE("buffer %p, context %p, bo %p.\n", buffer, context, bo); >>>> + >>>> + buffer->buffer_ops->buffer_set_bo(buffer, context, bo); >>>> + buffer->buffer_object = (uintptr_t)bo; >>>> + wined3d_buffer_validate_location(buffer, WINED3D_LOCATION_BUFFER); >>>> + wined3d_buffer_invalidate_location(buffer, ~WINED3D_LOCATION_BUFFER); >>>> +} >>>> + >>> Is wined3d_buffer_set_bo() the proper place for the >>> validate/invalidate? Arguably it would be more appropriate to do that >>> in wined3d_cs_exec_update_sub_resource(). I suppose it mirrors >>> wined3d_buffer_copy_bo_address(). >> >> It definitely seems that most validation logic is done within buffer.c, >> and that makes more sense to me, along the lines of letting that be a >> buffer-specific implementation detail. I can change it if you feel >> strongly otherwise, though. >> > The issue isn't so much that it's in buffer.c; in principle it would > be fine to introduce e.g. wined3d_buffer_update_sub_resource() > containing the implementation of wined3d_cs_exec_update_sub_resource() > for buffers. The reason that doesn't already exist is mostly just that > the buffer part of wined3d_cs_exec_update_sub_resource() is fairly > straightforward. The issue here is more that wined3d_buffer_set_bo() > seems like something that's conceptually on a lower level than > sub-resource updates, much like e.g. > wined3d_resource_prepare_sysmem(), > wined3d_buffer_vk_create_buffer_object(), or > wined3d_buffer_gl_create_buffer_object(). I.e., it's not obvious to me > that replacing a buffer's bo should modify its location flags. Okay, that makes sense to me. I'll add a wined3d_buffer_update_sub_resource() helper.

Nov. 8, 2021

Re: [PATCH 5/5] winegstreamer: Implement IWMOutputMediaProps::GetType().

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=101492 Your paranoid android. === w7u_2qxl (32 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w7u_adm (32 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w7u_el (32 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w8 (32 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w8adm (32 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w864 (32 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w1064v1507 (32 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w1064v1809 (32 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w1064 (32 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w1064_tsign (32 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. wmvcore: Timeout === w864 (64 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w1064v1507 (64 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w1064v1809 (64 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w1064 (64 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w1064_2qxl (64 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w1064_tsign (64 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w10pro64_ar (64 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w10pro64_he (64 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w10pro64_ja (64 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out. === w10pro64_zh_CN (64 bit report) === wmvcore: wmvcore.c:1682: Test failed: Wait timed out.

Nov. 8, 2021

Re: [PATCH v3 4/4] wined3d: Try to allocate new Vulkan BOs from the client thread for DISCARD maps.

by Henri Verbeet

On Mon, 8 Nov 2021 at 18:01, Zebediah Figura <zfigura(a)codeweavers.com> wrote: > On 11/5/21 12:39 PM, Henri Verbeet wrote: > > On Fri, 5 Nov 2021 at 04:52, Zebediah Figura <zfigura(a)codeweavers.com> wrote: > >> + if (!bo_vk->b.map_ptr) > >> + { > >> + WARN_(d3d_perf)("BO %p (chunk %p, slab %p) is not persistently mapped.\n", > >> + bo_vk, bo_vk->memory ? bo_vk->memory->chunk : NULL, bo_vk->slab); > >> + > >> + if (!wined3d_bo_vk_map(bo_vk, context_vk)) > >> + ERR("Failed to map bo.\n"); > >> + } > >> + > > I suppose we could clean up and return "false" if mapping the bo fails > > here. On the other hand, I suppose it shouldn't fail in the first > > place. > > Cleaning up is a bit nontrivial; I'd rather leave it as is (or use an > assert?) > The ERR is fine. > >> +void wined3d_buffer_set_bo(struct wined3d_buffer *buffer, struct wined3d_context *context, struct wined3d_bo *bo) > >> +{ > >> + TRACE("buffer %p, context %p, bo %p.\n", buffer, context, bo); > >> + > >> + buffer->buffer_ops->buffer_set_bo(buffer, context, bo); > >> + buffer->buffer_object = (uintptr_t)bo; > >> + wined3d_buffer_validate_location(buffer, WINED3D_LOCATION_BUFFER); > >> + wined3d_buffer_invalidate_location(buffer, ~WINED3D_LOCATION_BUFFER); > >> +} > >> + > > Is wined3d_buffer_set_bo() the proper place for the > > validate/invalidate? Arguably it would be more appropriate to do that > > in wined3d_cs_exec_update_sub_resource(). I suppose it mirrors > > wined3d_buffer_copy_bo_address(). > > It definitely seems that most validation logic is done within buffer.c, > and that makes more sense to me, along the lines of letting that be a > buffer-specific implementation detail. I can change it if you feel > strongly otherwise, though. > The issue isn't so much that it's in buffer.c; in principle it would be fine to introduce e.g. wined3d_buffer_update_sub_resource() containing the implementation of wined3d_cs_exec_update_sub_resource() for buffers. The reason that doesn't already exist is mostly just that the buffer part of wined3d_cs_exec_update_sub_resource() is fairly straightforward. The issue here is more that wined3d_buffer_set_bo() seems like something that's conceptually on a lower level than sub-resource updates, much like e.g. wined3d_resource_prepare_sysmem(), wined3d_buffer_vk_create_buffer_object(), or wined3d_buffer_gl_create_buffer_object(). I.e., it's not obvious to me that replacing a buffer's bo should modify its location flags. > >> + if (prev_bo) > >> + { > >> + struct wined3d_bo_user *bo_user; > >> + > >> + LIST_FOR_EACH_ENTRY(bo_user, &prev_bo->b.users, struct wined3d_bo_user, entry) > >> + bo_user->valid = false; > >> + assert(list_empty(&bo_vk->b.users)); > >> + list_move_head(&bo_vk->b.users, &prev_bo->b.users); > >> + > >> + wined3d_context_vk_destroy_bo(context_vk, prev_bo); > >> + heap_free(prev_bo); > >> + } > >> + else > >> + { > >> + list_add_head(&bo_vk->b.users, &buffer_vk->b.bo_user.entry); > >> + } > >> +} > >> + > > Note that aside from the wined3d_context_vk_destroy_bo() call, this is > > all just generic code now. > > > > Yes, which is why I wanted to turn destroy_bo() into an adapter op > earlier. I guess if we had a wined3d_texture_set_bo() it would also be > all generic code except for a destroy_bo() call, so maybe it does make > sense to make destroy_bo() an adapter op, if not per se for > unload_location() as well. Right, I think doing bo destruction through the adapter ops would make sense for this purpose.

Nov. 8, 2021

Re: [PATCH 4/5] winegstreamer: Implement IWMReader::Open().

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=101491 Your paranoid android. === w7u_2qxl (32 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w8 (32 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w864 (32 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w1064v1507 (32 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w1064v1809 (32 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w1064 (32 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w1064_tsign (32 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. wmvcore: Timeout === w10pro64 (32 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w864 (64 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w1064v1507 (64 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w1064v1809 (64 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w1064 (64 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w1064_2qxl (64 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w1064_tsign (64 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w10pro64 (64 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w10pro64_ar (64 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w10pro64_he (64 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out. === w10pro64_ja (64 bit report) === wmvcore: wmvcore.c:1658: Test failed: Wait timed out.

Nov. 8, 2021

Re: [PATCH 2/5] winegstreamer: Implement INSSBuffer::GetMaxLength().

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=101489 Your paranoid android. === debiant2 (32 bit French report) === wmvcore: wmvcore.c:1486: Test failed: Output 0: Format 0: Got hr 0x8007000e. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00407666).

Nov. 8, 2021

Re: [PATCH 2/2] ucrtbase/tests: Test for FreeLibrary() vs _beginthread[ex]() race condition.

by Piotr Caban

Signed-off-by: Piotr Caban <piotr(a)codeweavers.com>

Nov. 8, 2021

Re: [PATCH 1/2] msvcrt: Increase module's reference count before returning from _beginthread[ex]().

by Piotr Caban

Signed-off-by: Piotr Caban <piotr(a)codeweavers.com>

Nov. 8, 2021