On Fri, May 6, 2011 at 6:23 PM, Joerg-Cyril.Hoehle@t-systems.com wrote:
Damjan,
@@ -407,6 +441,20 @@ static MMSYSTEM_MapType MCI_UnMapMsg16To32W(WORD wMsg, DWORD dwFlags, DWORD_PTR
- mdsp16->dwCallback = mdsp32w->dwCallback;
- mdsp16->dwItem = mdsp32w->dwItem;
- mdsp16->dwTrack = mdsp32w->dwTrack;
This is superfluous. dwReturn is the only documented output. This was different with your former MCI_WHERE patch, where a RECT is output.
I'm aware it will be superfluous in the majority of cases, but are you convinced no MCI driver ever modifies fields it shouldn't, and no application depends on that exceptional behaviour?
I'd try hard not to turn one message call (Status) into 2 (GetDevCaps + Status). I've seen logs of apps that continuously poll MCI_STATUS_POSITION. Instead, I'd continue the path lead by winmm/mci.c:MCI_MapMsgAtoW: Allocate the largest structure and fill it depending on dwFlags, e.g.: case MCI_OPEN: { /* MCI_ANIM_OPEN_PARMS is the largest known MCI_OPEN_PARMS * structure, larger than MCI_WAVE_OPEN_PARMS */ BTW, I've a note that MCI_INFO needs similar treatment, but never wrote the patch...
Select the right set of flags to fill the slots, otherwise fallback to defaults like: /* We don't know how many DWORD follow, as * the structure depends on the device. */ if (HIWORD(dwParam1)) memcpy(&mci_openW->dwStyle, &mci_openA->dwStyle, sizeof(MCI_ANIM_OPEN_PARMSW) - sizeof(MCI_OPEN_PARMSW));
Of course, one may argue that this is a heuristic and can fail. Well, GetDevCaps is a heuristic as well that only supports devtype_digital_video.
What determines what subtype of MCI_STATUS_PARMS is passed to mciSendCommand() for an MCI_STATUS message? According to the API, "For VCR devices" uses one type, "For digital-video devices" uses another type, etc. But must the digital video MCI driver assume every MCI_STATUS lpStatus parameter points to MCI_DGV_STATUS_PARMS, or must it assume it got MCI_DGV_STATUS_PARMS only when it deals with a DGV-specific status request like MCI_DGV_STATUS_DISKSPACE and fall back to the generic MCI_STATUS_PARMS for all other cases?
An advantage IMHO of the existing approach is that it does not invent within the same source code base yet another mechanism to map 16 to 32 or A to W. I'd rather follow the existing model until the time it proves unbearable, or immediately switch the *whole* code base to another model, not the single MCI_STATUS function.
For instance, you can argue that it makes sense for winmm to cache the device type (winmm:MCI_Open has it in several cases). It would arguably also make sense to cache the MCI auto_open'ed property, but I digress.
Your MCI_get_device_type is bogus. MCI_GETDEVCAPS_DEVICE_TYPE belongs into dwItem, not dwFlags.
Let's finish this discussion, then I'll see whether to fix that bug or eliminate the entire function.
Regards, Jörg Höhle
Regards Damjan Jovanovic