[ros-diffs] [silverblade] 19954: Cleanup of existing code, basic implementation of play/stop/resume states.

Cleanup of existing code, basic implementation of play/stop/resume
states.
Added: trunk/reactos/lib/wdmaud/README.TXT
Modified: trunk/reactos/lib/wdmaud/control.c
Modified: trunk/reactos/lib/wdmaud/devices.c
Modified: trunk/reactos/lib/wdmaud/kernel.c
Modified: trunk/reactos/lib/wdmaud/memtrack.c
Added: trunk/reactos/lib/wdmaud/midi.c
Modified: trunk/reactos/lib/wdmaud/threads.c
Modified: trunk/reactos/lib/wdmaud/user.c
Added: trunk/reactos/lib/wdmaud/wave.c
Deleted: trunk/reactos/lib/wdmaud/wavehdr.c
Modified: trunk/reactos/lib/wdmaud/wdmaud.h
Modified: trunk/reactos/lib/wdmaud/wdmaud.xml
  _____  

Added: trunk/reactos/lib/wdmaud/README.TXT
--- trunk/reactos/lib/wdmaud/README.TXT	2005-12-07 18:33:12 UTC (rev
19953)
+++ trunk/reactos/lib/wdmaud/README.TXT	2005-12-07 18:37:49 UTC (rev
19954)
@@ -0,0 +1,97 @@

+User-mode Multimedia Driver for WDM Audio
+-----------------------------------------
+
+USAGE
+-----
+This is a "drop-in" replacement for the Windows XP wdmaud.drv
component. To
+make use of it, you'll need to disable system file protection somehow
(easy
+way is to rename all *.cab files in the sub-folders of
C:\WINDOWS\DRIVER CACHE
+to something else, then delete or rename WDMAUD.DRV in both
C:\WINDOWS\SYSTEM32
+and C:\WINDOWS\SYSTEM32\DLLCACHE.)
+
+Now put the ReactOS wdmaud.drv in C:\WINDOWS\SYSTEM32. At some point,
you'll
+be asked to insert the Windows CD as some files are missing - cancel
this and
+choose "yes" when asked if you're sure. This is due to the system file
+protection not being able to have its own way.
+
+That should be all there is to it.
+
+
+IMPLEMENTATION/DEVELOPMENT NOTES
+--------------------------------
+The style of driver used here dates back to the days of Windows 3.1.
Not much
+has changed - the NT 4 Sound Blaster driver exported the same functions
and
+supported the same message codes as the traditional Windows 3.1
user-mode
+drivers did.
+
+But in XP, something strange happens with WDMAUD (which is why you
can't just
+put this next to the existing WDMAUD under a different name!)
+
+It appears that WINMM.DLL treats WDMAUD.DRV differently to other
drivers.
+
+Here's a summary of how things work differently:
+
+1)	It seems DRV_ENABLE and DRV_DISABLE are the only important
messages
+	processed by DriverProc. These open and close the kernel-mode
+	driver.
+
+2)	Each message handling function (aside from DriverProc) receives
a
+	DRVM_INIT message after the driver has been opened. The second
+	parameter of this is a pointer to a string containing a device
+	path (\\?\... format.) Returning zero seems to be the accepted
+	thing to do, in any case.
+
+	The purpose of this function (in our case) is to allow
WDMAUD.DRV
+	to let WDMAUD.SYS know which device we want to play with.
+
+	Presumably, this is called when new devices are added to the
system,
+	as well. I don't know if this is called once per hardware
device...
+
+3)	xxxx_GETNUMDEVS now has extra data passed to it! The first
+	parameter is a device path string - which will have been passed
+	to DRVM_INIT previously.
+
+4)	xxxx_GETDEVCAPS is a bit hazardous. The old set of parameters
were:
+	1 - Pointer to a capabilities structure (eg: WAVEOUTCAPS)
+	2 - Size of the above structure
+
+	But now, the parameters are:
+	1 - Pointer to a MDEVCAPSEX struct (which points to a regular
+	    capabilities structure)
+	2 - Device path string
+
+	So anything expecting the second parameter to be a size (for
copying
+	memory maybe) is in for a bit of a surprise there!
+
+The reason for the above changes is Plug and Play. It seems that the
extra
+functionality was added in Windows 98 (possibly 95 as well) to make it
+possible to hot-swap winmm-supported devices without requiring a
restart of
+whatever applications are using the devices.
+
+That's the theory, at least.
+
+
+TODO
+----
+Our WINMM.DLL will need hacking to make sure it can take into account
the
+preferential treatment given to WDMAUD.DRV
+
+I'm not sure if it'll work with it yet as there seems to be some
accomodation
+for the PnP DRVM_INIT and DRVM_EXIT messages, but whether or not winmm
will
+function correctly with this WDMAUD.DRV replacement is something that
remains
+to be seen.
+
+
+THANKS
+------
+Thanks to everyone who has encouraged me to continue developing the
audio
+system for ReactOS.
+
+In particular, I'd like to thank Alex Ionescu for the many hours of
assistance
+he has given me in figuring out how things are done.
+
+
+-
+
+Andrew Greenwood
+andrew.greenwood AT silverblade DOT co DOT uk
Property changes on: trunk/reactos/lib/wdmaud/README.TXT
___________________________________________________________________
Name: svn:executable
   + *
