[PATCH 1/2] winegstreamer: Implement pausing the media source.

[Derek Lesho]

On 6/10/21 3:17 PM, Giovanni Mascellani wrote:
> Hi,
>
> Il 10/06/21 18:12, Derek Lesho ha scritto:
>> This patch probably warrants a few tests. I would test at least
>
> In line of principle I would agree. The reason I am not submitting any 
> is that Nikolay told me on the chat, as I understand it, that for this 
> kind of patches he prefers tests to be left out-of-tree. Since he is 
> the one who usually reviews my MF patches, I am inclined to meet his 
> requests.
>
>> - what happens to ::RequestSample calls when a stream is paused.
>
> As soon as the source is restarted, a corresponding number of 
> MEMediaSample is generated.
>
> I assumed that this means that samples are generated anyway during 
> pause and are later emitted after Start, but as others have noted this 
> is not necessarily true: requests could be queued instead of samples. 
> The two cases differ for example if the Start includes a seek (if 
> requests are stored, then samples are emitted at the freshly seeked 
> location, otherwise they are emitted at the location of pausing). And 
> thinking again about it storing the requests seems more logical than 
> storing the samples. Tomorrow I'll check on Windows to see what's 
> happening there.
Yeah that would be really useful to see.  My impression is that, since 
on Windows the stream has a separate request queue, the simplest 
implementation would just stop processing requests on the stream queue 
while the stream is paused.  This wouldn't cause problems with the 
source receiving the start request, as it has its own queue and then 
upon starting again the stream would run through its saved requests.
>
>> - whether MESourcePaused or the MEStreamPaused events come first.
>
> Hmm, does this really make sense? They are emitted on two different 
> queues. Does MF guarantees order of delivery across different queues? 
> Even if one is submitted before the other, the system scheduler might 
> decide to execute in the opposite order, if it wakes up the other 
> thread first. Or is there a stronger guarantee?
You're right, never mind.
>
>>> - IMFMediaEventQueue_QueueEventParamUnk(stream->event_queue, 
>>> MEMediaSample,
>>> -            &GUID_NULL, S_OK, (IUnknown *)sample);
>>> +    if (stream->parent_source->state == SOURCE_PAUSED)
>>> +    {
>>> +        struct pending_sample *pending = malloc(sizeof(*pending));
>>> +        if (!pending)
>>> +        {
>>> +            ERR("Cannot allocate pending sample\n");
>>> +            goto out;
>>> +        }
>>> +        pending->sample = sample;
>>> +        sample = NULL;
>>> +        list_add_tail(&stream->pending_samples, &pending->entry);
>> Since our media source outputs uncompressed samples, if it's common 
>> behavior for an application to request a significant amount of 
>> samples while the source is paused, we might instead want to add a 
>> separate sample request queue.  (Lest we use too much memory)  I 
>> think this is what I was referring to in my old comment.
>
> More than the memory usage (which I doubt will be significant: after 
> all until now apparently even the usage of Pause was not that 
> pressing), I find concerning the observable behavior, as I said above. 
> I will check better.
>
> Thanks for the review, Giovanni.
>