On Fri Jun 24 06:22:15 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list: ``` On 6/23/22 12:12, Rémi Bernon wrote:
From: Rémi Bernon <rbernon(a)codeweavers.com>
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_transform.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index e05432f6ac7..b0048fad644 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -314,10 +314,15 @@ static struct wg_sample *transform_request_sample(gsize size, void *context)
GST_LOG("size %#zx, context %p", size, transform);
- sample = InterlockedExchangePointer((void **)&transform->output_wg_sample, NULL); - if (!sample || sample->max_size < size) + if (!(sample = InterlockedExchangePointer((void **)&transform->output_wg_sample, NULL))) return NULL;
+ if (sample->max_size < size) + { + InterlockedDecrement(&sample->refcount); + return NULL; + } + return sample; }
I'll sign off on this because it's an improvement over the current code, but on reflection I think this pattern is not very idiomatic. More idiomatic would be to protect the whole thing with a lock, and not set the pointer to NULL in this function (but instead add an extra reference). ``` I'm wary of adding locks, there's many threads involved in winegstreamer and it's hard to tell which combination of locks is safe to hold.
Then I've changed the code to use a default allocator callback and a new helper to provide samples one at a time, using the allocator lock only. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/302#note_3003