Name: eol-style
   + native
Property changes on: trunk/reactos/lib/wdmaud/TODO
___________________________________________________________________
Name: eol-style
   + native
Property changes on: trunk/reactos/lib/wdmaud/callbacks.c
___________________________________________________________________
Name: eol-style
   + native
  _____  

Modified: trunk/reactos/lib/wdmaud/control.c
--- trunk/reactos/lib/wdmaud/control.c	2005-12-07 18:33:12 UTC (rev
19953)
+++ trunk/reactos/lib/wdmaud/control.c	2005-12-07 18:37:49 UTC (rev
19954)
@@ -13,8 +13,13 @@

 #include "wdmaud.h"
 
 /*
-    TODO:
-    Make these work for the other device types!
+    StartDevice
+
+    Creates a completion thread for a device, sets the "is_running"
member
+    of the device state to "true", and tells the kernel device to start
+    processing audio/MIDI data.
+
+    Wave devices always start paused.
 */
 
 MMRESULT StartDevice(PWDMAUD_DEVICE_INFO device)
@@ -25,21 +30,165 @@
     result = ValidateDeviceInfoAndState(device);
 
     if ( result != MMSYSERR_NOERROR )
+    {
+        DPRINT1("Device info/state not valid\n");
         return result;
+    }
 
-    ioctl_code = device == WDMAUD_WAVE_IN ? IOCTL_WDMAUD_WAVE_IN_START
:
-                 device == WDMAUD_WAVE_OUT ?
IOCTL_WDMAUD_WAVE_OUT_START :
-                 0x0000;
+    ioctl_code =
+        IsWaveInDeviceType(device->type)  ? IOCTL_WDMAUD_WAVE_IN_START
:
+        IsWaveOutDeviceType(device->type) ? IOCTL_WDMAUD_WAVE_OUT_START
:
+        IsMidiInDeviceType(device->type)  ? IOCTL_WDMAUD_MIDI_IN_START
:
+        0x0000;
 
     ASSERT( ioctl_code );
+
+    result = CreateCompletionThread(device);
+
+    if ( MM_FAILURE( result ) )
+    {
+        DPRINT1("Failed to create completion thread\n");
+        return result;
+    }
+
+    device->state->is_running = TRUE;
+
+    result = CallKernelDevice(device, ioctl_code, 0, 0);
+
+    if ( MM_FAILURE( result ) )
+    {
+        DPRINT1("Audio could not be started\n");
+        return result;
+    }
+
+    if ( ! IsWaveDeviceType(device->type) )
+        device->state->is_paused = FALSE;
+
+    return result;
 }
 
 MMRESULT StopDevice(PWDMAUD_DEVICE_INFO device)
 {
+    MMRESULT result;
+    DWORD ioctl_code;
+
+    result = ValidateDeviceInfoAndState(device);
+
+    if ( result != MMSYSERR_NOERROR )
+    {
+        DPRINT1("Device info/state not valid\n");
+        return result;
+    }
+
+    ioctl_code =
+        IsWaveInDeviceType(device->type)  ? IOCTL_WDMAUD_WAVE_IN_STOP :
+        IsWaveOutDeviceType(device->type) ? IOCTL_WDMAUD_WAVE_OUT_STOP
:
+        IsMidiInDeviceType(device->type)  ? IOCTL_WDMAUD_MIDI_IN_STOP :
+        0x0000;
+
+    ASSERT( ioctl_code );
+
+    if ( IsMidiInDeviceType(device->type) )
+    {
+        EnterCriticalSection(device->state->device_queue_guard);
+
+        if ( ! device->state->is_running )
+        {
+            /* TODO: Free the MIDI data queue */
+        }
+        else
+        {
+            device->state->is_running = FALSE;
+        }
+
+        LeaveCriticalSection(device->state->device_queue_guard);
+    }
+    else /* wave device */
+    {
+        device->state->is_paused = TRUE;
+    }
+
+    result = CallKernelDevice(device, ioctl_code, 0, 0);
+
+    if ( MM_FAILURE( result ) )
+    {
+        DPRINT1("Audio could not be stopped\n");
+        return result;
+    }
+
+    if ( IsWaveDeviceType(device-type) )
+    {
+        device->state->is_paused = TRUE;
+    }
+    else    /* MIDI Device */
+    {
+        /* TODO: Destroy completion thread etc. */
+    }
+
+    return result;
 }
 
