June 2022 - Wine-Devel - list.winehq.org

[PATCH v2] d2d1: Add support for painting image brush as a bitmap.

by Dmitry Timoshkov

v2: Add a test for sourceRectangle, drop ::FillGeometry() change. Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/d2d1/brush.c | 85 +++++++++++++++++++++++++++++++++++++----- dlls/d2d1/tests/d2d1.c | 46 +++++++++++++++++++++++ 2 files changed, 121 insertions(+), 10 deletions(-) diff --git a/dlls/d2d1/brush.c b/dlls/d2d1/brush.c index b9a673fe54c..7febafc375c 100644 --- a/dlls/d2d1/brush.c +++ b/dlls/d2d1/brush.c @@ -1450,7 +1450,23 @@ BOOL d2d_brush_fill_cb(const struct d2d_brush *brush, struct d2d_brush_cb *cb) return TRUE; case D2D_BRUSH_TYPE_BITMAP: - bitmap = brush->u.bitmap.bitmap; + case D2D_BRUSH_TYPE_IMAGE: + { + ID2D1Bitmap *src_bitmap; + + if (brush->type == D2D_BRUSH_TYPE_BITMAP) + bitmap = brush->u.bitmap.bitmap; + else + { + if (FAILED(ID2D1Image_QueryInterface(brush->u.image.image, &IID_ID2D1Bitmap, (void **)&src_bitmap))) + { + FIXME("ID2D1Image doesn't support ID2D1Bitmap interface.\n"); + return FALSE; + } + + bitmap = unsafe_impl_from_ID2D1Bitmap(src_bitmap); + cb->type = D2D_BRUSH_TYPE_BITMAP; + } /* Scale for bitmap size and dpi. */ b = brush->transform; @@ -1477,7 +1493,11 @@ BOOL d2d_brush_fill_cb(const struct d2d_brush *brush, struct d2d_brush_cb *cb) cb->u.bitmap.ignore_alpha = bitmap->format.alphaMode == D2D1_ALPHA_MODE_IGNORE; + if (brush->type == D2D_BRUSH_TYPE_IMAGE) + ID2D1Bitmap_Release(src_bitmap); + return TRUE; + } default: FIXME("Unhandled brush type %#x.\n", brush->type); @@ -1485,31 +1505,31 @@ BOOL d2d_brush_fill_cb(const struct d2d_brush *brush, struct d2d_brush_cb *cb) } } -static void d2d_brush_bind_bitmap(struct d2d_brush *brush, struct d2d_device_context *context, - unsigned int brush_idx) +static void d2d_brush_bind(struct d2d_bitmap *bitmap, struct d2d_device_context *context, + const D2D1_IMAGE_BRUSH_PROPERTIES *image_brush_desc, unsigned int brush_idx) { ID3D11SamplerState **sampler_state; ID3D11DeviceContext *d3d_context; HRESULT hr; ID3D11Device1_GetImmediateContext(context->d3d_device, &d3d_context); - ID3D11DeviceContext_PSSetShaderResources(d3d_context, brush_idx, 1, &brush->u.bitmap.bitmap->srv); + ID3D11DeviceContext_PSSetShaderResources(d3d_context, brush_idx, 1, &bitmap->srv); sampler_state = &context->sampler_states - [brush->u.bitmap.interpolation_mode % D2D_SAMPLER_INTERPOLATION_MODE_COUNT] - [brush->u.bitmap.extend_mode_x % D2D_SAMPLER_EXTEND_MODE_COUNT] - [brush->u.bitmap.extend_mode_y % D2D_SAMPLER_EXTEND_MODE_COUNT]; + [image_brush_desc->interpolationMode % D2D_SAMPLER_INTERPOLATION_MODE_COUNT] + [image_brush_desc->extendModeX % D2D_SAMPLER_EXTEND_MODE_COUNT] + [image_brush_desc->extendModeY % D2D_SAMPLER_EXTEND_MODE_COUNT]; if (!*sampler_state) { D3D11_SAMPLER_DESC sampler_desc; - if (brush->u.bitmap.interpolation_mode == D2D1_INTERPOLATION_MODE_NEAREST_NEIGHBOR) + if (image_brush_desc->interpolationMode == D2D1_INTERPOLATION_MODE_NEAREST_NEIGHBOR) sampler_desc.Filter = D3D11_FILTER_MIN_MAG_MIP_POINT; else sampler_desc.Filter = D3D11_FILTER_MIN_MAG_MIP_LINEAR; - sampler_desc.AddressU = texture_address_mode_from_extend_mode(brush->u.bitmap.extend_mode_x); - sampler_desc.AddressV = texture_address_mode_from_extend_mode(brush->u.bitmap.extend_mode_y); + sampler_desc.AddressU = texture_address_mode_from_extend_mode(image_brush_desc->extendModeX); + sampler_desc.AddressV = texture_address_mode_from_extend_mode(image_brush_desc->extendModeY); sampler_desc.AddressW = D3D11_TEXTURE_ADDRESS_CLAMP; sampler_desc.MipLODBias = 0.0f; sampler_desc.MaxAnisotropy = 0; @@ -1529,6 +1549,47 @@ static void d2d_brush_bind_bitmap(struct d2d_brush *brush, struct d2d_device_con ID3D11DeviceContext_Release(d3d_context); } +static void d2d_brush_bind_bitmap(struct d2d_brush *brush, struct d2d_device_context *context, + unsigned int brush_idx) +{ + D2D1_IMAGE_BRUSH_PROPERTIES image_brush_desc; + + image_brush_desc.sourceRectangle.left = 0.0f; + image_brush_desc.sourceRectangle.top = 0.0f; + image_brush_desc.sourceRectangle.right = brush->u.bitmap.bitmap->pixel_size.width; + image_brush_desc.sourceRectangle.bottom = brush->u.bitmap.bitmap->pixel_size.height; + image_brush_desc.extendModeX = brush->u.bitmap.extend_mode_x; + image_brush_desc.extendModeY = brush->u.bitmap.extend_mode_y; + image_brush_desc.interpolationMode = brush->u.bitmap.interpolation_mode; + + d2d_brush_bind(brush->u.bitmap.bitmap, context, &image_brush_desc, brush_idx); +} + +static void d2d_brush_bind_image(struct d2d_brush *brush, struct d2d_device_context *context, + unsigned int brush_idx) +{ + ID2D1Bitmap *src_bitmap; + struct d2d_bitmap *bitmap; + D2D1_IMAGE_BRUSH_PROPERTIES image_brush_desc; + + if (FAILED(ID2D1Image_QueryInterface(brush->u.image.image, &IID_ID2D1Bitmap, (void **)&src_bitmap))) + { + FIXME("ID2D1Image doesn't support ID2D1Bitmap interface.\n"); + return; + } + + bitmap = unsafe_impl_from_ID2D1Bitmap(src_bitmap); + + image_brush_desc.sourceRectangle = brush->u.image.source_rect; + image_brush_desc.extendModeX = brush->u.bitmap.extend_mode_x; + image_brush_desc.extendModeY = brush->u.bitmap.extend_mode_y; + image_brush_desc.interpolationMode = brush->u.bitmap.interpolation_mode; + + d2d_brush_bind(bitmap, context, &image_brush_desc, brush_idx); + + ID2D1Bitmap_Release(src_bitmap); +} + void d2d_brush_bind_resources(struct d2d_brush *brush, struct d2d_device_context *context, unsigned int brush_idx) { switch (brush->type) @@ -1548,6 +1609,10 @@ void d2d_brush_bind_resources(struct d2d_brush *brush, struct d2d_device_context d2d_brush_bind_bitmap(brush, context, brush_idx); break; + case D2D_BRUSH_TYPE_IMAGE: + d2d_brush_bind_image(brush, context, brush_idx); + break; + default: FIXME("Unhandled brush type %#x.\n", brush->type); break; diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index bc662b4f8ae..752e34bf4da 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -2509,11 +2509,13 @@ static void test_image_brush(BOOL d3d11) D2D1_BITMAP_PROPERTIES bitmap_desc; D2D1_BRUSH_PROPERTIES brush_desc; ID2D1Image *image, *tmp_image; + D2D1_RECT_F dst_rect; struct d2d1_test_context ctx; D2D1_EXTEND_MODE extend_mode; D2D1_MATRIX_3X2_F matrix; ID2D1ImageBrush *brush; ID2D1Bitmap *bitmap; + D2D1_COLOR_F color; D2D1_SIZE_U size; D2D1_RECT_F rect; ULONG refcount; @@ -2585,6 +2587,50 @@ static void test_image_brush(BOOL d3d11) ID2D1ImageBrush_Release(brush); + /* FillRectangle */ + set_rect(&image_brush_desc.sourceRectangle, 0.0f, 0.0f, 4.0f, 4.0f); + hr = ID2D1DeviceContext_CreateImageBrush(device_context, image, &image_brush_desc, NULL, &brush); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + + ID2D1ImageBrush_SetInterpolationMode(brush, D2D1_INTERPOLATION_MODE_NEAREST_NEIGHBOR); + + ID2D1RenderTarget_BeginDraw(ctx.rt); + + set_color(&color, 0.0f, 0.0f, 1.0f, 1.0f); + ID2D1RenderTarget_Clear(ctx.rt, &color); + + set_rect(&dst_rect, 40.0f, 120.0f, 120.0f, 360.0f); + ID2D1RenderTarget_FillRectangle(ctx.rt, &dst_rect, (ID2D1Brush *)brush); + + hr = ID2D1RenderTarget_EndDraw(ctx.rt, NULL, NULL); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + match = compare_surface(&ctx, "89917481db82e6d683a75f068d3984fe2703cce5"); + ok(match, "Surface does not match.\n"); + + ID2D1ImageBrush_Release(brush); + + set_rect(&image_brush_desc.sourceRectangle, 0.0f, 0.0f, 1.0f, 1.0f); + hr = ID2D1DeviceContext_CreateImageBrush(device_context, image, &image_brush_desc, NULL, &brush); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + + ID2D1ImageBrush_SetInterpolationMode(brush, D2D1_INTERPOLATION_MODE_NEAREST_NEIGHBOR); + + ID2D1RenderTarget_BeginDraw(ctx.rt); + + set_color(&color, 0.0f, 0.0f, 1.0f, 1.0f); + ID2D1RenderTarget_Clear(ctx.rt, &color); + + set_rect(&dst_rect, 40.0f, 120.0f, 120.0f, 360.0f); + ID2D1RenderTarget_FillRectangle(ctx.rt, &dst_rect, (ID2D1Brush *)brush); + + hr = ID2D1RenderTarget_EndDraw(ctx.rt, NULL, NULL); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + match = compare_surface(&ctx, "23544adf9695a51428c194a1cffd531be3416e65"); + todo_wine + ok(match, "Surface does not match.\n"); + + ID2D1ImageBrush_Release(brush); + /* Custom brush description and image pointer. */ brush_desc.opacity = 2.0f; set_matrix_identity(&brush_desc.transform); -- 2.36.1

June 7, 2022

Re: [PATCH v3] d2d1: Make ID2D1Device::CreateImageBrush() accept only bitmap as a source image.

by Nikolay Sivov

On 6/7/22 12:12, Dmitry Timoshkov wrote: > Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > >>> +static void d2d_brush_bind_bitmap(struct d2d_brush *brush, struct >>> d2d_device_context *context, >>> + unsigned int brush_idx) >>> +{ >>> + D2D1_IMAGE_BRUSH_PROPERTIES image_brush_desc; >>> + >>> + image_brush_desc.sourceRectangle.left = 0.0f; >>> + image_brush_desc.sourceRectangle.top = 0.0f; >>> + image_brush_desc.sourceRectangle.right = >>> brush->u.bitmap.bitmap->pixel_size.width; >>> + image_brush_desc.sourceRectangle.bottom = >>> brush->u.bitmap.bitmap->pixel_size.height; >> This needs a test, so we don't assume coordinate system here. We have >> tests for filling with 4x4 bitmap brush, it will be a matter of creating >> image brush with same bitmap, and rectangle as (0,0-4,4) vs (0,0-1,1). >> If that shows no difference it means this is using normalized >> coordinates, and we should initialize it here as (0,0-1,1). > I've added the test in the attached version of the patch, is that what you > had in mind? The test shows that (0,0-4,4) vs (0,0-1,1) source rectangles > lead to different painting results under Windows. However, since source > rectangle is completely ignored in current image brush implementation > that doesn't really change anything, so I'm not sure what kind of result > this test is supposed to have for the proposed patch. It shows if we should be using normalized rectangle for regular bitmap brushes, or rectangle in pixel coordinates, like you did. It looks like pixel coordinates are used, after I tried some visual tests. > >>> @@ -1043,7 +1043,7 @@ static void STDMETHODCALLTYPE >>> d2d_device_context_FillGeometry(ID2D1DeviceContext >>> if (FAILED(context->error.code)) >>> return; >>> >>> - if (opacity_brush && brush_impl->type != D2D_BRUSH_TYPE_BITMAP) >>> + if (opacity_brush && !(brush_impl->type == D2D_BRUSH_TYPE_BITMAP >>> || brush_impl->type == D2D_BRUSH_TYPE_IMAGE)) >>> { >>> d2d_device_context_set_error(context, >>> D2DERR_INCOMPATIBLE_BRUSH_TYPES); >>> return; >> Same here, we have a test for this case already, that needs to be >> extended to verify this change. > I guess you mean the tests in test_opacity_brush()? Anyway, it looks like > omitting this part of the patch changes nothing in my application, moreover > MSDN states that when the opacity brush is specified in FillGeometry() brush > must be an ID2D1BitmapBrush. So, this part is clearly wrong, and if desired > should be sent as a separate change. Yes, I think it's fine to leave this for a separate change. I'd expect bitmap brush and (image brush with a bitmap) be treated the same way here. Documentation could as well be outdated, because image brush was introduced with later versions, device contexts were not used in initially released d2d API. > > Thanks again for the helpful comments. >

June 7, 2022

Re: [PATCH v3] d2d1: Make ID2D1Device::CreateImageBrush() accept only bitmap as a source image.

by Dmitry Timoshkov

Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > > +static void d2d_brush_bind_bitmap(struct d2d_brush *brush, struct > > d2d_device_context *context, > > + unsigned int brush_idx) > > +{ > > + D2D1_IMAGE_BRUSH_PROPERTIES image_brush_desc; > > + > > + image_brush_desc.sourceRectangle.left = 0.0f; > > + image_brush_desc.sourceRectangle.top = 0.0f; > > + image_brush_desc.sourceRectangle.right = > > brush->u.bitmap.bitmap->pixel_size.width; > > + image_brush_desc.sourceRectangle.bottom = > > brush->u.bitmap.bitmap->pixel_size.height; > This needs a test, so we don't assume coordinate system here. We have > tests for filling with 4x4 bitmap brush, it will be a matter of creating > image brush with same bitmap, and rectangle as (0,0-4,4) vs (0,0-1,1). > If that shows no difference it means this is using normalized > coordinates, and we should initialize it here as (0,0-1,1). I've added the test in the attached version of the patch, is that what you had in mind? The test shows that (0,0-4,4) vs (0,0-1,1) source rectangles lead to different painting results under Windows. However, since source rectangle is completely ignored in current image brush implementation that doesn't really change anything, so I'm not sure what kind of result this test is supposed to have for the proposed patch. > > @@ -1043,7 +1043,7 @@ static void STDMETHODCALLTYPE > > d2d_device_context_FillGeometry(ID2D1DeviceContext > > if (FAILED(context->error.code)) > > return; > > > > - if (opacity_brush && brush_impl->type != D2D_BRUSH_TYPE_BITMAP) > > + if (opacity_brush && !(brush_impl->type == D2D_BRUSH_TYPE_BITMAP > > || brush_impl->type == D2D_BRUSH_TYPE_IMAGE)) > > { > > d2d_device_context_set_error(context, > > D2DERR_INCOMPATIBLE_BRUSH_TYPES); > > return; > Same here, we have a test for this case already, that needs to be > extended to verify this change. I guess you mean the tests in test_opacity_brush()? Anyway, it looks like omitting this part of the patch changes nothing in my application, moreover MSDN states that when the opacity brush is specified in FillGeometry() brush must be an ID2D1BitmapBrush. So, this part is clearly wrong, and if desired should be sent as a separate change. Thanks again for the helpful comments. -- Dmitry.

June 7, 2022

Re: [PATCH 2/6] uiautomationcore/tests: Skip UiaProviderFromIAccessible tests on Win10v1507.

by Huw Davies

On Wed, May 25, 2022 at 02:37:35PM -0400, Connor McAdams wrote: > In Windows 10 version 1507, UiaProviderFromIAccessible will try to check > the IAccessible passed in against the root IAccessible of the HWND > associated with the passed in IAccessible. In all other Windows > versions, this check is done on a call to get_HostRawElementProvider, or > the first time navigating towards a parent or sibling. Rather than work > around this, just skip the tests if this happens. Could we write the tests so that they don't rely on implementation details? Presumably applications run correctly on Windows 10 v1507. Huw.

June 7, 2022

Re: [PATCH 5/5] winegstreamer: Support zero-copy in wg_transform_push_data.

by Rémi Bernon

On 6/7/22 10:51, Marvin wrote: > 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=116377 > > Your paranoid android. > > > === debian11 (32 bit Japanese:Japan report) === > > mf: > Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x7d6032f9). > Looks like something broke since yesterday, doesn't seem to be related to the patches and more something with GLX driver: https://test.winehq.org/data/dd4a92bc259c5bddcf5265d608231646215496fd/linux… -- Rémi Bernon <rbernon(a)codeweavers.com>

June 7, 2022

Re: [PATCH 5/5] winegstreamer: Support zero-copy in wg_transform_push_data.

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=116377 Your paranoid android. === debian11 (32 bit Japanese:Japan report) === mf: Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x7d6032f9).

June 7, 2022

[PATCH 5/5] winegstreamer: Support zero-copy in wg_transform_push_data.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/mf/tests/mf.c | 1 - dlls/winegstreamer/gst_private.h | 9 ++- dlls/winegstreamer/h264_decoder.c | 17 ++++-- dlls/winegstreamer/mfplat.c | 83 ++++++++++++++++++++++++++- dlls/winegstreamer/quartz_transform.c | 9 +++ dlls/winegstreamer/unixlib.h | 2 + dlls/winegstreamer/wg_transform.c | 28 ++++++++- dlls/winegstreamer/wma_decoder.c | 15 ++++- 8 files changed, 149 insertions(+), 15 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 03035aa5e9f..13f0e38b0b8 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6423,7 +6423,6 @@ static void test_wma_decoder(void) hr = IMFTransform_ProcessInput(transform, 0, sample, 0); ok(hr == MF_E_NOTACCEPTING, "ProcessInput returned %#lx\n", hr); ret = IMFSample_Release(sample); - todo_wine ok(ret == 1, "Release returned %lu\n", ret); /* As output_info.dwFlags doesn't have MFT_OUTPUT_STREAM_CAN_PROVIDE_SAMPLES diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index ab89942847a..c06bbb610ff 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -64,6 +64,12 @@ static inline const char *debugstr_time(REFERENCE_TIME time) #define MEDIATIME_FROM_BYTES(x) ((LONGLONG)(x) * 10000000) +struct wg_sample_queue; + +HRESULT wg_sample_queue_create(struct wg_sample_queue **out); +void wg_sample_queue_destroy(struct wg_sample_queue *queue); +void wg_sample_queue_release(struct wg_sample_queue *queue, struct wg_sample *wg_sample, bool all); + struct wg_parser *wg_parser_create(enum wg_parser_type type, bool unlimited_buffering); void wg_parser_destroy(struct wg_parser *parser); @@ -123,7 +129,8 @@ void mf_media_type_to_wg_format(IMFMediaType *type, struct wg_format *format); HRESULT wg_sample_create_mf(IMFSample *sample, struct wg_sample **out); void wg_sample_release(struct wg_sample *wg_sample); -HRESULT wg_transform_push_mf(struct wg_transform *transform, struct wg_sample *sample); +HRESULT wg_transform_push_mf(struct wg_transform *transform, struct wg_sample *sample, + struct wg_sample_queue *queue); HRESULT wg_transform_read_mf(struct wg_transform *transform, struct wg_sample *sample, struct wg_format *format); diff --git a/dlls/winegstreamer/h264_decoder.c b/dlls/winegstreamer/h264_decoder.c index 8d3de4355bb..c8a289a7bdd 100644 --- a/dlls/winegstreamer/h264_decoder.c +++ b/dlls/winegstreamer/h264_decoder.c @@ -52,6 +52,7 @@ struct h264_decoder struct wg_format wg_format; struct wg_transform *wg_transform; + struct wg_sample_queue *wg_sample_queue; }; static struct h264_decoder *impl_from_IMFTransform(IMFTransform *iface) @@ -237,6 +238,8 @@ static ULONG WINAPI transform_Release(IMFTransform *iface) IMFMediaType_Release(decoder->input_type); if (decoder->output_type) IMFMediaType_Release(decoder->output_type); + + wg_sample_queue_destroy(decoder->wg_sample_queue); free(decoder); } @@ -543,9 +546,7 @@ static HRESULT WINAPI transform_ProcessInput(IMFTransform *iface, DWORD id, IMFS if (FAILED(hr = wg_sample_create_mf(sample, &wg_sample))) return hr; - hr = wg_transform_push_mf(decoder->wg_transform, wg_sample); - wg_sample_release(wg_sample); - return hr; + return wg_transform_push_mf(decoder->wg_transform, wg_sample, decoder->wg_sample_queue); } static HRESULT WINAPI transform_ProcessOutput(IMFTransform *iface, DWORD flags, DWORD count, @@ -582,7 +583,8 @@ static HRESULT WINAPI transform_ProcessOutput(IMFTransform *iface, DWORD flags, return MF_E_BUFFERTOOSMALL; } - hr = wg_transform_read_mf(decoder->wg_transform, wg_sample, &wg_format); + if (SUCCEEDED(hr = wg_transform_read_mf(decoder->wg_transform, wg_sample, &wg_format))) + wg_sample_queue_release(decoder->wg_sample_queue, NULL, false); wg_sample_release(wg_sample); if (hr == MF_E_TRANSFORM_STREAM_CHANGE) @@ -648,6 +650,7 @@ HRESULT h264_decoder_create(REFIID riid, void **ret) static const struct wg_format input_format = {.major_type = WG_MAJOR_TYPE_H264}; struct wg_transform *transform; struct h264_decoder *decoder; + HRESULT hr; TRACE("riid %s, ret %p.\n", debugstr_guid(riid), ret); @@ -669,6 +672,12 @@ HRESULT h264_decoder_create(REFIID riid, void **ret) decoder->wg_format.u.video.fps_n = 30000; decoder->wg_format.u.video.fps_d = 1001; + if (FAILED(hr = wg_sample_queue_create(&decoder->wg_sample_queue))) + { + free(decoder); + return hr; + } + *ret = &decoder->IMFTransform_iface; TRACE("Created decoder %p\n", *ret); return S_OK; diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index 40199706759..e4c6a56cdd5 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -26,6 +26,7 @@ #include "mfapi.h" #include "wine/debug.h" +#include "wine/list.h" WINE_DEFAULT_DEBUG_CHANNEL(mfplat); @@ -958,11 +959,18 @@ void mf_media_type_to_wg_format(IMFMediaType *type, struct wg_format *format) FIXME("Unrecognized major type %s.\n", debugstr_guid(&major_type)); } +struct wg_sample_queue +{ + CRITICAL_SECTION cs; + struct list samples; +}; + struct mf_sample { IMFSample *sample; IMFMediaBuffer *media_buffer; struct wg_sample wg_sample; + struct list entry; }; HRESULT wg_sample_create_mf(IMFSample *sample, struct wg_sample **out) @@ -1008,14 +1016,79 @@ void wg_sample_release(struct wg_sample *wg_sample) IMFMediaBuffer_Unlock(mf_sample->media_buffer); IMFMediaBuffer_Release(mf_sample->media_buffer); IMFSample_Release(mf_sample->sample); + free(mf_sample); } -HRESULT wg_transform_push_mf(struct wg_transform *transform, struct wg_sample *sample) +static void wg_sample_queue_append(struct wg_sample_queue *queue, struct wg_sample *wg_sample) +{ + struct mf_sample *mf_sample = CONTAINING_RECORD(wg_sample, struct mf_sample, wg_sample); + + /* make sure a concurrent wg_sample_queue_release call won't release the sample until we're done */ + InterlockedIncrement(&wg_sample->refcount); + mf_sample->wg_sample.flags |= WG_SAMPLE_FLAG_HAS_REFCOUNT; + + EnterCriticalSection(&queue->cs); + list_add_tail(&queue->samples, &mf_sample->entry); + LeaveCriticalSection(&queue->cs); +} + +void wg_sample_queue_release(struct wg_sample_queue *queue, struct wg_sample *wg_sample, bool all) { - struct mf_sample *mf_sample = CONTAINING_RECORD(sample, struct mf_sample, wg_sample); + struct mf_sample *mf_sample, *next; + + /* release temporary ref taken in wg_sample_queue_append */ + if (wg_sample) + InterlockedDecrement(&wg_sample->refcount); + + EnterCriticalSection(&queue->cs); + + LIST_FOR_EACH_ENTRY_SAFE(mf_sample, next, &queue->samples, struct mf_sample, entry) + { + if (!InterlockedOr(&mf_sample->wg_sample.refcount, 0) || all) + { + list_remove(&mf_sample->entry); + wg_sample_release(&mf_sample->wg_sample); + } + } + + LeaveCriticalSection(&queue->cs); +} + +HRESULT wg_sample_queue_create(struct wg_sample_queue **out) +{ + struct wg_sample_queue *queue; + + if (!(queue = calloc(1, sizeof(*queue)))) + return E_OUTOFMEMORY; + + InitializeCriticalSection(&queue->cs); + queue->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": cs"); + list_init(&queue->samples); + + TRACE("Created sample queue %p\n", queue); + *out = queue; + + return S_OK; +} + +void wg_sample_queue_destroy(struct wg_sample_queue *queue) +{ + wg_sample_queue_release(queue, NULL, true); + + queue->cs.DebugInfo->Spare[0] = 0; + InitializeCriticalSection(&queue->cs); + + free(queue); +} + +HRESULT wg_transform_push_mf(struct wg_transform *transform, struct wg_sample *wg_sample, + struct wg_sample_queue *queue) +{ + struct mf_sample *mf_sample = CONTAINING_RECORD(wg_sample, struct mf_sample, wg_sample); LONGLONG time, duration; UINT32 value; + HRESULT hr; if (SUCCEEDED(IMFSample_GetSampleTime(mf_sample->sample, &time))) { @@ -1030,7 +1103,11 @@ HRESULT wg_transform_push_mf(struct wg_transform *transform, struct wg_sample *s if (SUCCEEDED(IMFSample_GetUINT32(mf_sample->sample, &MFSampleExtension_CleanPoint, &value)) && value) mf_sample->wg_sample.flags |= WG_SAMPLE_FLAG_SYNC_POINT; - return wg_transform_push_data(transform, sample); + wg_sample_queue_append(queue, wg_sample); + hr = wg_transform_push_data(transform, wg_sample); + wg_sample_queue_release(queue, wg_sample, false); + + return hr; } HRESULT wg_transform_read_mf(struct wg_transform *transform, struct wg_sample *wg_sample, diff --git a/dlls/winegstreamer/quartz_transform.c b/dlls/winegstreamer/quartz_transform.c index 326b8691a42..b701b3f6369 100644 --- a/dlls/winegstreamer/quartz_transform.c +++ b/dlls/winegstreamer/quartz_transform.c @@ -40,6 +40,7 @@ struct transform IQualityControl *qc_sink; struct wg_transform *transform; + struct wg_sample_queue *sample_queue; const struct transform_ops *ops; }; @@ -76,6 +77,7 @@ static void transform_destroy(struct strmbase_filter *iface) strmbase_sink_cleanup(&filter->sink); strmbase_filter_cleanup(&filter->filter); + wg_sample_queue_destroy(filter->sample_queue); free(filter); } @@ -572,11 +574,18 @@ static const IQualityControlVtbl source_quality_control_vtbl = static HRESULT transform_create(IUnknown *outer, const CLSID *clsid, const struct transform_ops *ops, struct transform **out) { struct transform *object; + HRESULT hr; object = calloc(1, sizeof(*object)); if (!object) return E_OUTOFMEMORY; + if (FAILED(hr = wg_sample_queue_create(&object->sample_queue))) + { + free(object); + return hr; + } + strmbase_filter_init(&object->filter, outer, clsid, &filter_ops); strmbase_sink_init(&object->sink, &object->filter, L"In", &sink_ops, NULL); strmbase_source_init(&object->source, &object->filter, L"Out", &source_ops); diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index 860a8ab2a52..ed56fb47908 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -121,6 +121,8 @@ enum wg_sample_flag WG_SAMPLE_FLAG_HAS_PTS = 2, WG_SAMPLE_FLAG_HAS_DURATION = 4, WG_SAMPLE_FLAG_SYNC_POINT = 8, + + WG_SAMPLE_FLAG_HAS_REFCOUNT = 0x10000, /* sample is queued on the client side and may be wrapped */ }; struct wg_sample diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 25630ab7596..e6c7344bc72 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -518,6 +518,13 @@ out: return status; } +static void wg_sample_free_notify(void *arg) +{ + struct wg_sample *sample = arg; + GST_DEBUG("Releasing wg_sample %p", sample); + InterlockedDecrement(&sample->refcount); +} + NTSTATUS wg_transform_push_data(void *args) { struct wg_transform_push_data_params *params = args; @@ -534,12 +541,28 @@ NTSTATUS wg_transform_push_data(void *args) return STATUS_SUCCESS; } - if (!(buffer = gst_buffer_new_and_alloc(sample->size))) + if (!(sample->flags & WG_SAMPLE_FLAG_HAS_REFCOUNT)) + { + if (!(buffer = gst_buffer_new_and_alloc(sample->size))) + { + GST_ERROR("Failed to allocate input buffer"); + return STATUS_NO_MEMORY; + } + gst_buffer_fill(buffer, 0, sample->data, sample->size); + GST_INFO("Copied %u bytes from sample %p to buffer %p", sample->size, sample, buffer); + } + else if (!(buffer = gst_buffer_new_wrapped_full(GST_MEMORY_FLAG_READONLY, sample->data, sample->max_size, + 0, sample->size, sample, wg_sample_free_notify))) { GST_ERROR("Failed to allocate input buffer"); return STATUS_NO_MEMORY; } - gst_buffer_fill(buffer, 0, sample->data, sample->size); + else + { + InterlockedIncrement(&sample->refcount); + GST_INFO("Wrapped %u/%u bytes from sample %p to buffer %p", sample->size, sample->max_size, sample, buffer); + } + if (sample->flags & WG_SAMPLE_FLAG_HAS_PTS) GST_BUFFER_PTS(buffer) = sample->pts * 100; if (sample->flags & WG_SAMPLE_FLAG_HAS_DURATION) @@ -548,7 +571,6 @@ NTSTATUS wg_transform_push_data(void *args) GST_BUFFER_FLAG_SET(buffer, GST_BUFFER_FLAG_DELTA_UNIT); gst_buffer_list_insert(transform->input, -1, buffer); - GST_INFO("Copied %u bytes from sample %p to input buffer list", sample->size, sample); params->result = S_OK; return STATUS_SUCCESS; } diff --git a/dlls/winegstreamer/wma_decoder.c b/dlls/winegstreamer/wma_decoder.c index 81285f5ad44..8ac63c4d54e 100644 --- a/dlls/winegstreamer/wma_decoder.c +++ b/dlls/winegstreamer/wma_decoder.c @@ -56,6 +56,7 @@ struct wma_decoder IMFMediaType *output_type; struct wg_transform *wg_transform; + struct wg_sample_queue *wg_sample_queue; }; static inline struct wma_decoder *impl_from_IUnknown(IUnknown *iface) @@ -135,6 +136,8 @@ static ULONG WINAPI unknown_Release(IUnknown *iface) IMFMediaType_Release(decoder->input_type); if (decoder->output_type) IMFMediaType_Release(decoder->output_type); + + wg_sample_queue_destroy(decoder->wg_sample_queue); free(decoder); } @@ -544,9 +547,7 @@ static HRESULT WINAPI transform_ProcessInput(IMFTransform *iface, DWORD id, IMFS return S_OK; } - hr = wg_transform_push_mf(decoder->wg_transform, wg_sample); - wg_sample_release(wg_sample); - return hr; + return wg_transform_push_mf(decoder->wg_transform, wg_sample, decoder->wg_sample_queue); } static HRESULT WINAPI transform_ProcessOutput(IMFTransform *iface, DWORD flags, DWORD count, @@ -590,6 +591,7 @@ static HRESULT WINAPI transform_ProcessOutput(IMFTransform *iface, DWORD flags, { if (wg_sample->flags & WG_SAMPLE_FLAG_INCOMPLETE) samples[0].dwStatus |= MFT_OUTPUT_DATA_BUFFER_INCOMPLETE; + wg_sample_queue_release(decoder->wg_sample_queue, NULL, false); } wg_sample_release(wg_sample); @@ -882,6 +884,7 @@ HRESULT wma_decoder_create(IUnknown *outer, IUnknown **out) static const struct wg_format input_format = {.major_type = WG_MAJOR_TYPE_WMA}; struct wg_transform *transform; struct wma_decoder *decoder; + HRESULT hr; TRACE("outer %p, out %p.\n", outer, out); @@ -895,6 +898,12 @@ HRESULT wma_decoder_create(IUnknown *outer, IUnknown **out) if (!(decoder = calloc(1, sizeof(*decoder)))) return E_OUTOFMEMORY; + if (FAILED(hr = wg_sample_queue_create(&decoder->wg_sample_queue))) + { + free(decoder); + return hr; + } + decoder->IUnknown_inner.lpVtbl = &unknown_vtbl; decoder->IMFTransform_iface.lpVtbl = &transform_vtbl; decoder->IMediaObject_iface.lpVtbl = &media_object_vtbl; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/197

June 7, 2022

[PATCH 4/5] winegstreamer: Support zero-copy output using the allocator.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Through a custom allocator, by borrowing memory from the reading thread and mapping it instead of the allocated memory. We cannot use the buffer pool to share wrapped buffers, because some decoder will hold on the acquired buffers longer than they should and we cannot remove our memory from them as long as they keep a reference. Swapping the memory on map should be safe. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/mfplat.c | 6 ++ dlls/winegstreamer/unix_private.h | 7 +- dlls/winegstreamer/unixlib.h | 1 + dlls/winegstreamer/wg_allocator.c | 135 ++++++++++++++++++++++++++++-- dlls/winegstreamer/wg_transform.c | 94 ++++++++++++++++++--- 5 files changed, 227 insertions(+), 16 deletions(-) diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index 28a3fc20ead..40199706759 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -999,6 +999,12 @@ void wg_sample_release(struct wg_sample *wg_sample) { struct mf_sample *mf_sample = CONTAINING_RECORD(wg_sample, struct mf_sample, wg_sample); + if (InterlockedOr(&wg_sample->refcount, 0)) + { + ERR("Sample %p is still in use, trouble ahead!\n", wg_sample); + return; + } + IMFMediaBuffer_Unlock(mf_sample->media_buffer); IMFMediaBuffer_Release(mf_sample->media_buffer); IMFSample_Release(mf_sample->sample); diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index 16615ef0833..e9f472986ae 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -37,7 +37,12 @@ extern NTSTATUS wg_transform_destroy(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_push_data(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_read_data(void *args) DECLSPEC_HIDDEN; -extern GstAllocator *wg_allocator_create(void) DECLSPEC_HIDDEN; +/* wg_allocator_release_sample can be used to release any sample that was requested. */ +typedef struct wg_sample *(*wg_allocator_request_sample_cb)(gsize size, void *context); +extern GstAllocator *wg_allocator_create(wg_allocator_request_sample_cb request_sample, + void *request_sample_context) DECLSPEC_HIDDEN; extern void wg_allocator_destroy(GstAllocator *allocator) DECLSPEC_HIDDEN; +extern void wg_allocator_release_sample(GstAllocator *allocator, struct wg_sample *sample, + bool discard_data) DECLSPEC_HIDDEN; #endif /* __WINE_WINEGSTREAMER_UNIX_PRIVATE_H */ diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index f334a168bd1..860a8ab2a52 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -128,6 +128,7 @@ struct wg_sample /* timestamp and duration are in 100-nanosecond units. */ UINT64 pts; UINT64 duration; + LONG refcount; /* unix refcount */ UINT32 flags; UINT32 max_size; UINT32 size; diff --git a/dlls/winegstreamer/wg_allocator.c b/dlls/winegstreamer/wg_allocator.c index 90dada288ae..c31751ce83f 100644 --- a/dlls/winegstreamer/wg_allocator.c +++ b/dlls/winegstreamer/wg_allocator.c @@ -33,20 +33,33 @@ #include "unix_private.h" +#include "wine/list.h" + GST_DEBUG_CATEGORY_EXTERN(wine); #define GST_CAT_DEFAULT wine typedef struct { GstMemory parent; + struct list entry; GstMemory *unix_memory; GstMapInfo unix_map_info; + + struct wg_sample *sample; + gsize written; } WgMemory; typedef struct { GstAllocator parent; + + wg_allocator_request_sample_cb request_sample; + void *request_sample_context; + + pthread_mutex_t mutex; + pthread_cond_t release_cond; + struct list memory_list; } WgAllocator; typedef struct @@ -58,6 +71,7 @@ G_DEFINE_TYPE(WgAllocator, wg_allocator, GST_TYPE_ALLOCATOR); static gpointer wg_allocator_map(GstMemory *gst_memory, GstMapInfo *info, gsize maxsize) { + WgAllocator *allocator = (WgAllocator *)gst_memory->allocator; WgMemory *memory = (WgMemory *)gst_memory; if (gst_memory->parent) @@ -65,7 +79,19 @@ static gpointer wg_allocator_map(GstMemory *gst_memory, GstMapInfo *info, gsize GST_LOG("memory %p, info %p, maxsize %#zx", memory, info, maxsize); - info->data = memory->unix_map_info.data; + pthread_mutex_lock(&allocator->mutex); + + if (!memory->sample) + info->data = memory->unix_map_info.data; + else + { + InterlockedIncrement(&memory->sample->refcount); + info->data = memory->sample->data; + } + if (info->flags & GST_MAP_WRITE) + memory->written = max(memory->written, maxsize); + + pthread_mutex_unlock(&allocator->mutex); GST_INFO("Mapped memory %p to %p", memory, info->data); return info->data; @@ -73,12 +99,23 @@ static gpointer wg_allocator_map(GstMemory *gst_memory, GstMapInfo *info, gsize static void wg_allocator_unmap(GstMemory *gst_memory, GstMapInfo *info) { + WgAllocator *allocator = (WgAllocator *)gst_memory->allocator; WgMemory *memory = (WgMemory *)gst_memory; if (gst_memory->parent) return wg_allocator_unmap(gst_memory->parent, info); GST_LOG("memory %p, info %p", memory, info); + + pthread_mutex_lock(&allocator->mutex); + + if (memory->sample && info->data == memory->sample->data) + { + InterlockedDecrement(&memory->sample->refcount); + pthread_cond_signal(&allocator->release_cond); + } + + pthread_mutex_unlock(&allocator->mutex); } static void wg_allocator_init(WgAllocator *allocator) @@ -91,6 +128,10 @@ static void wg_allocator_init(WgAllocator *allocator) allocator->parent.mem_unmap_full = wg_allocator_unmap; GST_OBJECT_FLAG_SET(allocator, GST_ALLOCATOR_FLAG_CUSTOM_ALLOC); + + pthread_mutex_init(&allocator->mutex, NULL); + pthread_cond_init(&allocator->release_cond, NULL); + list_init(&allocator->memory_list); } static void wg_allocator_finalize(GObject *object) @@ -99,6 +140,9 @@ static void wg_allocator_finalize(GObject *object) GST_LOG("allocator %p", allocator); + pthread_cond_destroy(&allocator->release_cond); + pthread_mutex_destroy(&allocator->mutex); + G_OBJECT_CLASS(wg_allocator_parent_class)->finalize(object); } @@ -116,8 +160,15 @@ static GstMemory *wg_allocator_alloc(GstAllocator *gst_allocator, gsize size, memory->unix_memory = gst_allocator_alloc(NULL, size, params); gst_memory_map(memory->unix_memory, &memory->unix_map_info, GST_MAP_WRITE); - GST_INFO("Allocated memory %p, unix_memory %p, data %p", memory, memory->unix_memory, - memory->unix_map_info.data); + pthread_mutex_lock(&allocator->mutex); + + memory->sample = allocator->request_sample(size, allocator->request_sample_context); + list_add_tail(&allocator->memory_list, &memory->entry); + + pthread_mutex_unlock(&allocator->mutex); + + GST_INFO("Allocated memory %p, sample %p, unix_memory %p, data %p", memory, + memory->sample, memory->unix_memory, memory->unix_map_info.data); return (GstMemory *)memory; } @@ -128,6 +179,16 @@ static void wg_allocator_free(GstAllocator *gst_allocator, GstMemory *gst_memory GST_LOG("allocator %p, memory %p", allocator, memory); + pthread_mutex_lock(&allocator->mutex); + + if (memory->sample) + InterlockedDecrement(&memory->sample->refcount); + memory->sample = NULL; + + list_remove(&memory->entry); + + pthread_mutex_unlock(&allocator->mutex); + gst_memory_unmap(memory->unix_memory, &memory->unix_map_info); gst_memory_unref(memory->unix_memory); g_slice_free(WgMemory, memory); @@ -145,18 +206,82 @@ static void wg_allocator_class_init(WgAllocatorClass *klass) root_class->finalize = wg_allocator_finalize; } -GstAllocator *wg_allocator_create(void) +GstAllocator *wg_allocator_create(wg_allocator_request_sample_cb request_sample, void *request_sample_context) +{ + WgAllocator *allocator; + + if (!(allocator = g_object_new(wg_allocator_get_type(), NULL))) + return NULL; + + allocator->request_sample = request_sample; + allocator->request_sample_context = request_sample_context; + return GST_ALLOCATOR(allocator); +} + +static void release_memory_sample(WgAllocator *allocator, WgMemory *memory, bool discard_data) { - return g_object_new(wg_allocator_get_type(), NULL); + struct wg_sample *sample; + + if (!(sample = memory->sample)) + return; + + while (sample->refcount > 1) + { + GST_WARNING("Waiting for sample %p to be unmapped", sample); + pthread_cond_wait(&allocator->release_cond, &allocator->mutex); + } + InterlockedDecrement(&sample->refcount); + + if (memory->written && !discard_data) + { + GST_WARNING("Copying %#zx bytes from sample %p, back to memory %p", memory->written, sample, memory); + memcpy(memory->unix_map_info.data, memory->sample->data, memory->written); + } + + memory->sample = NULL; + GST_INFO("Released sample %p from memory %p", sample, memory); } void wg_allocator_destroy(GstAllocator *gst_allocator) { WgAllocator *allocator = (WgAllocator *)gst_allocator; + WgMemory *memory; GST_LOG("allocator %p", allocator); + pthread_mutex_lock(&allocator->mutex); + LIST_FOR_EACH_ENTRY(memory, &allocator->memory_list, WgMemory, entry) + release_memory_sample(allocator, memory, true); + pthread_mutex_unlock(&allocator->mutex); + g_object_unref(allocator); GST_INFO("Destroyed buffer allocator %p", allocator); } + +static WgMemory *find_sample_memory(WgAllocator *allocator, struct wg_sample *sample) +{ + WgMemory *memory; + + LIST_FOR_EACH_ENTRY(memory, &allocator->memory_list, WgMemory, entry) + if (memory->sample == sample) + return memory; + + return NULL; +} + +void wg_allocator_release_sample(GstAllocator *gst_allocator, struct wg_sample *sample, + bool discard_data) +{ + WgAllocator *allocator = (WgAllocator *)gst_allocator; + WgMemory *memory; + + GST_LOG("allocator %p, sample %p, discard_data %u", allocator, sample, discard_data); + + pthread_mutex_lock(&allocator->mutex); + if ((memory = find_sample_memory(allocator, sample))) + release_memory_sample(allocator, memory, discard_data); + else if (sample->refcount) + GST_ERROR("Couldn't find memory for sample %p", sample); + pthread_mutex_unlock(&allocator->mutex); +} diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index c87536e5fbb..25630ab7596 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -54,6 +54,7 @@ struct wg_transform GstBufferList *input; guint input_max_length; guint output_plane_align; + struct wg_sample *output_wg_sample; GstAtomicQueue *output_queue; GstSample *output_sample; bool output_caps_changed; @@ -305,6 +306,20 @@ static bool transform_append_element(struct wg_transform *transform, GstElement return success; } +static struct wg_sample *transform_request_sample(gsize size, void *context) +{ + struct wg_transform *transform = context; + struct wg_sample *sample; + + GST_LOG("size %#zx, context %p", size, transform); + + sample = InterlockedExchangePointer((void **)&transform->output_wg_sample, NULL); + if (!sample || sample->max_size < size) + return NULL; + + return sample; +} + NTSTATUS wg_transform_create(void *args) { struct wg_transform_create_params *params = args; @@ -329,7 +344,7 @@ NTSTATUS wg_transform_create(void *args) goto out; if (!(transform->output_queue = gst_atomic_queue_new(8))) goto out; - if (!(transform->allocator = wg_allocator_create())) + if (!(transform->allocator = wg_allocator_create(transform_request_sample, transform))) goto out; transform->input_max_length = 1; transform->output_plane_align = 0; @@ -622,10 +637,22 @@ static bool copy_buffer(GstBuffer *buffer, GstCaps *caps, struct wg_sample *samp static NTSTATUS read_transform_output_data(GstBuffer *buffer, GstCaps *caps, gsize plane_align, struct wg_sample *sample) { + bool ret, needs_copy; gsize total_size; - bool ret; + GstMapInfo info; - if (is_caps_video(caps)) + if (!gst_buffer_map(buffer, &info, GST_MAP_READ)) + { + GST_ERROR("Failed to map buffer %p", buffer); + sample->size = 0; + return STATUS_UNSUCCESSFUL; + } + needs_copy = info.data != sample->data; + gst_buffer_unmap(buffer, &info); + + if ((ret = !needs_copy)) + total_size = sample->size = info.size; + else if (is_caps_video(caps)) ret = copy_video_buffer(buffer, caps, plane_align, sample, &total_size); else ret = copy_buffer(buffer, caps, sample, &total_size); @@ -657,7 +684,18 @@ static NTSTATUS read_transform_output_data(GstBuffer *buffer, GstCaps *caps, gsi if (!GST_BUFFER_FLAG_IS_SET(buffer, GST_BUFFER_FLAG_DELTA_UNIT)) sample->flags |= WG_SAMPLE_FLAG_SYNC_POINT; - GST_INFO("Copied %u bytes, sample %p, flags %#x", sample->size, sample, sample->flags); + if (needs_copy) + { + if (is_caps_video(caps)) + GST_WARNING("Copied %u bytes, sample %p, flags %#x", sample->size, sample, sample->flags); + else + GST_INFO("Copied %u bytes, sample %p, flags %#x", sample->size, sample, sample->flags); + } + else if (sample->flags & WG_SAMPLE_FLAG_INCOMPLETE) + GST_ERROR("Partial read %u bytes, sample %p, flags %#x", sample->size, sample, sample->flags); + else + GST_INFO("Read %u bytes, sample %p, flags %#x", sample->size, sample, sample->flags); + return STATUS_SUCCESS; } @@ -667,23 +705,38 @@ NTSTATUS wg_transform_read_data(void *args) struct wg_transform *transform = params->transform; struct wg_sample *sample = params->sample; struct wg_format *format = params->format; - GstBufferList *input = transform->input; + GstFlowReturn ret = GST_FLOW_OK; GstBuffer *output_buffer; + bool discard_data = true; + GstBufferList *input; GstCaps *output_caps; - GstFlowReturn ret; NTSTATUS status; + /* Provide the sample for transform_request_sample to pick it up */ + InterlockedIncrement(&sample->refcount); + InterlockedExchangePointer((void **)&transform->output_wg_sample, sample); + if (!gst_buffer_list_length(transform->input)) GST_DEBUG("Not input buffer queued"); - else if (!(transform->input = gst_buffer_list_new())) + else if ((input = gst_buffer_list_new())) + { + ret = gst_pad_push_list(transform->my_src, transform->input); + transform->input = input; + } + else { GST_ERROR("Failed to allocate new input queue"); - gst_buffer_list_unref(input); - return STATUS_NO_MEMORY; + ret = GST_FLOW_ERROR; } - else if ((ret = gst_pad_push_list(transform->my_src, input))) + + /* Remove the sample so transform_request_sample cannot use it */ + if (InterlockedExchangePointer((void **)&transform->output_wg_sample, NULL)) + InterlockedDecrement(&sample->refcount); + + if (ret) { GST_ERROR("Failed to push transform input, error %d", ret); + wg_allocator_release_sample(transform->allocator, sample, false); return STATUS_UNSUCCESSFUL; } @@ -692,6 +745,7 @@ NTSTATUS wg_transform_read_data(void *args) sample->size = 0; params->result = MF_E_TRANSFORM_NEED_MORE_INPUT; GST_INFO("Cannot read %u bytes, no output available", sample->max_size); + wg_allocator_release_sample(transform->allocator, sample, false); return STATUS_SUCCESS; } @@ -730,19 +784,39 @@ NTSTATUS wg_transform_read_data(void *args) params->result = MF_E_TRANSFORM_STREAM_CHANGE; GST_INFO("Format changed detected, returning no output"); + wg_allocator_release_sample(transform->allocator, sample, false); return STATUS_SUCCESS; } if ((status = read_transform_output_data(output_buffer, output_caps, transform->output_plane_align, sample))) + { + wg_allocator_release_sample(transform->allocator, sample, false); return status; + } if (!(sample->flags & WG_SAMPLE_FLAG_INCOMPLETE)) { + /* Taint the buffer memory to make sure it cannot be reused by the buffer pool, + * for the pool to always requests new memory from the allocator, and so we can + * then always provide output sample memory to achieve zero-copy. + * + * However, some decoder keep a reference on the buffer they passed downstream, + * to re-use it later. In this case, it will not be possible to do zero-copy, + * and we should copy the data back to the buffer and leave it unchanged. + * + * Some other plugins make assumptions that the returned buffer will always have + * at least one memory attached, we cannot just remove it and need to replace the + * memory instead. + */ + if ((discard_data = gst_buffer_is_writable(output_buffer))) + gst_buffer_replace_all_memory(output_buffer, gst_allocator_alloc(NULL, 0, NULL)); + gst_sample_unref(transform->output_sample); transform->output_sample = NULL; } params->result = S_OK; + wg_allocator_release_sample(transform->allocator, sample, discard_data); return STATUS_SUCCESS; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/197

June 7, 2022

[PATCH 3/5] winegstreamer: Introduce a new custom memory allocator.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/Makefile.in | 1 + dlls/winegstreamer/unix_private.h | 3 + dlls/winegstreamer/wg_allocator.c | 162 ++++++++++++++++++++++++++++++ dlls/winegstreamer/wg_transform.c | 12 ++- 4 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 dlls/winegstreamer/wg_allocator.c diff --git a/dlls/winegstreamer/Makefile.in b/dlls/winegstreamer/Makefile.in index e4c2636d02d..50f4dc861d4 100644 --- a/dlls/winegstreamer/Makefile.in +++ b/dlls/winegstreamer/Makefile.in @@ -14,6 +14,7 @@ C_SRCS = \ mfplat.c \ quartz_parser.c \ quartz_transform.c \ + wg_allocator.c \ wg_format.c \ wg_parser.c \ wg_transform.c \ diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index 7bce8263aaf..16615ef0833 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -37,4 +37,7 @@ extern NTSTATUS wg_transform_destroy(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_push_data(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_read_data(void *args) DECLSPEC_HIDDEN; +extern GstAllocator *wg_allocator_create(void) DECLSPEC_HIDDEN; +extern void wg_allocator_destroy(GstAllocator *allocator) DECLSPEC_HIDDEN; + #endif /* __WINE_WINEGSTREAMER_UNIX_PRIVATE_H */ diff --git a/dlls/winegstreamer/wg_allocator.c b/dlls/winegstreamer/wg_allocator.c new file mode 100644 index 00000000000..90dada288ae --- /dev/null +++ b/dlls/winegstreamer/wg_allocator.c @@ -0,0 +1,162 @@ +/* + * GStreamer memory allocator + * + * Copyright 2022 Rémi Bernon for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep unix +#endif + +#include "config.h" + +#include <assert.h> +#include <stdarg.h> + +#include <gst/gst.h> +#include <gst/video/video.h> +#include <gst/audio/audio.h> + +#include "unix_private.h" + +GST_DEBUG_CATEGORY_EXTERN(wine); +#define GST_CAT_DEFAULT wine + +typedef struct +{ + GstMemory parent; + + GstMemory *unix_memory; + GstMapInfo unix_map_info; +} WgMemory; + +typedef struct +{ + GstAllocator parent; +} WgAllocator; + +typedef struct +{ + GstAllocatorClass parent_class; +} WgAllocatorClass; + +G_DEFINE_TYPE(WgAllocator, wg_allocator, GST_TYPE_ALLOCATOR); + +static gpointer wg_allocator_map(GstMemory *gst_memory, GstMapInfo *info, gsize maxsize) +{ + WgMemory *memory = (WgMemory *)gst_memory; + + if (gst_memory->parent) + return wg_allocator_map(gst_memory->parent, info, maxsize); + + GST_LOG("memory %p, info %p, maxsize %#zx", memory, info, maxsize); + + info->data = memory->unix_map_info.data; + + GST_INFO("Mapped memory %p to %p", memory, info->data); + return info->data; +} + +static void wg_allocator_unmap(GstMemory *gst_memory, GstMapInfo *info) +{ + WgMemory *memory = (WgMemory *)gst_memory; + + if (gst_memory->parent) + return wg_allocator_unmap(gst_memory->parent, info); + + GST_LOG("memory %p, info %p", memory, info); +} + +static void wg_allocator_init(WgAllocator *allocator) +{ + GST_LOG("allocator %p", allocator); + + allocator->parent.mem_type = "Wine"; + + allocator->parent.mem_map_full = wg_allocator_map; + allocator->parent.mem_unmap_full = wg_allocator_unmap; + + GST_OBJECT_FLAG_SET(allocator, GST_ALLOCATOR_FLAG_CUSTOM_ALLOC); +} + +static void wg_allocator_finalize(GObject *object) +{ + WgAllocator *allocator = (WgAllocator *)object; + + GST_LOG("allocator %p", allocator); + + G_OBJECT_CLASS(wg_allocator_parent_class)->finalize(object); +} + +static GstMemory *wg_allocator_alloc(GstAllocator *gst_allocator, gsize size, + GstAllocationParams *params) +{ + WgAllocator *allocator = (WgAllocator *)gst_allocator; + WgMemory *memory; + + GST_LOG("allocator %p, size %#zx, params %p", allocator, size, params); + + memory = g_slice_new0(WgMemory); + gst_memory_init(GST_MEMORY_CAST(memory), 0, GST_ALLOCATOR_CAST(allocator), + NULL, size, 0, 0, size); + memory->unix_memory = gst_allocator_alloc(NULL, size, params); + gst_memory_map(memory->unix_memory, &memory->unix_map_info, GST_MAP_WRITE); + + GST_INFO("Allocated memory %p, unix_memory %p, data %p", memory, memory->unix_memory, + memory->unix_map_info.data); + return (GstMemory *)memory; +} + +static void wg_allocator_free(GstAllocator *gst_allocator, GstMemory *gst_memory) +{ + WgAllocator *allocator = (WgAllocator *)gst_allocator; + WgMemory *memory = (WgMemory *)gst_memory; + + GST_LOG("allocator %p, memory %p", allocator, memory); + + gst_memory_unmap(memory->unix_memory, &memory->unix_map_info); + gst_memory_unref(memory->unix_memory); + g_slice_free(WgMemory, memory); +} + +static void wg_allocator_class_init(WgAllocatorClass *klass) +{ + GstAllocatorClass *parent_class = (GstAllocatorClass *)klass; + GObjectClass *root_class = (GObjectClass *)klass; + + GST_LOG("klass %p", klass); + + parent_class->alloc = wg_allocator_alloc; + parent_class->free = wg_allocator_free; + root_class->finalize = wg_allocator_finalize; +} + +GstAllocator *wg_allocator_create(void) +{ + return g_object_new(wg_allocator_get_type(), NULL); +} + +void wg_allocator_destroy(GstAllocator *gst_allocator) +{ + WgAllocator *allocator = (WgAllocator *)gst_allocator; + + GST_LOG("allocator %p", allocator); + + g_object_unref(allocator); + + GST_INFO("Destroyed buffer allocator %p", allocator); +} diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index fb852b4cf3d..c87536e5fbb 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -47,6 +47,7 @@ GST_DEBUG_CATEGORY_EXTERN(wine); struct wg_transform { GstElement *container; + GstAllocator *allocator; GstPad *my_src, *my_sink; GstPad *their_sink, *their_src; GstSegment segment; @@ -152,6 +153,7 @@ static gboolean transform_sink_query_cb(GstPad *pad, GstObject *parent, GstQuery gst_buffer_pool_config_set_params(config, caps, info.size, 0, 0); + gst_buffer_pool_config_set_allocator(config, transform->allocator, NULL); if (!gst_buffer_pool_set_config(pool, config)) GST_ERROR("Failed to set pool %p config.", pool); } @@ -161,9 +163,10 @@ static gboolean transform_sink_query_cb(GstPad *pad, GstObject *parent, GstQuery GST_ERROR("Pool %p failed to activate.", pool); gst_query_add_allocation_pool(query, pool, info.size, 0, 0); + gst_query_add_allocation_param(query, transform->allocator, NULL); - GST_INFO("Proposing pool %p, buffer size %#zx, for query %p.", - pool, info.size, query); + GST_INFO("Proposing pool %p, buffer size %#zx, allocator %p, for query %p.", + pool, info.size, transform->allocator, query); g_object_unref(pool); return true; @@ -221,6 +224,7 @@ NTSTATUS wg_transform_destroy(void *args) while ((sample = gst_atomic_queue_pop(transform->output_queue))) gst_sample_unref(sample); + wg_allocator_destroy(transform->allocator); g_object_unref(transform->their_sink); g_object_unref(transform->their_src); g_object_unref(transform->container); @@ -325,6 +329,8 @@ NTSTATUS wg_transform_create(void *args) goto out; if (!(transform->output_queue = gst_atomic_queue_new(8))) goto out; + if (!(transform->allocator = wg_allocator_create())) + goto out; transform->input_max_length = 1; transform->output_plane_align = 0; @@ -481,6 +487,8 @@ out: gst_object_unref(transform->my_src); if (src_caps) gst_caps_unref(src_caps); + if (transform->allocator) + wg_allocator_destroy(transform->allocator); if (transform->output_queue) gst_atomic_queue_unref(transform->output_queue); if (transform->input) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/197

June 7, 2022

[PATCH 2/5] winegstreamer: Rename mf_(create|destroy)_wg_sample helpers.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/gst_private.h | 4 ++-- dlls/winegstreamer/h264_decoder.c | 10 +++++----- dlls/winegstreamer/mfplat.c | 4 ++-- dlls/winegstreamer/wma_decoder.c | 12 ++++++------ 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index ca7396ad5cd..ab89942847a 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -120,8 +120,8 @@ extern HRESULT mfplat_DllRegisterServer(void); IMFMediaType *mf_media_type_from_wg_format(const struct wg_format *format); void mf_media_type_to_wg_format(IMFMediaType *type, struct wg_format *format); -HRESULT mf_create_wg_sample(IMFSample *sample, struct wg_sample **out); -void mf_destroy_wg_sample(struct wg_sample *wg_sample); +HRESULT wg_sample_create_mf(IMFSample *sample, struct wg_sample **out); +void wg_sample_release(struct wg_sample *wg_sample); HRESULT wg_transform_push_mf(struct wg_transform *transform, struct wg_sample *sample); HRESULT wg_transform_read_mf(struct wg_transform *transform, struct wg_sample *sample, diff --git a/dlls/winegstreamer/h264_decoder.c b/dlls/winegstreamer/h264_decoder.c index 0ee9084e12e..8d3de4355bb 100644 --- a/dlls/winegstreamer/h264_decoder.c +++ b/dlls/winegstreamer/h264_decoder.c @@ -540,11 +540,11 @@ static HRESULT WINAPI transform_ProcessInput(IMFTransform *iface, DWORD id, IMFS if (!decoder->wg_transform) return MF_E_TRANSFORM_TYPE_NOT_SET; - if (FAILED(hr = mf_create_wg_sample(sample, &wg_sample))) + if (FAILED(hr = wg_sample_create_mf(sample, &wg_sample))) return hr; hr = wg_transform_push_mf(decoder->wg_transform, wg_sample); - mf_destroy_wg_sample(wg_sample); + wg_sample_release(wg_sample); return hr; } @@ -573,17 +573,17 @@ static HRESULT WINAPI transform_ProcessOutput(IMFTransform *iface, DWORD flags, samples[0].dwStatus = 0; if (!samples[0].pSample) return E_INVALIDARG; - if (FAILED(hr = mf_create_wg_sample(samples[0].pSample, &wg_sample))) + if (FAILED(hr = wg_sample_create_mf(samples[0].pSample, &wg_sample))) return hr; if (wg_sample->max_size < info.cbSize) { - mf_destroy_wg_sample(wg_sample); + wg_sample_release(wg_sample); return MF_E_BUFFERTOOSMALL; } hr = wg_transform_read_mf(decoder->wg_transform, wg_sample, &wg_format); - mf_destroy_wg_sample(wg_sample); + wg_sample_release(wg_sample); if (hr == MF_E_TRANSFORM_STREAM_CHANGE) { diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index 026237bdf5d..28a3fc20ead 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -965,7 +965,7 @@ struct mf_sample struct wg_sample wg_sample; }; -HRESULT mf_create_wg_sample(IMFSample *sample, struct wg_sample **out) +HRESULT wg_sample_create_mf(IMFSample *sample, struct wg_sample **out) { DWORD current_length, max_length; struct mf_sample *mf_sample; @@ -995,7 +995,7 @@ out: return hr; } -void mf_destroy_wg_sample(struct wg_sample *wg_sample) +void wg_sample_release(struct wg_sample *wg_sample) { struct mf_sample *mf_sample = CONTAINING_RECORD(wg_sample, struct mf_sample, wg_sample); diff --git a/dlls/winegstreamer/wma_decoder.c b/dlls/winegstreamer/wma_decoder.c index fdb68328c3a..81285f5ad44 100644 --- a/dlls/winegstreamer/wma_decoder.c +++ b/dlls/winegstreamer/wma_decoder.c @@ -534,18 +534,18 @@ static HRESULT WINAPI transform_ProcessInput(IMFTransform *iface, DWORD id, IMFS if (FAILED(hr = IMFTransform_GetInputStreamInfo(iface, 0, &info))) return hr; - if (FAILED(hr = mf_create_wg_sample(sample, &wg_sample))) + if (FAILED(hr = wg_sample_create_mf(sample, &wg_sample))) return hr; /* WMA transform uses fixed size input samples and ignores samples with invalid sizes */ if (wg_sample->size % info.cbSize) { - mf_destroy_wg_sample(wg_sample); + wg_sample_release(wg_sample); return S_OK; } hr = wg_transform_push_mf(decoder->wg_transform, wg_sample); - mf_destroy_wg_sample(wg_sample); + wg_sample_release(wg_sample); return hr; } @@ -576,13 +576,13 @@ static HRESULT WINAPI transform_ProcessOutput(IMFTransform *iface, DWORD flags, return MF_E_TRANSFORM_NEED_MORE_INPUT; } - if (FAILED(hr = mf_create_wg_sample(samples[0].pSample, &wg_sample))) + if (FAILED(hr = wg_sample_create_mf(samples[0].pSample, &wg_sample))) return hr; wg_sample->size = 0; if (wg_sample->max_size < info.cbSize) { - mf_destroy_wg_sample(wg_sample); + wg_sample_release(wg_sample); return MF_E_BUFFERTOOSMALL; } @@ -592,7 +592,7 @@ static HRESULT WINAPI transform_ProcessOutput(IMFTransform *iface, DWORD flags, samples[0].dwStatus |= MFT_OUTPUT_DATA_BUFFER_INCOMPLETE; } - mf_destroy_wg_sample(wg_sample); + wg_sample_release(wg_sample); if (hr == MF_E_TRANSFORM_STREAM_CHANGE) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/197

June 7, 2022