[PATCH v5 0/6] MR140: qasf: More ASF reader filter implementation.

[Zebediah Figura]

On 7/5/22 00:46, Rémi Bernon (@rbernon) wrote:
> On Mon Jul  4 20:26:47 2022 +0000, **** wrote:
>> Zebediah Figura replied on the mailing list:
>> ```
>> On 6/15/22 01:58, Rémi Bernon wrote:
>>> +static HRESULT asf_reader_init_stream(struct strmbase_filter *iface)
>>> +{
>>> +    struct asf_reader *filter = impl_from_strmbase_filter(iface);
>>> +    HRESULT hr;
>>> +    int i;
>>> +
>>> +    TRACE("iface %p\n", iface);
>>> +
>>> +    for (i = 0; i < filter->stream_count; ++i)
>>> +    {
>>> +        struct asf_stream *stream = filter->streams + i;
>>> +
>>> +        if (!stream->source.pin.peer)
>>> +            continue;
>>> +
>>> +        hr = IMemAllocator_Commit(stream->source.pAllocator);
>>> +        if (FAILED(hr))
>>> +        {
>>> +            WARN("Failed to commit stream %u allocator, hr %#lx\n",
>> i, hr);
>>> +            continue;
>>> +        }
>>> +
>>> +        hr = IPin_NewSegment(stream->source.pin.peer, 0, 0, 1);
>>> +        if (FAILED(hr))
>>> +        {
>>> +            WARN("Failed to start stream %u new segment, hr %#lx\n",
>> i, hr);
>>> +            continue;
>>> +        }
>>> +    }
>>> +
>>> +    return IWMReader_Start(filter->reader, 0, 0, 1, NULL);
>>> +}
>>> +
>>> +static HRESULT asf_reader_cleanup_stream(struct strmbase_filter *iface)
>>> +{
>>> +    struct asf_reader *filter = impl_from_strmbase_filter(iface);
>>> +    int i;
>>> +
>>> +    TRACE("iface %p\n", iface);
>> Shouldn't the reader be stopped here?
>> ```
> Maybe, but it causes all sort of deadlocks which I've stopped trying to fix, and in practice it seems to work without it.
> 
> The deadlock is because `WMReader_Stop` calling `stop_streaming` which needs to enter the streaming thread CS to stop it, and then waits for it.

We don't wait for the streaming thread while holding stream_cs, though. 
Do you mean the reader CS? If we're taking the reader CS from the stream 
thread, that's also a bug that'll need to be solved somehow.

> 
> It can either be that the streaming thread is blocked in `wm_reader_get_stream_sample`, which, for some reason may be waiting for samples which never come.

If that's happening there's a bug, I think. If we're EOS then 
wm_reader_get_stream_sample() should return immediately.

> 
> Or, that the streaming thread calls the callbacks before exiting, and that the callbacks need to enter the filter CS, which is currently held by asf_reader_cleanup_stream. For instance either because there's a sample that's been received, or because of the `WMT_END_OF_STREAMING` notification.

There's a (unfortunately as yet unwritten) rule that nothing from the 
streaming thread is allowed to ever grab the filter CS.

I failed to notice this during review the first time, but that means 
that patch 3/7 is broken. It shouldn't be necessary to take the filter 
CS, though, since pin connection state can't change while the filter is 
running.