-MMRESULT PauseDevice(PWDMAUD_DEVICE_INFO device)
+MMRESULT ResetDevice(PWDMAUD_DEVICE_INFO device)
 {
+    MMRESULT result;
+    DWORD ioctl_code;
+
+    result = ValidateDeviceInfoAndState(device);
+
+    if ( result != MMSYSERR_NOERROR )
+    {
+        DPRINT1("Device info/state not valid\n");
+        return result;
+    }
+
+    ioctl_code =
+        IsWaveInDeviceType(device->type)  ? IOCTL_WDMAUD_WAVE_IN_RESET
:
+        IsWaveOutDeviceType(device->type) ? IOCTL_WDMAUD_WAVE_OUT_RESET
:
+        IsMidiInDeviceType(device->type)  ? IOCTL_WDMAUD_MIDI_IN_RESET
:
+        0x0000;
+
+    ASSERT( ioctl_code );
+
+    if ( IsMidiInDeviceType(device->type) )
+    {
+        EnterCriticalSection(device->state->device_queue_guard);
+
+        if ( ! device->state->is_running )
+        {
+            /* TODO: Free the MIDI data queue */
+        }
+        else
+        {
+            device->state->is_running = FALSE;
+        }
+
+        LeaveCriticalSection(device->state->device_queue_guard);
+    }
+
+    result = CallKernelDevice(device, ioctl_code, 0, 0);
+
+    if ( MM_FAILURE( result ) )
+    {
+        DPRINT1("Audio could not be reset\n");
+        return result;
+    }
+
+    if ( IsWaveDeviceType(device->type) )
+    {
+        if ( IsWaveInDeviceType(device->type) )
+            device->state->is_paused = TRUE;
+        else if ( IsWaveOutDeviceType(device->type) )
+            device->state->is_paused = FALSE;
+
+        /* TODO: Destroy completion thread + check ret val */
+    }
+    else /* MIDI input device */
+    {
+        /* TODO: Destroy completion thread + check ret val */
+        /* TODO - more stuff */
+    }
+
+    return result;
 }
 
 MMRESULT StopDeviceLooping(PWDMAUD_DEVICE_INFO device)
Property changes on: trunk/reactos/lib/wdmaud/control.c
___________________________________________________________________
Name: eol-style
   + native
  _____  

Modified: trunk/reactos/lib/wdmaud/devices.c
--- trunk/reactos/lib/wdmaud/devices.c	2005-12-07 18:33:12 UTC (rev
19953)
+++ trunk/reactos/lib/wdmaud/devices.c	2005-12-07 18:37:49 UTC (rev
19954)
@@ -17,9 +17,15 @@

 
 const char WDMAUD_DEVICE_INFO_SIG[4] = "WADI";
 const char WDMAUD_DEVICE_STATE_SIG[4] = "WADS";
-const char WDMAUD_NULL_SIGNATURE[4] = {0,0,0,0};
 
 
+/*
+    IsValidDevicePath
+
+    Just checks to see if the string containing the path to the device
path
+    (object) is a valid, readable string.
+*/
+
 BOOL IsValidDevicePath(WCHAR* path)
 {
     if (IsBadReadPtr(path, 1))  /* TODO: Replace with flags */
@@ -33,43 +39,93 @@
     return TRUE;
 }
 
