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

[Giovanni Mascellani]

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.

> - 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?

>> -    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.