[PATCH v2 1/2] quartz: Cache IMediaSeeking for filters.
Gabriel Ivăncescu
gabrielopcode at gmail.com
Tue Apr 7 09:17:33 CDT 2020
On 07/04/2020 16:47, Nikolay Sivov wrote:
>
>
> On 4/7/20 4:05 PM, Gabriel Ivăncescu wrote:
>> -static BOOL is_renderer(IBaseFilter *filter)
>> +static BOOL is_renderer(struct filter *filter)
>> {
>> IAMFilterMiscFlags *flags;
>> - IMediaSeeking *seeking;
>> BOOL ret = FALSE;
>> - if (SUCCEEDED(IBaseFilter_QueryInterface(filter,
>> &IID_IAMFilterMiscFlags, (void **)&flags)))
>> + if (SUCCEEDED(IBaseFilter_QueryInterface(filter->filter,
>> &IID_IAMFilterMiscFlags, (void **)&flags)))
>> {
>> if (IAMFilterMiscFlags_GetMiscFlags(flags) &
>> AM_FILTER_MISC_FLAGS_IS_RENDERER)
>> ret = TRUE;
>> IAMFilterMiscFlags_Release(flags);
>> }
>> - else if (SUCCEEDED(IBaseFilter_QueryInterface(filter,
>> &IID_IMediaSeeking, (void **)&seeking)))
>> - {
>> - IMediaSeeking_Release(seeking);
>> - if (!has_output_pins(filter))
>> - ret = TRUE;
>> - }
>> + else if (get_filter_seeking(filter) &&
>> !has_output_pins(filter->filter))
>> + ret = TRUE;
>> return ret;
>> }
>> @@ -675,12 +680,13 @@ static HRESULT WINAPI
>> FilterGraph2_AddFilter(IFilterGraph2 *iface,
>> }
>> IBaseFilter_AddRef(entry->filter = filter);
>> + entry->seeking = NULL;
>> list_add_head(&graph->filters, &entry->entry);
>> list_add_head(&graph->sorted_filters, &entry->sorted_entry);
>> entry->sorting = FALSE;
>> ++graph->version;
>> - if (is_renderer(filter))
>> + if (is_renderer(entry))
>> ++graph->nRenderers;
>
> Helper name does not imply important state change, will it work if you
> queried once right after IBaseFilter_AddRef()? This looks like the only
> place when
> filter is added to the list. After that you can simply make
> is_renderer() check for null pointer.
>
Hi Nikolay,
I guess it would work for this particular case, but it would be
different from how Windows does it, so I'd rather not. I don't think the
state change is concerning, because it should be transparent, the rest
of the code shouldn't have to be cluttered with such corner case IMO. I
believe it should be enough that I commented on the helper why it's done
the way it is (plus the tests on next patch).
Note that if I do it on AddRef, there's a possibility of adding
side-effects if the MediaSeeking would otherwise not be used at all in
another app, for example. In this case it would be queried and
eventually released, which might introduce other bugs (because
currently, it isn't, and neither on Windows). I personally don't think
it's worth it.
Thanks,
Gabriel
More information about the wine-devel
mailing list