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.
- return pin; }