[PATCH v5 3/5] winegstreamer: Introduce a new wg_allocator_set_next_sample helper.

[Zebediah Figura]

On 6/30/22 07:33, Rémi Bernon wrote:
> From: Rémi Bernon <rbernon at 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 at 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.