5 Jul
2022
5 Jul
'22
6:04 a.m.
On Mon Jul 4 21:34:22 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list:
```
On 6/30/22 07:33, Rémi Bernon wrote:
I don't think a fixed pool can work. We cannot expect the output buffers
passed downstream to the application to ever be released, or, even if they probably will,
I don't think we can easily and safely put them in back in the pool when they are.
1) We'd need a custom buffer implementation for each API.
2) Some API provide allocation callback which conflicts with 1) needs.
3) The allocation callbacks may block.
4) The buffers may be released after the pipeline has been shutdown, we'd need to keep
it alive until all the buffers are released, which is likely going to cause problems.
Another way would be to keep a list of all the allocated buffers, monitoring it for
released buffers, but that doesn't sound great either. Especially with the allocation
callbacks there's no way of telling whether the pipeline is the only owner.
I think the `wg_allocator` callback is overall simpler, even though there's
considerations to have too, especially with the blocking API callbacks, which need to be
solved without blocking the reading thread or other streams allocations.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/302#note_3305
From: Rémi Bernon
<rbernon(a)codeweavers.com>
Using the allocator lock and replacing the transform_request_sample
callback with a default wg_allocator callback.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
dlls/winegstreamer/unix_private.h | 2 ++
dlls/winegstreamer/wg_allocator.c | 42 +++++++++++++++++++++++++++++--
dlls/winegstreamer/wg_transform.c | 24 +++---------------
3 files changed, 46 insertions(+), 22 deletions(-)
This is making things even more complicated; now we have two entirely
different code paths to set samples.
I understand that adding more locks increases the mental burden, but I
think this is worse—sure, we're "reusing" a lock that was going to be
created anyway, but it's still effectively introducing another lock,
plus now this requires us to understand when GStreamer is going to
implicitly take that lock.
Not to mention that this is similar to what I was trying to propose,
enough that I'd like to revisit it. I really don't want to argue about
the way forward—there's been too much of that already—but I'm still
finding it hard to justify the complexity of this whole callback
infrastructure, when we could more simply set up a fixed-size pool
beforehand. Yes, we might run out of space in the pool and need to
allocate more sysmem buffers, but it should be easy enough to find a
pool size large enough that that doesn't happen in practice.
[Ideally it would take an application leaking an entire stream of a
muxed file—which has been known to happen—but in that case, it strikes
me as better to eat up the Unix address space instead of the win32
address space, so the limited size is actually a good thing.]
Or, failing that (or in addition to it?), maybe we could just not use
the same wg_allocator object for both transforms and parsers. Yes, it's
more gobject boilerplate, which is annoying, but wg_allocator is a
complex beast and I really don't want to make it worse. And when it
comes to output samples, the allocator and the transform really do seem
to have entirely different considerations.
```