From: "Rémi Bernon (@rbernon)" Subject: Re: [PATCH v4 0/5] MR302: winegstreamer: Some wg_transform H264 fixes for Mortal Kombat 11 and Yakuza 4. Message-Id: Date: Thu, 30 Jun 2022 08:08:19 +0000 In-Reply-To: References: 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 > > > > Signed-off-by: Rémi Bernon > > --- > > 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