-MMRESULT ValidateDeviceInfo(PWDMAUD_DEVICE_INFO device_info)
+/*
+    ValidateDeviceData
+
+    Checks that the memory pointed at by the device data pointer is
writable,
+    and that it has a valid signature.
+
+    If the "state" member isn't NULL, the state structure is also
validated
+    in the same way. If the "require_state" parameter is TRUE and the
"state"
+    member is NULL, an error code is returned. Otherwise the "state"
member
+    isn't validated and no error occurs.
+*/
+
+MMRESULT ValidateDeviceData(
+    PWDMAUD_DEVICE_INFO device,
+    BOOL require_state
+)
 {
-    if ( IsBadWritePtr(device_info, sizeof(WDMAUD_DEVICE_INFO)) )
+    if ( IsBadWritePtr(device, sizeof(WDMAUD_DEVICE_INFO)) )
+    {
+        DPRINT1("Device data structure not writable\n");
         return MMSYSERR_INVALPARAM;
+    }
 
-    if ( *device_info->signature != *WDMAUD_DEVICE_INFO_SIG )
+    if ( strncmp(device->signature, WDMAUD_DEVICE_INFO_SIG, 4) != 0 )
+    {
+        DPRINT1("Device signature is invalid\n");
         return MMSYSERR_INVALPARAM;
+    }
 
-    return MMSYSERR_NOERROR;
-}
+    if ( ! IsValidDeviceType(device->type) )
+    {
+        DPRINT1("Invalid device type\n");
+        return MMSYSERR_INVALPARAM;
+    }
 
-MMRESULT ValidateDeviceState(PWDMAUD_DEVICE_STATE state)
-{
-    if ( IsBadWritePtr(state, sizeof(WDMAUD_DEVICE_INFO)) )
+    if ( device->id > 100 )
+    {
+        DPRINT1("Device ID is out of range\n");
         return MMSYSERR_INVALPARAM;
+    }
 
-    if ( *state->signature != *WDMAUD_DEVICE_STATE_SIG )
+    /* Now we validate the device state (if present) */
+
+    if ( device->state )
+    {
+        if ( IsBadWritePtr(device->state, sizeof(WDMAUD_DEVICE_INFO)) )
+        {
+            DPRINT1("Device state structure not writable\n");
+            return MMSYSERR_INVALPARAM;
+        }
+
+        if ( strncmp(device->state->signature,
+                     WDMAUD_DEVICE_STATE_SIG,
+                     4) != 0 )
+        {
+            DPRINT1("Device state signature is invalid\n");
+            return MMSYSERR_INVALPARAM;
+        }
+
+        /* TODO: Validate state events */
+    }
+    else if ( require_state )
+    {
         return MMSYSERR_INVALPARAM;
+    }
 
     return MMSYSERR_NOERROR;
 }
 
+
 /*
     ValidateDeviceStateEvents should be used in conjunction with the
standard
-    state validation routine.
-*/
+    state validation routine (NOT on its own!)
 
+    FIXME: The tests are wrong
+*/
+/*
 MMRESULT ValidateDeviceStateEvents(PWDMAUD_DEVICE_STATE state)
 {
-    if ( ( (DWORD) state->exit_thread_event == 0x00000000 ) &&
+    if ( ( (DWORD) state->exit_thread_event != 0x00000000 ) &&
          ( (DWORD) state->exit_thread_event != 0x48484848 ) )
     {
         DPRINT1("Bad exit thread event\n");
         return MMSYSERR_INVALPARAM;
     }
 
-    if ( ( (DWORD) state->queue_event == 0x00000000 ) &&
+    if ( ( (DWORD) state->queue_event != 0x00000000 ) &&
          ( (DWORD) state->queue_event != 0x42424242 ) &&
          ( (DWORD) state->queue_event != 0x43434343 ) )
     {
@@ -79,28 +135,32 @@
 
     return MMSYSERR_NOERROR;
 }
+*/
 
