[PATCH v2] winegstreamer: Do not block forever if EOS races with command queue.

[Nikolay Sivov]

On 2/23/22 00:15, Nikolay Sivov wrote:
>
>
> On 2/23/22 00:09, Nikolay Sivov wrote:
>>
>>
>> On 2/9/22 11:40, Giovanni Mascellani wrote:
>>> Currently, the winegstreamer media source checks for EOS when
>>> RequestSample() is called, but doesn't handle the cases when EOS
>>> is detected between the RequestSample() call and the moment when
>>> the request is popped from the command queue and serviced. This
>>> can result in the media source waiting forever for a sample and
>>> get stuck.
>>>
>>> This commit fixes the bug by adding a check for EOS in
>>> wait_for_event().
>>>
>>> This commit fixes Medieval Dynasty hanging on developer logos on
>>> the Steam Deck.
>>>
>>> Signed-off-by: Giovanni Mascellani <gmascellani at codeweavers.com>
>>> ---
>>>   dlls/winegstreamer/media_source.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/dlls/winegstreamer/media_source.c 
>>> b/dlls/winegstreamer/media_source.c
>>> index 85ec31d2498..ca8f92b07ea 100644
>>> --- a/dlls/winegstreamer/media_source.c
>>> +++ b/dlls/winegstreamer/media_source.c
>>> @@ -534,6 +534,12 @@ static void wait_on_sample(struct media_stream 
>>> *stream, IUnknown *token)
>>>         TRACE("%p, %p\n", stream, token);
>>>   +    if (stream->eos)
>>> +    {
>>> + IMFMediaEventQueue_QueueEventParamVar(stream->event_queue, 
>>> MEError, &GUID_NULL, MF_E_END_OF_STREAM, &empty_var);
>>> +        return;
>>> +    }
>>> +
>>>       for (;;)
>>>       {
>>>           if (!wg_parser_stream_get_event(stream->wg_stream, &event))
>> Sorry, I missed this one. What I don't understand about this is why 
>> wg_parser_stream_get_event() can't simply return for streams that 
>> reached eos. It seems it should be able to track such state 
>> differences. There is already eos flag in wg_parser_stream(), but I 
>> see it means something else, and is never reset (I think).
>>
>> Anyway, since there is no way to test this situation on Windows, at 
>> least for file-based sources that I tried, I think we should simply 
>> return without producing an event.
>>
> Let's have it with an event for now, I'll have to test how pipeline 
> should react to that, we currently ignore async errors.
>

I tested for Source Reader, and error event is ignored there, it won't 
break anything, but won't trigger MF_SOURCE_READERF_ERROR or 
MF_SOURCE_READERF_ENDOFSTREAM. In our case it will rely on MEEndStream 
being sent before MEError, otherwise it will wait forever. So at least 
for reader sending MEError here is unnecessary, will check the session next.

I also found additional bug that we don't handle failed MEMediaSample 
response properly (when it has error set and does not have attached 
sample), this should return READERF_ERROR flag and error code from the 
event as ReadSample() return code.