[PATCH v5 0/10] MR22: winegstreamer: Implement more sample attributes and timestamps.

[Rémi Bernon (@rbernon)]

On Fri May  6 17:46:13 2022 +0000, **** wrote:
> Zebediah Figura (she/her) replied on the mailing list:
> ```
> On 5/6/22 12:20, Rémi Bernon (@rbernon) wrote:
> >> Sure, then the question becomes, can we move the entire logic to the
> >> client side? That would require a separate call to retrieve the type of
> >> the current sample, given the below (or alternatively some extra queuing
> >> on the client side, but that seems more complex than necessary), but
> >> structurally it feels a lot less awkward.
> > 
> > It would only makes things more complicated, with an additional entry
> > point which you would have to call on every sample to get their format,
> > back to the client to compare them, and then only eventually read it.
> > 
> > I don't see how this is awkward at all, the client optionally provides
> > the format it believes the stream has and that it expects to be for the
> > output samples, the transform returns either with a read success and
> > unmodified format, or a format change result and the update format.
> > 
> > It's overall a dozen lines of code. Having an additional entry point
> > will be much larger change, with wrappers, etc.
> I've already tried to explain why this feels awkward to me. I'd rather 
> have two function calls that make sense structurally, than try to shove 
> everything into one call, regardless of how many lines it takes.
> (For that matter, we don't even need two function calls. With the 
> current state of things, all we need to do is pass a maximum size of 
> zero to the read_data function. It's not clear how that'll work with a 
> zero-copy infrastructure, but we could easily just add a "peek" flag.)
> > If at any point we need all this to be thread safe, because right now
> > it's definitely not and I hope we won't need it, the separate entry
> > point would make everything much more difficult whereas with this design
> > you could just lock the read_data and either return a format change or
> > read success atomically.
> I don't see how thread safety makes this any more complex?
> ```
> I've already tried to explain why this feels awkward to me. I'd rather
> have two function calls that make sense structurally, than try to shove
> everything into one call, regardless of how many lines it takes.


I really don't see how it make more sense "structurally" one way or the
other, it's imho only a matter of taste.


> (For that matter, we don't even need two function calls. With the
> current state of things, all we need to do is pass a maximum size of
> zero to the read_data function. It's not clear how that'll work with a
> zero-copy infrastructure, but we could easily just add a "peek" flag.) 


Yes, and as I already have done some investigation and implementation of
zero copy on the output side, I can say already that it will make things
much more complicated.

We cannot hold on the samples that were provided by the client, so
having a single unix entry point that is supposed to atomically take a
buffer, checks the expected format, eventually uses it to decode, and
return it to the client either filled or empty with the new format, is
going to be much easier to work with.

Output data is generally going to be produced, synchronously, when we
push the input buffers to the pipeline, from the gst_pad_push_list call.
To achieve zero copy, we need to prepare the output buffer beforehand,
knowing its expected format, put it in a pool that we propose to the
decoder as a reply to its allocation query.

A caps change can also happen during that call, and in which case we
will have to release the buffer instead, as it would not be compatible
anymore, but also because we don't want it to be used and return the
format change instead, allocate a unix one for the decoder and return
from the call.

Only after that call, we can tell whether format has changed, and if not
if our buffer has been used or if we need to copy from the output sample
buffer instead.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/22#note_732