-MMRESULT ValidateDeviceInfoAndState(PWDMAUD_DEVICE_INFO device_info)
-{
-    MMRESULT result;
+/*
+    CreateDeviceData
 
-    result = ValidateDeviceInfo(device_info);
+    This is a glorified memory allocation routine, which acts as a
primitive
+    constructor for a device data structure.
 
-    if ( result != MMSYSERR_NOERROR )
-        return result;
+    It validates the device path given, allocates memory for both the
device
+    data and the device state data, copies the signatures over and sets
the
+    device type accordingly.
 
-    result = ValidateDeviceState(device_info->state);
+    In some cases, a state structure isn't required, so the creation of
one can
+    be avoided by passing FALSE for the "with_state" parameter.
+*/
 
-    if ( result != MMSYSERR_NOERROR )
-        return result;
-
-    return MMSYSERR_NOERROR;
-}
-
-PWDMAUD_DEVICE_INFO CreateDeviceData(CHAR device_type, WCHAR*
device_path)
+PWDMAUD_DEVICE_INFO
+CreateDeviceData(
+    CHAR device_type,
+    DWORD device_id,
+    WCHAR* device_path,
+    BOOL with_state
+)
 {
-    HANDLE heap = 0;
-    PWDMAUD_DEVICE_INFO device_data = 0;
+    BOOL success = FALSE;
+    PWDMAUD_DEVICE_INFO device = 0;
     int path_size = 0;
 
     DPRINT("Creating device data for device type %d\n", (int)
device_type);
@@ -115,121 +175,119 @@
     path_size = (lstrlen(device_path) + 1) * sizeof(WCHAR);
     /* DPRINT("Size of path is %d\n", (int) path_size); */
 
-    heap = GetProcessHeap();
-
-    if ( ! heap )
-    {
-        DPRINT1("Couldn't get the process heap (error %d)\n",
-                (int) GetLastError());
-        goto cleanup;
-    }
-
-    DPRINT("Allocating %d bytes\n",
+    DPRINT("Allocating %d bytes for device data\n",
            path_size + sizeof(WDMAUD_DEVICE_INFO));
-/*
-    device_data = (PWDMAUD_DEVICE_INFO) HeapAlloc(heap,
-                                                  HEAP_ZERO_MEMORY,
-                                                  path_size +
sizeof(WDMAUD_DEVICE_INFO));
-*/
 
-    device_data = (PWDMAUD_DEVICE_INFO)
+    device = (PWDMAUD_DEVICE_INFO)
         AllocMem(path_size + sizeof(WDMAUD_DEVICE_INFO));
 
-    if ( ! device_data )
+    if ( ! device )
     {
         DPRINT1("Unable to allocate memory for device data (error
%d)\n",
                 (int) GetLastError());
         goto cleanup;
     }
 
-    DPRINT("Copying signature\n");
-    memcpy(device_data->signature, WDMAUD_DEVICE_INFO_SIG, 4);
+    /* Copy the signature and device path */
+    memcpy(device->signature, WDMAUD_DEVICE_INFO_SIG, 4);
+    lstrcpy(device->path, device_path);
 
-    DPRINT("Copying path (0x%x)\n", (int)device_path);
-    lstrcpy(device_data->path, device_path);
+    /* Initialize these common members */
+    device->id = device_id;
+    device->type = device_type;
 
-    device_data->type = device_type;
+    if ( with_state )
+    {
+        /* Allocate device state structure */
+        device->state = AllocMem(sizeof(WDMAUD_DEVICE_STATE));
 
+        if ( ! device->state )
+        {
+            DPRINT1("Couldn't allocate memory for device state (error
%d)\n",
+                    (int) GetLastError());
+            goto cleanup;
+        }
+
+        /* Copy the signature */
+        memcpy(device->state->signature, WDMAUD_DEVICE_STATE_SIG, 4);
+    }
+
+    success = TRUE;
+
     cleanup :
     {
-        /* No cleanup needed (no failures possible after allocation.)
*/
-        DPRINT("Performing cleanup\n");
+        if ( ! success )
+        {
+            if ( device )
+            {
+                if ( device->state )
+                {
+                    ZeroMemory(device->state->signature, 4);
+                    FreeMem(device->state);
+                }
 
-        return device_data;
+                ZeroMemory(device->signature, 4);
+                FreeMem(device);
+            }
+        }
+
+        return (success ? device : NULL);
     }
 }
 
+
 /*
-    CloneDeviceData
+    DeleteDeviceData
 
-    This isn't all that great... Maybe some macros would be better:
-    BEGIN_CLONING_STRUCT(source, target)
-        CLONE_MEMBER(member)
-    END_CLONING_STRUCT()
+    Blanks out the device and device state structures, and frees the
memory
+    associated with the structures.
 
-    The main problem is that sometimes we'll want to copy more than
-    the data presented here. I guess we could blindly copy EVERYTHING
-    but that'd be excessive.
+    TODO: Free critical sections / events if set?
 */
 
