Re: [QUARTZ] Add implementation for IFilterMapper interface
Christian Costa wrote:
Hi,
This patch adds implementation for the IFilterMapper interface.
Nice. I have few comments (see below).
However filters registered with this interface require some work in devenum to be seen from application. This will be done when time permits.
Ok. I had assumed devenum was complete, but obviously not. Does it spit out any fixme's? ...
@@ -879,7 +893,7 @@ if (SUCCEEDED(hrSub)) hrSub = ICreateDevEnum_CreateClassEnumerator(pCreateDevEnum, &clsidCat, &pEnum, 0);
- if (SUCCEEDED(hrSub)) + if (SUCCEEDED(hrSub) && (hrSub == S_OK))
You can remove the SUCCEEDED(hrSub) completely here. ...
+ if (SUCCEEDED(hrSub)) + { + len = (strlenW((WCHAR*)&V_UNION(&var, bstrVal))+1) * sizeof(WCHAR); + if (!(regfilters[idx].Name = CoTaskMemAlloc(len*2))) + hr = E_OUTOFMEMORY; + } + + if (SUCCEEDED(hrSub)) + { + memcpy(regfilters[idx].Name, &V_UNION(&var, bstrVal), len); + regfilters[idx].Clsid = clsid; + idx++; + }
Indentation messed up a bit here and many other places. ...
+ if (cFetched > 0) + { + for(i = 0; i < cFetched; i++) { + /* The string in the REGFILTER structure must be allocated in the same block as the REGFILTER structure itself */ + ppRegFilter[i] = (REGFILTER*)CoTaskMemAlloc(sizeof(REGFILTER)+(strlenW(This->RegFil ters[i].Name)+1)*sizeof(WCHAR)); + if (!ppRegFilter[i]) { + while(i) { + CoTaskMemFree(ppRegFilter[--i]); + ppRegFilter[i] = NULL; + } + return E_OUTOFMEMORY; + } + ppRegFilter[i]->Clsid = This->RegFilters[i].Clsid; + ppRegFilter[i]->Name = (WCHAR*)((char*)ppRegFilter[i]+sizeof(REGFILTER)); + CopyMemory(ppRegFilter[i]->Name, This->RegFilters[i].Name, (strlenW(This->RegFilters[i].Name)+1)*sizeof(WCHAR)); + }
Please choose one style of bracket placement and stick to it (this occurs in a few places too). Other than these things, it looks fine. Rob
Robert Shearman wrote:
Christian Costa wrote:
Hi,
This patch adds implementation for the IFilterMapper interface.
Nice. I have few comments (see below).
However filters registered with this interface require some work in devenum to be seen from application. This will be done when time permits.
Ok. I had assumed devenum was complete, but obviously not. Does it spit out any fixme's?
No, it doesn't. In fact, Devenum enumerates entries in: 1) HROOT\CLSID\{"clsid of categories"]\Instance 2) HCURRENTUSER\Software\Microsoft\ActiveMovie\devenum For Direct Show filters, Devenum retreives filters registered with IFilterMapper and recreates entries in ....\devenum so when enumerating filters we get the IFilterMapper2 filters followed by IFilterMapper ones. I think this is the same for ICM and ACM filters. The current code only do 1 or 2 but not both at the same time.
...
@@ -879,7 +893,7 @@ if (SUCCEEDED(hrSub)) hrSub = ICreateDevEnum_CreateClassEnumerator(pCreateDevEnum, &clsidCat, &pEnum, 0);
- if (SUCCEEDED(hrSub)) + if (SUCCEEDED(hrSub) && (hrSub == S_OK))
You can remove the SUCCEEDED(hrSub) completely here.
...
He! Right! :-)
+ if (SUCCEEDED(hrSub)) + { + len = (strlenW((WCHAR*)&V_UNION(&var, bstrVal))+1) * sizeof(WCHAR); + if (!(regfilters[idx].Name = CoTaskMemAlloc(len*2))) + hr = E_OUTOFMEMORY; + } + + if (SUCCEEDED(hrSub)) + { + memcpy(regfilters[idx].Name, &V_UNION(&var, bstrVal), len); + regfilters[idx].Clsid = clsid; + idx++; + }
Indentation messed up a bit here and many other places.
Ok! I will fix that.
...
+ if (cFetched > 0) + { + for(i = 0; i < cFetched; i++) { + /* The string in the REGFILTER structure must be allocated in the same block as the REGFILTER structure itself */ + ppRegFilter[i] = (REGFILTER*)CoTaskMemAlloc(sizeof(REGFILTER)+(strlenW(This->RegFil ters[i].Name)+1)*sizeof(WCHAR)); + if (!ppRegFilter[i]) { + while(i) { + CoTaskMemFree(ppRegFilter[--i]); + ppRegFilter[i] = NULL; + } + return E_OUTOFMEMORY; + } + ppRegFilter[i]->Clsid = This->RegFilters[i].Clsid; + ppRegFilter[i]->Name = (WCHAR*)((char*)ppRegFilter[i]+sizeof(REGFILTER)); + CopyMemory(ppRegFilter[i]->Name, This->RegFilters[i].Name, (strlenW(This->RegFilters[i].Name)+1)*sizeof(WCHAR)); + }
Please choose one style of bracket placement and stick to it (this occurs in a few places too).
I tried to stick to your style. It seems I forgot some brackets. :-)
Other than these things, it looks fine.
Rob
Fine. Thanks Rob, Christian
Christian Costa wrote:
Ok. I had assumed devenum was complete, but obviously not. Does it spit out any fixme's?
No, it doesn't.
In fact, Devenum enumerates entries in: 1) HROOT\CLSID\{"clsid of categories"]\Instance 2) HCURRENTUSER\Software\Microsoft\ActiveMovie\devenum
For Direct Show filters, Devenum retreives filters registered with IFilterMapper and recreates entries in ....\devenum so when enumerating filters we get the IFilterMapper2 filters followed by IFilterMapper ones. I think this is the same for ICM and ACM filters.
The current code only do 1 or 2 but not both at the same time.
That makes a lot of sense. If I get some time I will implement this. Thanks for the new patch, it looks good. Rob
Robert Shearman wrote:
Christian Costa wrote:
Ok. I had assumed devenum was complete, but obviously not. Does
it spit out
any fixme's?
No, it doesn't.
In fact, Devenum enumerates entries in: 1) HROOT\CLSID\{"clsid of categories"]\Instance 2) HCURRENTUSER\Software\Microsoft\ActiveMovie\devenum
For Direct Show filters, Devenum retreives filters registered with IFilterMapper and recreates entries in ....\devenum so when enumerating filters we get the IFilterMapper2 filters followed by IFilterMapper ones. I think this is the same for ICM and ACM filters.
The current code only do 1 or 2 but not both at the same time.
That makes a lot of sense. If I get some time I will implement this.
Fine. :-) BTW, Windows seems to only update the devenum entries, if any, of the category eumerated. I don't think this is significant or not though... Bye, Christian
Christian Costa wrote:
Fine. :-) BTW, Windows seems to only update the devenum entries, if any, of the category eumerated. I don't think this is significant or not though...
devenum entries are Just a cache that quartz is keeping. It is different between DirectShow versions. I have never figured out how/when it decides to refresh the cache but he does a good Job. So I suspect it is only an in-session cache that is refreshed every App load (first use). The main REG, that is standard, and that gets written by DllRegisterServer is the HKEY_CLASS_ROOT one.
Boaz Harrosh wrote:
Christian Costa wrote:
Fine. :-) BTW, Windows seems to only update the devenum entries, if any, of the category eumerated. I don't think this is significant or not though...
devenum entries are Just a cache that quartz is keeping. It is different between DirectShow versions. I have never figured out how/when it decides to refresh the cache but he does a good Job. So I suspect it is only an in-session cache that is refreshed every App load (first use).
The main REG, that is standard, and that gets written by DllRegisterServer is the HKEY_CLASS_ROOT one.
So it doesn't matter. Unless we want to mix different versions (native and builtin) of devenum.dll and quartz.dll... Thanks for these details. :-) Bye, Christian
participants (3)
-
Boaz Harrosh -
Christian Costa -
Robert Shearman