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

[Giovanni Mascellani]

Hi,

Il 10/06/21 18:59, Nikolay Sivov ha scritto:
> On 6/10/21 7:25 PM, Zebediah Figura (she/her) wrote:
>> I agree with Derek's comment here. Assuming that buffering samples
>> like this is the right thing to do, I also wonder:
>>
>> * should we buffer samples, or requests?
>>
>> * should the samples/requests be stored in a flat array instead of a
>> linked list? Based on usage patterns it seems that'd be easier and
>> more efficient.
>>
> Agreed, list vs array is not really important one. Why do we need to
> buffer samples, is that to cover for discrepancy for already issued
> requests before Pause() was called? And what should really happen there,
> if we queue async command on RequestSample() (which is already
> questionable) any number of times, and then call Pause(), or Stop(), we
> should have a way to drop pending requests. And that's not possible with
> MFPutWorkItem().
> 
> With this patch any Start() when paused results in draining all the
> samples to the receiver, and that's not obviously correct, unless that
> was somehow tested. I imagine if you seek to a new position in either
> direction, you're not really interested in old samples. If session
> currently does not handle this correctly (by resetting expected samples
> counters, etc), we should figure out how to test it separately.

We need to somehow remember how many times RequestSample was called 
during pause, because when we start again we have to emit a 
corresponding number of samples. Windows does that.

As for whether we need to buffer requests or samples, I will check 
tomorrow, as I already discussed replying to other reviews.

>> -    unix_funcs->wg_parser_stream_seek(source->streams[0]->wg_stream, 1.0,
>> -            position->hVal.QuadPart, 0, AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning);
>> +    if (position->vt != VT_EMPTY)
>> +        unix_funcs->wg_parser_stream_seek(source->streams[0]->wg_stream, 1.0,
>> +                position->hVal.QuadPart, 0, AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning);
>>       unix_funcs->wg_parser_end_flush(source->wg_parser);
> Is this really relevant to Pause() ?

No, you're right. I will move it to another patch.

>> @@ -318,6 +345,8 @@ static void start_pipeline(struct media_source *source, struct source_async_comm
>>   
>>               IMFMediaEventQueue_QueueEventParamVar(stream->event_queue,
>>                   seek_message ? MEStreamSeeked : MEStreamStarted, &GUID_NULL, S_OK, position);
>> +
>> +            flush_pending_queue(stream, TRUE);
>>           }
>>       }
>>   
> This will send old samples before MESourceStarted/MESourceSeeked, which
> is questionable.

As I asked to Derek, is it relevant to compare which event comes first 
from two different queues? I see no indication that two different queues 
should be somehow synchronized. Even if queued MESourceStarted before, 
the application could receive it later.

Thanks, Giovanni.