Octavian,
I just looked in mmddk.h and they have this: #define MCI_INTEGER64 13
It's good you found out about MCI_INTEGER64. Yet it's completely crazy because googling it yields 6 hits in total, all about Wine bug 22146. Bing shows nothing at all. How can such a name be that invisible?
So, do you want to fix your patches and send them, or do you want me to continue working on this patch set?
I'm fine if you do (as I've plenty of other patches in my queue) especially if you also fix the parser to handle MCI_HWND (another DWORD_PTR), which is the second half needed for 64bit as I mention in http://bugs.winehq.org/show_bug.cgi?id=22146#c12
GetDWORD also needs some change to be able to parse DWORD_PTR things, e.g. window handles. I've not yet written a single test for that (no mciavi tests). I'll send you all the MCI tests I have written ASAP.
Even without written tests, one can use WINEDEBUG=+mci and analyse the parser logs for individual commands sent e.g. via the interactive MCI shell you'll find in http://bugs.winehq.org/show_bug.cgi?id=20232#c10
You obviously own a 64bit system so it's easier for you to test. I don't and had to beg.
Regards, Jörg Höhle
Hi,
Octavian Voicu wrote:
so binary compatibility in RC files is required
We don't have this right now. Native winmm.dll uses integer resource ids (id 200 for core commands, [...]) Wine currently uses string ids
Could you please elaborate on that? I wasn't aware of an incompatibility.
I don't agree here. IMO the parser should be the most agnostic possibly against the real data structures
Not sure if I understand exactly what you mean here
I believe Eric envisions a parser that needs no MCI_*_PARMS at all because everything lies in the resource (except that the first DWORD_PTR is the callback). Even MCI_RETURN need not be second (yet there's no known case where that happens). It would be a major change from the current parser.
It could be sketched as follows: - get first token (verb) - get second token (beware of quoting) ("device") - find driver according to device - either name of an existing alias - or name of MCI driver ("waveaudio") - or waveaudio!foo.wav - or "all" - or something else - load driver's command table (may need loading driver!) - parse command options according to table
Now we know the command's numeric value and can act upon it. That would be much more compatible than what is in place now.
Still, the devil lies in the details. E.g. a) sound sound notify are valid MCI commands. Look ma, no device!
b) Sysinfo cdaudio quantity open need IMHO not load the cdaudio driver. It strictly operates at winmm's level. Perhaps first try to identify the verb via the core table, and if it's a core command like MCI_SYSINFO, never load the driver? MCI_SYSINFO_PARMS.wDeviceType needs special treatment anyway, the parser is not enough.
c) When to unload the driver?
At some point one needs MCI_*_PARMS. For instance, it's winmm's job to detect MCIERR_DUPLICATE_ALIAS. How is it going to access the alias field if not via MCI_OPEN_PARMS and thus imposing that all MCI_*_OPEN_PARMS are congruent? (e.g. MCI_DGV|WAVE_OPEN_PARMS).
One could imagine backparsing the resource file to detect the position of the slot that defines the MCI_ALIAS element... This is just crazy.
I've tests that prove (and a patch in my queue) that unlike the mciSendCommand interface, the string parser always sets an alias. So at some point it must set the lpstrAlias slot. How is it going to do that if not via MCI_OPEN_PARMS?
Regards, Jörg Höhle
On Fri, Aug 27, 2010 at 7:36 PM, Joerg-Cyril.Hoehle@t-systems.com wrote:
Octavian Voicu wrote:
so binary compatibility in RC files is required
We don't have this right now. Native winmm.dll uses integer resource ids (id 200 for core commands, [...]) Wine currently uses string ids
Could you please elaborate on that? I wasn't aware of an incompatibility.
Check out this job on wine testbot: https://testbot.winehq.org/JobDetails.pl?Key=4830
I copied the table loading code from mci.c, but had to change the code that loaded the tables, because in native winmm.dll, tables are identified by an integer resource id (not a string one):
str = MAKEINTRESOURCEW(200); /* this is for CORE table I guess, as it doesn't match any known device type */
I read somewhere that WIN98 and earlier stored tables in mmsystem.dll, that's why all the tests fail.
If somebody would be interested, he could use resedit (free download from www.resedit.net) to open resources in dlls (www.topdll.com has a lot of dlls of various versions). Latest ones have versions something like 6.1.7600.* (ie. the ones used by win7).
Anyway, compare the full log (from the testbot job 4830 linked eralier) for tests W7PROX64 (32 bit mci) and W7PROX64 (64 bit mci). Conclusion is that 32 bit mci uses MCI_INTEGER as return type for MCI_STATUS, while 64 bit one uses MCI_INTEGER64.
I'll add MCI_INTEGER64 to mmddk.h (#ifdef-ed like in the ddk) and also something like MCI_INTEGER3264 which will be MCI_INTEGER on 32 bit systems, and MCI_INTEGER64 on 64 bit systems. Then use MCI_INTEGER3264 in winmm_res.rc & others.
I don't agree here. IMO the parser should be the most agnostic possibly against the real data structures
Not sure if I understand exactly what you mean here
I believe Eric envisions a parser that needs no MCI_*_PARMS at all because everything lies in the resource (except that the first DWORD_PTR is the callback). Even MCI_RETURN need not be second (yet there's no known case where that happens). It would be a major change from the current parser.
It could be sketched as follows: - get first token (verb) - get second token (beware of quoting) ("device") - find driver according to device - either name of an existing alias - or name of MCI driver ("waveaudio") - or waveaudio!foo.wav - or "all" - or something else - load driver's command table (may need loading driver!) - parse command options according to table
Now we know the command's numeric value and can act upon it. That would be much more compatible than what is in place now.
Still, the devil lies in the details. E.g. a) sound sound notify are valid MCI commands. Look ma, no device!
I'll look into it, sounds like a challenge :)
b) Sysinfo cdaudio quantity open need IMHO not load the cdaudio driver. It strictly operates at winmm's level. Perhaps first try to identify the verb via the core table, and if it's a core command like MCI_SYSINFO, never load the driver? MCI_SYSINFO_PARMS.wDeviceType needs special treatment anyway, the parser is not enough.
c) When to unload the driver?
At some point one needs MCI_*_PARMS. For instance, it's winmm's job to detect MCIERR_DUPLICATE_ALIAS. How is it going to access the alias field if not via MCI_OPEN_PARMS and thus imposing that all MCI_*_OPEN_PARMS are congruent? (e.g. MCI_DGV|WAVE_OPEN_PARMS).
One could imagine backparsing the resource file to detect the position of the slot that defines the MCI_ALIAS element... This is just crazy.
I've tests that prove (and a patch in my queue) that unlike the mciSendCommand interface, the string parser always sets an alias. So at some point it must set the lpstrAlias slot. How is it going to do that if not via MCI_OPEN_PARMS?
Not very familiar with this part yet, so I'll focus on getting basic 64 bit support working first, while also looking at the big picture when possible.
Octavian
Hi,
Octavian Voicu wrote:
b) Sysinfo cdaudio quantity open
The latest patches I've now submitted show how to modify parameters in the parser. The key is to do it after the parsing.
I'll focus on getting basic 64 bit support working first, while also looking at the big picture when possible.
Besides the points I previously mentioned there are 3 major ones.
1.The bummer: err = mciSendString("capability x can save", buf, sizeof(buf), hwnd); todo_wine ok(!err,"capability can (space) save: %s\n", dbg_mcierr(err)); This test alone convinced me that the native parser is not based on a strcmp() of the strings found in the resource file. Something else is going on.
2. The chicken and egg problem: open foo.mpg alias w buffer 5 type waveaudio The "buffer" parameter proves that the command is parsed using the waveaudio command table, not the core one. How does the parser know that "waveaudio" must be used? It does not depend on foo.mpg's file extension.
Obviously the command line must be preparsed somehow to find the "type" keyword. But you can only parse correctly when you know the right table...
Wine currently uses strstr(). It's easily fooled by input like open foo.mpg alias "foo type sequencer bar.zot" Native winmm is not fooled.
I already fixed some bugs in Wine's special-casing of the open command. Yet I know there's at least one more bug therein.
3. Incorrect parsing of "optional" ("\0") parameters: I already mentioned "sound notify". MCI_SAVE is another example. open new type waveaudio alias r save r notify mci.c:1376: command: save r notify mci.c:1379: Test failed: mci save r notify error: 298(42 MCIERR_UNNAMED_RESOURCE) save r notify grr.wav mci.c:1376: command: save r notify grr.wav mci.c:1355: Notification type 0001 The filename is not "notify". info r file mci.c:1376: command: info r file mci.c:1381: Response: X:...\grr.wav
The unnamed ("\0") "optional" parameter can appear out of order, like the named ones.
Understanding these points will undoubtfully lead to the big picture.
Regards, Jörg Höhle.
-
Joerg-Cyril.Hoehle＠t-systems.com -
Octavian Voicu