-PWDMAUD_DEVICE_INFO CloneDeviceData(PWDMAUD_DEVICE_INFO original)
+void DeleteDeviceData(PWDMAUD_DEVICE_INFO device_data)
 {
-    PWDMAUD_DEVICE_INFO clone = NULL;
+    DPRINT("Deleting device data\n");
 
-    if ( ValidateDeviceInfo(original) != MMSYSERR_NOERROR)
-    {
-        DPRINT1("Original device data was invalid\n");
-        return NULL;
-    }
+    ASSERT( device_data );
 
-    /* This will set the type and path, so we can forget about those */
-    clone = CreateDeviceData(original->type, original->path);
+    /* We don't really care if the structure is valid or not */
+    if ( ! device_data )
+        return;
 
-    if ( ! clone )
+    if ( device_data->state )
     {
-        DPRINT1("Clone creation failed\n");
-        return NULL;
-    }
+        /* We DON'T want these to be set - should we clean up? */
+        ASSERT ( ! device_data->state->device_queue_guard );
+        ASSERT ( ! device_data->state->queue_event );
+        ASSERT ( ! device_data->state->exit_thread_event );
 
-    clone->id = original->id;
-    clone->wave_handle = original->wave_handle; /* ok? */
+        /* Insert a cow (not sure if this is right or not) */
+        device_data->state->sample_size = 0xDEADBEEF;
 
-    /* TODO: Maybe we should copy some more? */
+        /* Overwrite the structure with zeroes and free it */
+        ZeroMemory(device_data->state, sizeof(WDMAUD_DEVICE_STATE));
+        FreeMem(device_data->state);
+    }
 
-    return clone;
+    /* Overwrite the structure with zeroes and free it */
+    ZeroMemory(device_data, sizeof(WDMAUD_DEVICE_INFO));
+    FreeMem(device_data);
 }
 
-void DeleteDeviceData(PWDMAUD_DEVICE_INFO device_data)
-{
-    HANDLE heap;
+/*
+    ModifyDevicePresence
 
-    ASSERT( device_data );
+    Use this to add or remove devices in the kernel-mode driver. If the
+    "adding" parameter is TRUE, the device is added, otherwise it is
removed.
 
-    /* Erase the signature to prevent any possible future mishaps */
-    *device_data->signature = *WDMAUD_NULL_SIGNATURE;
+    "device_type" is WDMAUD_WAVE_IN, WDMAUD_WAVE_OUT, etc...
 
-    heap = GetProcessHeap();
+    "device_path" specifies the NT object path of the device.
 
-    if ( ! heap )
-    {
-        DPRINT1("Couldn't get the process heap (error %d)\n",
-                (int) GetLastError());
-        goto exit;
-    }
+    (I'm not sure what happens to devices that are added but never
removed.)
+*/
 
-    FreeMem(device_data);
-    /*
-    {
-        DPRINT1("Couldn't free device data memory (error %d)\n",
-                (int) GetLastError());
-    }
-    */
-
-    exit :
-        /* We just return */
-        return;
-}
-
 MMRESULT ModifyDevicePresence(
     CHAR device_type,
     WCHAR* device_path,
@@ -249,7 +307,7 @@
     ASSERT( IsValidDeviceType(device_type) );
     ASSERT( device_path );
 
-    device_data = CreateDeviceData(device_type, device_path);
+    device_data = CreateDeviceData(device_type, 0, device_path, FALSE);
 
     if ( ! device_data )
     {
@@ -292,6 +350,16 @@
     }
 }
 
