[PATCH 0/5] MR61: winegstreamer: Dynamic transform stream format change support.

[Zebediah Figura]

On 5/12/22 12:12, Rémi Bernon (@rbernon) wrote:
> On Thu May 12 16:42:57 2022 +0000, **** wrote:
>> Zebediah Figura replied on the mailing list:
>> ```
>> On 5/12/22 03:15, Rémi Bernon (@rbernon) wrote:
>>> Compared to last attempt, I moved the format pointer to a dedicated
>> `wg_transform_read_data` parameter instead of a property of the sample.
>> Most of the transforms use `NULL` there as they don't expect or support
>> format change. H264 decoder passes a transform format member where it
>> tracks the current stream format.
>>>
>> I'm sorry, but I still don't see why the wg_transform object needs to
>> store the previous format. It may not make the API any more complex to
>> you, but conceptually I find it more complex, and I'd rather avoid
>> maintaining that complexity.
>> What is it that makes "peeking" for a sample untenable, or buffering it
>> on the client side?
>> ```
>> I'm sorry, but I still don't see why the wg_transform object needs to
>> store the previous format. It may not make the API any more complex to
>> you, but conceptually I find it more complex, and I'd rather avoid
>> maintaining that complexity.
>>
> 
> The `wg_transform` doesn't store the current format, it keeps the
> format of each buffer it queues.

I'm sorry, that was an error on my part. I should have said not "store 
the previous format" but "know the previous format".

> 
> 
>> What is it that makes "peeking" for a sample untenable, or buffering it
>> on the client side?
>>
> 
> There's no really such thing as "peeking" when `read_data` also pushes
> the input buffers, and when output presense also depends on it. Unless
> of course you consider that peeking should do that first, in which case
> it's not really just peeking.
> 
> Then, like I previously described, to be able to implement zero copy,
> we will need to have a buffer ready for the decoder to write to, before
> we generate any output, and so before we push the input data.
> 
> The buffer we will want to write to is the buffer that the MF transform
> user has given us, and we shouldn't write to it if the buffer format
> changes. In that particular case, yes, we will have to buffer the data
> instead, but that's the only case where we want to do it.
> 
> If "peeking" does everything because it has to (prepares output buffer,
> pushes input data, check whether output has been produced or if format
> has changed), it's not really just peeking anymore and it's exactly and
> everything that `read_data` is going to do.
> 

I guess "peek" implies that it writes the output data without removing 
it, so maybe that's a failure of language on my part. I don't 
necessarily mean we need to write the output data. We just need a way to 
retrieve *only* the caps (possibly with other metadata), without 
dequeuing anything else.

Granted, you can't implement a "peek" operation with zero-copy, either, 
but you also don't get "read" with quite the same semantics. At the 
moment it's hard for me to say what the zero-copy API should look like.

If nothing else it can be made into a separate syscall, like 
"get_format". Yes, having a separate syscall means more glue, but like I 
said, I'd rather have more syscalls with a clear and idiomatic 
organization, than less syscalls with a monolithic one.