[PATCH 2/3] winegstreamer: Use decodebin to initialize media streams.

[Zebediah Figura]

On 9/11/20 12:23 PM, Derek Lesho wrote:
> 
> On 9/11/20 11:24 AM, Zebediah Figura wrote:
>> On 9/11/20 10:18 AM, Derek Lesho wrote:
>>> On 9/10/20 6:43 PM, Zebediah Figura wrote:
>>>
>>>> I got the impression from some past communication that the reason for
>>>> using appsink in the first place (instead of just a solitary sink pad)
>>>> is so that we can buffer, or is there some other reason?
>>> Well, buffering is necessary, as media streams operate in a sort of
>>> pull/push mode where we can only send them samples if they have
>>> requested one with IMFMediaStream::RequestSample.
>> Sure, but it also seems potentially simple enough to just wait on a
>> semaphore in the chain function.
> It's definitely more complicated for a system like that (I used to have 
> it that way).  With appsink, we just insert the request sample call into 
> a command queue, and pull the buffer from the appsink in the async 
> command.  The command queue is completely synchronous so we don't have 
> to worry about stuff like seeking or stopping interfering.  With the 
> semaphore solution, I'm not sure how much we have to worry about those 
> cases, but I remember having all sorts of strange bugs.

Could you please describe in more detail the problems you encountered?
To be sure you'd need an extra event in there to handle flushes, but
that doesn't seem like a deal-breaker to me either.

>>   If it makes sense (for latency reasons)
>> to buffer more than that, then there's a potential argument for appsink
>> (but also, perhaps, just an argument for appending a queue element after
>> each stream). I don't know/remember the relevant details for Media
>> Foundation timing.
>>
>>> Also, appsink is just
>>> more convenient than manually setting up a pad, I've also considered
>>> changing the source pad to appsrc, but if it isn't broken....
>>>
>> Is it, though? I don't know what all the things you need to hook up are,
>> but it looks to me like you essentially have to set up the same
>> callbacks
> 
> We don't set up any callbacks for appsink, just pull a buffer when we 
> need one. 

Well, you do, actually, to get current caps (and, eventually, for caps
negotiation). appsink does save you a chain function, but I don't
immediately see anything else that it saves you.

> And given how much boilerplate and how many naming problems 
> callbacks cause in winegstreamer, it's probably better this way.

Wiring up GStreamer callbacks is annoying, yes, but that annoyance may
be worthwhile. In particular adding another callback doesn't actually
increase code complexity; it's just copying an existing pattern.

winegstreamer has some bad names, as I've complained in this thread, but
that's a preëxisting problem that wouldn't be exacerbated by adding a
chain function.

> 
> https://github.com/Guy1524/wine/commit/366946876d7a53f80d91749f290f511294415cf5#diff-2873f4b2d2de6a23dbc67b2960e28f88
> 
>> , just in a different form. In the case of caps it makes things
>> more than a little uglier
> What does, appsink or a custom pad?  With appsink all you need to do is 
> set the desired caps property.
>> , especially if you later end up doing more
>> complicated caps negotiation. Note also that GstPad has default handling
>> of EOS events and GST_PAD_IS_EOS(), which I'm inclined to understand is
>> more useful for MF than handling GST_EVENT_EOS directly.
> I'm not sure what you're referring to here.  All we need to do in terms 
> of end of stream is read the "eos" property on the appsink during the 
> RequestSample-derived command, and dispatch the relevant events.

Yes, my point is that it's not really any different for GstPad; quartz
waits for GST_EVENT_EOS but that's only because it matches quartz's
threading model much better.


To be quite clear, I'm not trying to argue that appsink shouldn't be
used here; I'm just trying to understand all the potential factors
behind the decision, and essentially stress-test it. So far as I see
appsink provides a built-in buffer that matches mfplat's model nicely,
which is good, but it also forces the use of probes and thereby some
tricky parameter passing, which is not good.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200914/44e770aa/attachment.sig>