[PATCH v2 3/4] winegstreamer: Only seek if it was requested by the caller.

[Zebediah Figura (she/her)]

On 6/11/21 4:10 PM, Zebediah Figura (she/her) wrote:
> On 6/11/21 5:53 AM, Giovanni Mascellani wrote:
>> Signed-off-by: Giovanni Mascellani <gmascellani at codeweavers.com>
>> ---
>>    dlls/winegstreamer/media_source.c | 5 +++--
>>    1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c
>> index 3e1e3001dc7..9188a350305 100644
>> --- a/dlls/winegstreamer/media_source.c
>> +++ b/dlls/winegstreamer/media_source.c
>> @@ -324,8 +324,9 @@ static void start_pipeline(struct media_source *source, struct source_async_comm
>>    
>>        source->state = SOURCE_RUNNING;
>>    
>> -    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_I8)
>> +        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);
>>    }
>>    
>>
> 
> I don't think this works reliably as-is. The API is a little awkward and
> doesn't communicate this clearly (sorry), but if GStreamer gives us a
> sample between wg_parser_stream_enable() and wg_parser_end_flush(),
> we'll return GST_FLOW_FLUSHING, which will generally cause the upstream
> element to stop sending data. The seek event is necessary to tell it to
> start sending data again.
> 
> This situation is probably more than a little undesirable. We probably
> need to modify the interface and backend to prevent any samples from
> getting lost (and, ideally, to avoid seeking the GStreamer backend if we
> don't need to).

Urgh, I confused myself, this isn't actually how it works. 
wg_parser_begin_flush() actually only exists to unblock a thread stuck 
in wg_parser_stream_get_event(). Unfortunately I named two different 
variables "flushing" :-(

So I think the patch should be fine as-is.

> What happens if you change which streams are selected without seeking?
> Currently the WG backend discards any events/buffers on disabled
> streams, which means that in general they won't be synchronized and we
> may end up throwing away an entire stream. My guess is that this is
> fine, because as far as I can tell mfplat doesn't actually make any
> synchronization guarantees even among selected streams, but I'd like to
> make sure...

I am still curious about this part, though.