On Mon May 23 23:15:35 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list: ``` On 5/23/22 07:07, Rémi Bernon wrote:
+static HRESULT source_query_interface(struct strmbase_pin *iface, REFIID iid, void **out) +{ + struct asf_stream *stream = impl_from_strmbase_pin(iface); + + TRACE("iface %p, iid %s, out %p.\n", iface, debugstr_guid(iid), out); + + if (IsEqualGUID(iid, &IID_IUnknown) + || IsEqualGUID(iid, &IID_IPin)) + *out = &stream->source.pin.IPin_iface; + else + { + *out = NULL; + WARN("%s not implemented, returning E_NOINTERFACE.\n", debugstr_guid(iid)); + return E_NOINTERFACE; + } + + IUnknown_AddRef((IUnknown *)*out); + return S_OK; +} + No need to handle IUnknown or IPin here; strmbase will do that for us if we return E_NOINTERFACE. (And if we don't need to handle any other interfaces, we can just leave the query_interface callback NULL.) @@ -46,12 +80,29 @@ static inline struct asf_reader *impl_from_strmbase_filter(struct strmbase_filte
static struct strmbase_pin *asf_reader_get_pin(struct strmbase_filter *iface, unsigned int index) { - return NULL; + struct asf_reader *filter = impl_from_strmbase_filter(iface); + struct strmbase_pin *pin; + + TRACE("iface %p, index %u.\n", iface, index); + + if (index >= InterlockedOr(&filter->stream_count, 0)) pin = NULL; + else pin = &filter->streams[index].source.pin; InterlockedOr() looks suspicious. If we're going to be thread safe (and we should) we should just take the filter lock around IFileSourceFilter::Load(). [It's already implicitly taken in the get_pin() callback.] Please keep if and else bodies on a separate line. Early return would obviate the need for a local variable here.
InterlockedOr() looks suspicious. If we're going to be thread safe (and we should) we should just take the filter lock around IFileSourceFilter::Load(). [It's already implicitly taken in the get_pin() callback.]
I was wary of locking because WM reader callbacks are done while holding WM reader lock, and prevent any concurrent call to other WM reader methods. I had some patches to leave the lock when calling the callbacks but I'm not sure it's a good way to go. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/116#note_1140