+/*
+    GetDeviceCount
+
+    Pretty straightforward - pass the device type (WDMAUD_WAVE_IN, ...)
and
+    a topology device (NT object path) to obtain the number of devices
+    present in that topology of that particular type.
+
+    The topology path is supplied to us by winmm.
+*/
+
 DWORD GetDeviceCount(CHAR device_type, WCHAR* topology_path)
 {
     PWDMAUD_DEVICE_INFO device_data;
@@ -299,7 +367,7 @@
 
     DPRINT("Topology path %S\n", topology_path);
 
-    device_data = CreateDeviceData(device_type, topology_path);
+    device_data = CreateDeviceData(device_type, 0, topology_path,
FALSE);
 
     if (! device_data)
     {
@@ -339,7 +407,8 @@
     This uses a different structure to the traditional documentation,
because
     we handle plug and play devices.
 
-    Much sleep was lost over implementing this.
+    Much sleep was lost over implementing this. I got the ID and type
+    parameters the wrong way round!
 */
 
 MMRESULT GetDeviceCapabilities(
@@ -363,7 +432,7 @@
 
     DPRINT("Going to have to query the kernel-mode part\n");
 
-    device = CreateDeviceData(device_type, device_path);
+    device = CreateDeviceData(device_type, device_id, device_path,
FALSE);
 
     if ( ! device )
     {
@@ -372,8 +441,9 @@
         goto cleanup;
     }
 
-    device->id = device_id;
-    device->with_critical_section = FALSE;
+    /* These are not needed as they're already initialized */
+    ASSERT( device_id == device->id );
+    ASSERT( ! device->with_critical_section );
 
     *(LPWORD)caps->pCaps = (WORD) 0x43;
 
@@ -400,305 +470,224 @@
     }
 }
 
-MMRESULT TryOpenDevice(
+
+/*
+    OpenDeviceViaKernel
+
+    Internal function to rub the kernel mode part of wdmaud the right
way
+    so it opens a device on our behalf.
+*/
+
+MMRESULT
+OpenDeviceViaKernel(
     PWDMAUD_DEVICE_INFO device,
     LPWAVEFORMATEX format
 )
 {
-    if ( device->id > 0x64 )    /* FIXME */
-    {
-        DPRINT1("device->id > 0x64 ! ???\n");
-        return MMSYSERR_BADDEVICEID; /* OK? */
-    }
+    DWORD format_struct_len = 0;
 
-    /* We'll only have a format set for wave devices */
-    if ( format )
+    DPRINT("Opening device via kernel\n");
+
+    if ( format->wFormatTag == 1 )  /* FIXME */
     {
-        if ( format->wFormatTag == 1 )
-        {
-            DWORD sample_size;
+        /* Standard PCM format */
+        DWORD sample_size;
 
-            DPRINT("Standard (PCM) format\n");
-            sample_size = format->nChannels * format->wBitsPerSample;
-            device->state->sample_size = sample_size;
+        DPRINT("Standard (PCM) format\n");
 
-            if ( CallKernelDevice(device,
-                                  IOCTL_WDMAUD_OPEN_DEVICE,
-                                  0x10,
-                                  (DWORD)format)
-                            != MMSYSERR_NOERROR )
-            {
-                DPRINT("Call failed\n");
-                /* FIXME */
-                return MMSYSERR_NOTSUPPORTED;   /* WAVERR_BADFORMAT? */
-            }
-        }
-        else
-        {
-            /* FIXME */
-            DPRINT("Non-PCM format\n");
-            return MMSYSERR_NOTSUPPORTED;
-        }
+        sample_size = format->nChannels * format->wBitsPerSample;
+
+        device->state->sample_size = sample_size;
+
+        format_struct_len = 16; /* FIXME */
     }
+    else
+    {
+        /* Non-standard format */
+        return MMSYSERR_NOTSUPPORTED; /* TODO */
+    }
 
-    /* If we got this far without error, the format is supported! */
-    return MMSYSERR_NOERROR;
+    return CallKernelDevice(device,
+                            IOCTL_WDMAUD_OPEN_DEVICE,
+                            format_struct_len,
+                            (DWORD)format);
 }
 
-MMRESULT OpenWaveDevice(
+
+/* MOVEME */
+LPCRITICAL_SECTION CreateCriticalSection()
+{
+    LPCRITICAL_SECTION cs;
+
+    cs = AllocMem(sizeof(CRITICAL_SECTION));
+
+    if ( ! cs )
+        return NULL;
+
+    InitializeCriticalSection(cs);
+
+    return cs;
+}
+
+
+/*
+    OpenDevice
+
+    A generic "open device" function, which makes use of the above
function
+    once parameters have been checked. This is capable of handling both
+    MIDI and wave devices, which is an improvement over the previous
+    implementation (which had a lot of duplicate functionality.)
+*/
+
+MMRESULT
+OpenDevice(
     CHAR device_type,
     DWORD device_id,
-    LPWAVEOPENDESC open_details,
+    LPVOID open_descriptor,
     DWORD flags,
-    DWORD user_data
+    PWDMAUD_DEVICE_INFO* user_data
 )
 {
-    HANDLE heap = 0;
-    PWDMAUD_DEVICE_INFO device = NULL;
-    WCHAR* device_path = NULL;
     MMRESULT result = MMSYSERR_ERROR;
+    WCHAR* device_path;
+    PWDMAUD_DEVICE_INFO device;
+    LPWAVEFORMATEX format;
 
-    /* ASSERT(open_details); */
+    /* As we support both types */
+    LPWAVEOPENDESC wave_opendesc = (LPWAVEOPENDESC) open_descriptor;
+    LPMIDIOPENDESC midi_opendesc = (LPMIDIOPENDESC) open_descriptor;
 
-    heap = GetProcessHeap();
+    /* FIXME: Does this just apply to wave, or MIDI also? */
+    if ( device_id > 100 )
+        return MMSYSERR_BADDEVICEID;
 
-    if ( ! heap )
-    {
-        DPRINT1("Couldn't get the process heap (error %d)\n",
-                (int) GetLastError());
-        result = MMSYSERR_ERROR;
-        goto cleanup;
-    }
+    /* Copy the appropriate dnDevNode value */
+    if ( IsWaveDeviceType(device_type) )
+        device_path = (WCHAR*) wave_opendesc->dnDevNode;
+    else if ( IsMidiDeviceType(device_type) )
+        device_path = (WCHAR*) midi_opendesc->dnDevNode;
+    else
+        return MMSYSERR_INVALPARAM;
 
-    DPRINT("OpenDevice called\n");
+    device = CreateDeviceData(device_type, device_id, device_path,
TRUE);
 
-    device_path = (WCHAR*) open_details->dnDevNode;
-    device = CreateDeviceData(device_type, device_path);
-
     if ( ! device )
     {
-        DPRINT1("Couldn't create device data\n");
+        DPRINT1("Couldn't allocate memory for device data\n");
         result = MMSYSERR_NOMEM;
         goto cleanup;
     }
 
-    DPRINT("Allocated device data, allocating device state\n");
-
-    device->state = HeapAlloc(heap,
-                              HEAP_ZERO_MEMORY,
-                              sizeof(WDMAUD_DEVICE_STATE));
-
-    if ( ! device->state )
-    {
-        DPRINT1("Couldn't allocate memory for device state (error
%d)\n",
-                (int) GetLastError());
-        result = MMSYSERR_NOMEM;
-        goto cleanup;
-    }
-
-    /* FIXME: ok here ? */
-    device->type = device_type;
-    device->id = device_id;
     device->flags = flags;
 
-    if ( flags & WAVE_FORMAT_QUERY )
+    if ( ( IsWaveDeviceType(device->type) ) &&
+         ( device->flags & WAVE_FORMAT_QUERY ) )
     {
-        DPRINT("Do I support this format? Hmm...\n");
+        result = OpenDeviceViaKernel(device, wave_opendesc->lpFormat);
 
-        result = TryOpenDevice(device, open_details->lpFormat);
-
         if ( result != MMSYSERR_NOERROR )
         {
-            DPRINT("Format not supported\n");
-            goto cleanup;
+            DPRINT1("Format not supported (mmsys error %d)\n", (int)
result);
+            result = WAVERR_BADFORMAT;
         }
-
-        DPRINT("Yes, I do support this format!\n");
-    }
-    else
-    {
-
-        DPRINT("You actually want me to open the device, huh?\n");
-
-        /* Allocate memory for the "queue" critical section */
-
-        device->state->queue_critical_section =
-            HeapAlloc(heap, HEAP_ZERO_MEMORY,
sizeof(CRITICAL_SECTION));
-
-        if ( ! device->state->queue_critical_section )
+        else
         {
-            DPRINT1("Couldn't allocate memory for queue critical
section (error %d)\n",
-                    (int) GetLastError());
-            result = MMSYSERR_NOMEM;
-            goto cleanup;
+            DPRINT("Format supported\n");
+            result = MMSYSERR_NOERROR;
         }
 
-        /* Initialize the critical section */
-
InitializeCriticalSection(device->state->queue_critical_section);
+        goto cleanup;
+    }
 
-        /* We need these so we can contact the client later */
-        device->client_instance = open_details->dwInstance;
-        device->client_callback = open_details->dwCallback;
+    device->state->device_queue_guard = CreateCriticalSection();
 
-        /* Reset state */
-        device->state->open_descriptor = NULL;
-        device->state->unknown_24 = 0;
+    if ( ! device->state->device_queue_guard )
+    {
+        DPRINT1("Couldn't create queue cs\n");
+        result = MMSYSERR_NOMEM;
+        goto cleanup;
+    }
 
-        device->state->is_running = FALSE;
-        device->state->is_paused =
-            device->type == WDMAUD_WAVE_IN ? TRUE : FALSE;
+    /* Set up the callbacks */
+    device->client_instance = IsWaveDeviceType(device->type)
+                              ? wave_opendesc->dwInstance
+                              : midi_opendesc->dwInstance;
[truncated at 1000 lines; 2515 more skipped]