RFC: implementation of driver functionality in msacm (RESEND)

[Alex Villací­s Lasso]

(resent because previous attempt never appeared in wine-devel)

This patch is the preliminary result of some work I have been doing in 
order to add missing functionality to builtin msacm32.dll. I am 
submitting this to wine-devel rather than wine-patches, and in one big 
patch rather than several because I would like comments on some choices 
I made while adding features. The following is the list of features 
added by the patch:

* Implementation of acmDriverAddW(), and delegation of acmDriverAddA() 
to acmDriverAddW()
* Working implementation of modes of operation of acmDriverAdd[AW]: add 
driver by new registry entry (ACM_DRIVERADDF_NAME), add (local) driver 
by combination of hModule/acmDriverProc (ACM_DRIVERADDF_FUNCTION), add 
notification window for event broadcasts (ACM_DRIVERADDF_NOTIFYHWND)
* Implementation of broadcasts to notification windows on driver 
add/remove, enabling/disabling, and priority changes
* Fix for implementation quirks of acmDriverMessage() in order to allow 
native codecs to display configuration dialogs
* Working implementation of acmDriverPriority(), with support of delayed 
notification broadcasts (for one process only). Includes saving new 
priorities and enabled status to registry
* Loading of codec priorities and enabled status from registry
* Support for ACM_METRIC_DRIVER_SUPPORT in acmMetrics()

I must note that in order to provide support for acmDriverAddW() in 
ACM_DRIVERADDF_FUNCTION mode, it is necessary to treat an 
application-supplied module with an application-supplied driverProc as a 
full-blown winmm driver. Therefore, the patch includes a new procedure 
in winmm called wineAddDriver(), that instructs winmm to build a hDrvr 
from a supplied hModule/driverProc pair, rather than loading both from a 
DLL, as OpenDriver() does. This allows the rest of the code to continue 
using SendDriverMessage() as usual.

Alex Villacís Lasso

[Eric Pouech]

Alex Villací­s Lasso wrote:
> (resent because previous attempt never appeared in wine-devel)
> 
> This patch is the preliminary result of some work I have been doing in 
> order to add missing functionality to builtin msacm32.dll. I am 
> submitting this to wine-devel rather than wine-patches, and in one big 
> patch rather than several because I would like comments on some choices 
> I made while adding features. 
Even for review it's easier by small chunks...
> The following is the list of features 
> added by the patch:
> 
> * Implementation of acmDriverAddW(), and delegation of acmDriverAddA() 
> to acmDriverAddW()
- you shouldn't compute the length of the necessary unicode buffer with 
strlen * sizeof(WCHAR). See rest of the code for good example
- when you free lParamW, lParamW can be another non null value (a window 
handle for example) and you shouldn't free it
> * Working implementation of modes of operation of acmDriverAdd[AW]: add 
> driver by new registry entry (ACM_DRIVERADDF_NAME), add (local) driver 
> by combination of hModule/acmDriverProc (ACM_DRIVERADDF_FUNCTION), add 
> notification window for event broadcasts (ACM_DRIVERADDF_NOTIFYHWND)
- I wonder if we should really (internally) register the nofication 
windows the same ways as the other drivers... this is only used for 
notification (one way). I'd rather use a separate internal type
> * Implementation of broadcasts to notification windows on driver 
> add/remove, enabling/disabling, and priority changes
- MSDN seems to state that differed notification is actually a counter, 
not a simple boolean (whereas enable/disable is a boolean)
> * Fix for implementation quirks of acmDriverMessage() in order to allow 
> native codecs to display configuration dialogs
this seems rather hackish. did you actually tested this on Windows ? 
Moreover, the size bits look especially suspicious. Where did you get 
the 16 value from ?
> * Working implementation of acmDriverPriority(), with support of delayed 
> notification broadcasts (for one process only). Includes saving new 
> priorities and enabled status to registry
> * Loading of codec priorities and enabled status from registry
> * Support for ACM_METRIC_DRIVER_SUPPORT in acmMetrics()
> 
> I must note that in order to provide support for acmDriverAddW() in 
> ACM_DRIVERADDF_FUNCTION mode, it is necessary to treat an 
> application-supplied module with an application-supplied driverProc as a 
> full-blown winmm driver. Therefore, the patch includes a new procedure 
> in winmm called wineAddDriver(), that instructs winmm to build a hDrvr 
> from a supplied hModule/driverProc pair, rather than loading both from a 
> DLL, as OpenDriver() does. This allows the rest of the code to continue 
> using SendDriverMessage() as usual.
this shouldn't be done that way, but rather by reimplementing 
senddrivermessage in msacm32

A+

-- 
Eric Pouech

[Alex Villací­s Lasso]

Eric Pouech wrote:
> > * Implementation of broadcasts to notification windows on driver 
> > add/remove, enabling/disabling, and priority changes
>
> - MSDN seems to state that differed notification is actually a 
> counter, not a simple boolean (whereas enable/disable is a boolean)

I have just read the MSDN web page, and I see no remark that suggests 
that deferred notification should behave as a counter instead of a 
simple on/off flag. Or maybe I am reading the page incorrectly...
> > * Fix for implementation quirks of acmDriverMessage() in order to 
> > allow native codecs to display configuration dialogs
>
> this seems rather hackish. did you actually tested this on Windows ? 
> Moreover, the size bits look especially suspicious. Where did you get 
> the 16 value from ?

I tested the native msacm32.dll from Windows 98 SE on Wine, and it 
reported a 16-byte struct size to the winemp3 codec. Since the goal was 
to allow native codecs no reason at all for not displaying the 
configuration dialog, I decided to use that size, even when the 
structure size in Wine is only 12 bytes.
> > * Working implementation of acmDriverPriority(), with support of 
> > delayed notification broadcasts (for one process only). Includes 
> > saving new priorities and enabled status to registry
> > * Loading of codec priorities and enabled status from registry
> > * Support for ACM_METRIC_DRIVER_SUPPORT in acmMetrics()
> >
> > I must note that in order to provide support for acmDriverAddW() in 
> > ACM_DRIVERADDF_FUNCTION mode, it is necessary to treat an 
> > application-supplied module with an application-supplied driverProc 
> > as a full-blown winmm driver. Therefore, the patch includes a new 
> > procedure in winmm called wineAddDriver(), that instructs winmm to 
> > build a hDrvr from a supplied hModule/driverProc pair, rather than 
> > loading both from a DLL, as OpenDriver() does. This allows the rest 
> > of the code to continue using SendDriverMessage() as usual.
>
> this shouldn't be done that way, but rather by reimplementing 
> senddrivermessage in msacm32

That was the very thing I didn't want to do. So, while we are at it, 
should it be reimplemented for all codecs, or just the ones supplied by 
the application? Native msacm32 seems to relay to winmm for registered 
codecs, since I can see calls to SendDriverMessage().

Alex Villacís Lasso