On Tue Oct 18 05:03:29 2022 +0000, Zebediah Figura wrote:
There could perhaps be alternative ways of doing this, but I don't see
any reason for this specific change to be unnacceptable. It simplifies the interface, as there'll be less calls to be done for the same thing. No, not unacceptable, but my point is to ask about other ways of approaching the problem, when something doesn't seem like clearly the best way. I think there is value in having the winegstreamer unixlib interface, i.e. the API, be simple and minimal, to aid its adoption in other places. Adding the "NULL = stream with the least pts" option to that interface makes it more complex. (Of course, with the monster that is zero-copy, this may be a losing battle. Just recently someone was put off from using it in new code...) It also makes the wg_parser backend glue more complex, which is probably the worst place to put more complexity right now. On the other hand, this is not really that bad of a change, at least comparatively, and it doesn't affect any of the other frontends, so I'm not flat-out opposed to it—just trying to see if another approach, which may not have been considered, might end up being preferable.
The wg_parser_stream_get_buffer function will need to be changed
anyway, in order for sample allocation requests to be eventually returned in addition to buffer output requests. This can hardly be done differently as the allocation needs be be done from a specific stream thread and may also be blocking.
The whole series is at
https://gitlab.winehq.org/rbernon/wine/-/commits/wip/zerocopy/v2 if you want to have a look. I've only looked briefly at the branch, but this seems very complicated. Do we need to call back to the PE side to allocate? Could we pool instead (and fall back to copying if the pool is empty)? I also don't quite see how that's relevant to this patch.
I don't think a preallocated pool can work. Not with allocations callbacks like in wmvcore and quartz. Especially as these may block and as they are supposed to be called at specific times and in specific threads.
And yes I have considered and tried it, and some alternate approaches, and I believe the one implemented here is good, and that it is also not so complex. It brings wg_transform and wg_parser interfaces closer to each other and use the same zero copy primitives for both.
Just recently someone was put off from using it in new code...
I've missed that probably, in which context? Was the discussion public?