30 Jun
2022
30 Jun
'22
8:08 a.m.
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@codeweavers.com> > > Signed-off-by: Rémi Bernon <rbernon@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.