June 2011 - Wine-Devel - list.winehq.org

Re: [1/3] kernel32/heap: Emulate Win9x if appropriate in GlobalMemoryStatusEx(). (resend)

by Adam Martinson

On 06/02/2011 09:21 PM, Charles Davis wrote: > On 6/2/11 2:46 PM, Adam Martinson wrote: >> On 06/02/2011 03:10 PM, Dmitry Timoshkov wrote: >>> Adam Martinson<amartinson(a)codeweavers.com> wrote: >>> >>>> + osver.dwOSVersionInfoSize = sizeof(osver); >>>> + GetVersionExW(&osver); >>>> + >>> ... >>>> + if (osver.dwMajorVersion>= 5) >>> Once again, that's wrong way of detecting win9x, or please try to explain >>> better how the patch does what the subject describes, and for which app >>> it's needed. >>> >> The stuff in that block is for win2k+ mode only. Win9x doesn't add >> [Total|Avail]Phys to [Total|Avail]PageFile. If wine is in win9x mode it >> should behave like win9x, or it confuses old apps. This is needed for >> the 2nd patch, IIRC removing the photoshop 4 hack in >> GlobalMemoryStatus() screws up programs like Adobe Illustrator 8 in >> win98 mode without it. > I guess what Dmitry is really trying to ask is: what about NT4? Its > major version is less than 5, but it's not a Win9x-family OS; it's an > NT-family OS. Ah ok, so should be if ( osver.dwMajorVersion >= 5 || osver.dwPlatformId != VER_PLATFORM_WIN32_WINDOWS ).

June 2, 2011

Re: [1/3] kernel32/heap: Emulate Win9x if appropriate in GlobalMemoryStatusEx(). (resend)

by Charles Davis

On 6/2/11 2:46 PM, Adam Martinson wrote: > On 06/02/2011 03:10 PM, Dmitry Timoshkov wrote: >> Adam Martinson<amartinson(a)codeweavers.com> wrote: >> >>> + osver.dwOSVersionInfoSize = sizeof(osver); >>> + GetVersionExW(&osver); >>> + >> ... >>> + if (osver.dwMajorVersion>= 5) >> Once again, that's wrong way of detecting win9x, or please try to explain >> better how the patch does what the subject describes, and for which app >> it's needed. >> > The stuff in that block is for win2k+ mode only. Win9x doesn't add > [Total|Avail]Phys to [Total|Avail]PageFile. If wine is in win9x mode it > should behave like win9x, or it confuses old apps. This is needed for > the 2nd patch, IIRC removing the photoshop 4 hack in > GlobalMemoryStatus() screws up programs like Adobe Illustrator 8 in > win98 mode without it. I guess what Dmitry is really trying to ask is: what about NT4? Its major version is less than 5, but it's not a Win9x-family OS; it's an NT-family OS.

June 2, 2011

Re: [2/3] kernel32/heap: Fix some hacks in GlobalMemoryStatus(). (try 2)

by Adam Martinson

On 06/02/2011 03:11 PM, Dmitry Timoshkov wrote: > Adam Martinson<amartinson(a)codeweavers.com> wrote: > >> +/* Check the current process and all ancestors that share its address space. >> + * Some apps run sub-processes that have the IMAGE_FILE_LARGE_ADDRESS_AWARE flag set, but >> + * the parent process doesn't (eg, Dragon NaturallySpeaking 7 setup runs Install Shield 6). >> + * If any don't have IMAGE_FILE_LARGE_ADDRESS_AWARE, return FALSE. */ > My previous question still applies. > Your previous quesion: > How do the processes running in separate address spaces have anything to do with that? I thought I addressed that with this version of the patch, but if I'm not checking address space correctly, please tell me the correct way. PEB::InheritedAddressSpace doesn't work, I'm not sure what else I should be looking for.

June 2, 2011

Re: [1/3] kernel32/heap: Emulate Win9x if appropriate in GlobalMemoryStatusEx(). (resend)

by Adam Martinson

On 06/02/2011 03:10 PM, Dmitry Timoshkov wrote: > Adam Martinson<amartinson(a)codeweavers.com> wrote: > >> + osver.dwOSVersionInfoSize = sizeof(osver); >> + GetVersionExW(&osver); >> + > ... >> + if (osver.dwMajorVersion>= 5) > Once again, that's wrong way of detecting win9x, or please try to explain > better how the patch does what the subject describes, and for which app > it's needed. > The stuff in that block is for win2k+ mode only. Win9x doesn't add [Total|Avail]Phys to [Total|Avail]PageFile. If wine is in win9x mode it should behave like win9x, or it confuses old apps. This is needed for the 2nd patch, IIRC removing the photoshop 4 hack in GlobalMemoryStatus() screws up programs like Adobe Illustrator 8 in win98 mode without it.

June 2, 2011

Re: [13/18]usp10/tests: add Gurmukhi shaping test

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=11438 Your paranoid android. === WXPPROSP3 (32 bit usp10) === TestLauncher.c:89: Test failed: Can't open test executable C:\winetest\usp10_test.exe, error 32

June 2, 2011

Re: [2/3] kernel32/heap: Fix some hacks in GlobalMemoryStatus(). (try 2)

by Dmitry Timoshkov

Adam Martinson <amartinson(a)codeweavers.com> wrote: > +/* Check the current process and all ancestors that share its address space. > + * Some apps run sub-processes that have the IMAGE_FILE_LARGE_ADDRESS_AWARE flag set, but > + * the parent process doesn't (eg, Dragon NaturallySpeaking 7 setup runs Install Shield 6). > + * If any don't have IMAGE_FILE_LARGE_ADDRESS_AWARE, return FALSE. */ My previous question still applies. -- Dmitry.

June 2, 2011

Re: [1/3] kernel32/heap: Emulate Win9x if appropriate in GlobalMemoryStatusEx(). (resend)

by Dmitry Timoshkov

Adam Martinson <amartinson(a)codeweavers.com> wrote: > + osver.dwOSVersionInfoSize = sizeof(osver); > + GetVersionExW(&osver); > + ... > + if (osver.dwMajorVersion >= 5) Once again, that's wrong way of detecting win9x, or please try to explain better how the patch does what the subject describes, and for which app it's needed. -- Dmitry.

June 2, 2011

Re: [2/19](resend)usp10: improve Sinhala shaping using Indic rules

by Alexandre Julliard

Aric Stewart <aric(a)codeweavers.com> writes: > @@ -0,0 +1,236 @@ > + > +#include "config.h" > +#include <stdarg.h> > +#include <stdio.h> > +#include <stdlib.h> Please add a license header on new files. > +#define IS_VALID_INDEX (next < cChar) Don't reference local variables directly, pass them to the macro. > +#define OPTIONAL_CHAR(a) if (IS_VALID_INDEX && lex(input[next])==a) {next++;} > +#define OPTIONAL_CHAR_JOINER() if (IS_VALID_INDEX && IS_JOINER(next)) {next++;} > +#define ZERO_OR_MORE(a) while(IS_VALID_INDEX && lex(input[next])==a){next++;} > +#define ZERO_OR_MORE_MANTRA() while (IS_VALID_INDEX && IS_MANTRA(next)){next++;} Same here, and these should be wrapped in do/while(0). -- Alexandre Julliard julliard(a)winehq.org

June 2, 2011

Re: ddraw: Add tests for Setcooperativelevel

by Stefan Dösinger

On Wednesday 01 June 2011 20:37:36 paulo lesgaz wrote: > Yes, see bug 25660 > My tests prove that > DDSCL_CREATEDEVICEWINDOW+DDSCL_EXCLUSIVE+DDSCL_FULLSCREEN is allowed with > a NULL window. That is what expected by Half life I demo. Erm, I missed that in your patch, it was right at the top of the function. > >have are somewhat unsatisfying, we don't have a single test where > >DDSCL_CREATEDEVICEWINDOW is supposed to succeed. > > Maybe it would be the subject of an another patch? Yes, although I am interested in things beyond the return value of SetCooperativeLevel. E.g. how does the device window work wrt clipping - is the primary surface rendered to the device window, or is it still rendered to the screen(to see what I mean run the dx7 sdk's ddraw font sample on Windows). Is there a relationship between the front buffer's size and the device window? With a device window, does the front buffer still have screen size, or does it have the size of the device window at creation time? Some of those things are probably tricky to test from an automated test.

June 2, 2011

Re: [1/26](resend)usp10: Add Indic syllable parsing and reordering system

by Alexandre Julliard

Aric Stewart <aric(a)codeweavers.com> writes: > +void Indic_ReorderCharacters( LPWSTR input, int cChar, lexical_function lex, reorder_function reorder_f) > +{ > + int index = 0; > + int next = 0; > + int center; > + > + if (!lex || ! reorder_f) > + { > + ERR("Failure to have required functions\n"); > + return; > + } > + > + debug_output_string(input, cChar, lex); > + while (next != -1) > + { > + ZERO_OR_MORE(lex_Generic); > + index = next; > + next = Indic_process_next_syllable(input, cChar, 0, &center, index, lex); > + if (next != -1) > + { > + reorder_f(input, index, center, next-1, lex); > + index = next; > + } > + else if (index < cChar) > + { > + int i; > + TRACE("Processing failed at %i\n",index); > + for (i = index; i < cChar; i++) > + if (lex(input[i])==lex_Generic) > + { > + TRACE("Restart processing at %i\n",i); > + next = i; > + index = i; > + break; > + } > + } > + } > + TRACE("Processed %i of %i characters\n",index,cChar); > +} Please don't add dead code. This function should be called from somewhere (same for the subsequent patches). -- Alexandre Julliard julliard(a)winehq.org

June 2, 2011

A request about the release notes

by Scott Ritchie

When 1.3.19 came out it changed the build requirements for OSS support -- OSS 3 was no longer supported and OSS 4 dev libraries were required at build time. This was the changelog: New sound driver architecture for MMDevAPI. Better support for relative mouse events in DInput. Debugger support for the ARM platform. Various improvements in D3DX9. More MSVC runtime functions. Various bug fixes. At least two distros broke their OSS support as a result, since the standard "update wine version but use same packaging" didn't return a complete Wine anymore. In my case I only noticed when a bug report came in and I manually looked at the package build log. This sort of thing would be much less likely to happen if such a build dependency change were noted in the release notes, such as a "OSS 3 no longer supported" bullet point. Thanks, Scott Ritchie

June 1, 2011

Re : ddraw: Add tests for Setcooperativelevel

by paulo lesgaz

________________________________ De : Stefan Dösinger <stefandoesinger(a)gmx.at> À : wine-devel(a)winehq.org Cc : paulo lesgaz <jeremielapuree(a)yahoo.fr> Envoyé le : Mer 1 juin 2011, 15h 11min 25s Objet : Re: ddraw: Add tests for Setcooperativelevel On Wednesday 01 June 2011 08:01:57 paulo lesgaz wrote: >> Hi, >> >> any problem with the patch >> >> http://source.winehq.org/patches/data/74702 >I guess Alexandre was waiting for a reply from me or Henri. > >The tests themselves look ok, but I wonder what your ultimate goal is - is >there a game that has issues with DDSCL_CREATEDEVICEWINDOW? Yes, see bug 25660 My tests prove that DDSCL_CREATEDEVICEWINDOW+DDSCL_EXCLUSIVE+DDSCL_FULLSCREEN is allowed with a NULL window. That is what expected by Half life I demo. Currently, Wine returns DDERR_INVALIDPARAMS. Obviously, there are already tests that prove that DDSCL_EXCLUSIVE+DDSCL_FULLSCREEN fails with a null window. > Also the tests we >have are somewhat unsatisfying, we don't have a single test where >DDSCL_CREATEDEVICEWINDOW is supposed to succeed. Maybe it would be the subject of an another patch? A+ David

June 1, 2011

Re: wined3d: Set container of swapchain surfaces to WINED3D_CONTAINER_NONE in swapchain_init() error path.

by Stefan Dösinger

On Wednesday 01 June 2011 01:38:24 Piotr Pawłow wrote: > Since commit fadfdf21c055caf833a137ffc269c2073dc87fd6 surface refcounts are > forwarded to the container. We need to break this forwarding in > swapchain_init() error path, otherwise the swapchain refcount goes down to > zero and swapchain_cleanup() gets called on an invalid, partly initialized > swapchain. This patch looks OK

June 1, 2011

Re: Added unbound sampler test for pixel shaders.

by Alexandre Julliard

Sean Sube <ssube(a)codeweavers.com> writes: > --- > dlls/d3d9/tests/visual.c | 80 > ++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 80 insertions(+), 0 deletions(-) It doesn't work here: ../../../tools/runtest -q -P wine -M d3d9.dll -T ../../.. -p d3d9_test.exe.so visual.c && touch visual.ok visual.c:8909: Tests skipped: D3DFMT_G16R16 textures not supported as render targets. visual.c:7503: Tests skipped: Card has unconditional pow2 support, skipping conditional NP2 tests visual.c:12112: Test failed: Color is 0xffffffff. make[1]: *** [visual.ok] Error 1 -- Alexandre Julliard julliard(a)winehq.org

June 1, 2011

Re: [4/4] ws2_32: Avoid a crash with an optional pointer in WSAAsyncGetServByName.

by Alexandre Julliard

Andrew Nguyen <anguyen(a)codeweavers.com> writes: > On 06/01/2011 07:39 AM, Marvin wrote: >> === WXPPROSP3 (32 bit sock) === >> sock.c:501: Test failed: oob_server (218): unexpectedly at the OOB mark: 0 >> sock.c:511: Test failed: oob_server (218): unexpectedly at the OOB mark: 0 > > The test failure is not related to my changes. I'm also seeing some random crashes here. For instance: ../../../tools/runtest -q -P wine -M ws2_32.dll -T ../../.. -p ws2_32_test.exe.so sock.c && touch sock.ok wine: Unhandled page fault on execute access to 0x00000000 at address (nil) (thread 0021), starting debugger... Unhandled exception: page fault on execute access to 0x00000000 in 32-bit code (0x00000000). Register dump: CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b EIP:00000000 ESP:0032f6a4 EBP:0032f798 EFLAGS:00010202( R- -- I - - - ) EAX:00000000 EBX:7bc9ed6c ECX:0032f5d0 EDX:00000001 ESI:00000000 EDI:00000000 Stack dump: 0x0032f6a4: 0032f800 0032f798 7bc7285c 00000002 0x0032f6b4: 0032f6d0 00000000 0032f6c0 00000000 0x0032f6c4: 00000000 00000000 0032f6d0 00000000 0x0032f6d4: 00000000 0032f801 00000001 0032f728 0x0032f6e4: 6818d724 00000040 7bc72710 00000006 0x0032f6f4: 0032f830 0032f8a8 68025b9e 0032f748 Backtrace: =>0 0x00000000 (0x0032f798) 1 0x7bc7cc4a RtlCreateUserThread+0x2d9(process=0xffffffff, descr=(nil), suspended=1, stack_addr=0x0(nil), stack_reserve=0, stack_commit=0, start=0x68686ac0, param=0x32fa78, handle_ptr=0x32f9ac, id=0x32f9a0) [/home/julliard/wine/wine/dlls/ntdll/thread.c:498] in ntdll (0x0032f968) 2 0x7b87247d CreateRemoteThread+0x7c(hProcess=0xffffffff, sa=(nil), stack=0, start=0x68686ac0, param=0x32fa78, flags=0, id=0x32fa70) [/home/julliard/wine/wine/dlls/kernel32/thread.c:89] in kernel32 (0x0032f9c8) 3 0x7b872578 CreateThread+0x47(sa=(nil), stack=0, start=0x68686ac0, param=0x32fa78, flags=0, id=0x32fa70) [/home/julliard/wine/wine/dlls/kernel32/thread.c:55] in kernel32 (0x0032f9f8) 4 0x686923ce test_send+0x10d() [/home/julliard/wine/wine/dlls/ws2_32/tests/sock.c:2955] in ws2_32_test (0x0032fa98) 5 0x6869b18b func_sock+0x36ca() [/home/julliard/wine/wine/dlls/ws2_32/tests/sock.c:4975] in ws2_32_test (0x0032fd38) 6 0x6869c23e run_test+0x14d(name="sock.c") [/home/julliard/wine/wine/dlls/ws2_32/tests/../../../include/wine/test.h:556] in ws2_32_test (0x0032fd88) 7 0x6869cd07 main+0x156(argc=0, argv=(nil)) [/home/julliard/wine/wine/dlls/ws2_32/tests/../../../include/wine/test.h:624] in ws2_32_test (0x0032fe48) 8 0x6869cedc __wine_spec_exe_entry+0x7b(peb=0x7ffdf000) [/home/julliard/wine/wine/dlls/winecrt0/exe_entry.c:36] in ws2_32_test (0x0032fe90) 9 0x7b85aaac call_process_entry+0xb() in kernel32 (0x0032fea8) 10 0x7b85b6df start_process+0x5e(peb=0x7ffdf000) [/home/julliard/wine/wine/dlls/kernel32/process.c:1087] in kernel32 (0x0032fee8) 11 0x7bc737d8 call_thread_func+0xb() in ntdll (0x0032fef8) 12 0x7bc76f3e call_thread_entry_point+0x6d(entry=0x7b85b680, arg=0x7ffdf000) [/home/julliard/wine/wine/dlls/ntdll/signal_i386.c:2499] in ntdll (0x0032ffc8) 13 0x7bc4c13e start_process+0x1d(kernel_start=0x7b85b680) [/home/julliard/wine/wine/dlls/ntdll/loader.c:2612] in ntdll (0x0032ffe8) 14 0x6802998d wine_call_on_stack+0x1c() in libwine.so.1 (0x00000000) 0x00000000: -- no code accessible -- -- Alexandre Julliard julliard(a)winehq.org

June 1, 2011

Re: RFC: d3d9: Increment the reference count of the IDirect3D9 instance when creating a device.

by Stefan Dösinger

On Tuesday 31 May 2011 13:55:21 Andrew Nguyen wrote: > The Pathologic demo in bug 21552 expects that the reference count of a > IDirect3D9 instance is incremented after a successful call to > CreateDevice with the instance. > > Does my proposed fix for d3d8 and d3d9 manage the reference count of the > device parent appropriately? In particular, I'm concerned about the > slightly cumbersome acquisition of the pointer to the parent instance > within the Release call for the Direct3D device interface. I think it is correct. But you can also just store a pointer to the IDirect3D8/9 interface in the d3d8/9 device implementation like the resources do. That way you can avoid the tricky object querying in the release function.

June 1, 2011

Re: mmdevapi buffering

by Andrew Eikum

On 06/01/2011 07:33 AM, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > The problematic situation is when avail is small, as the following sleep for half a > buffer size in MSDN's example will inevitably produce an xrun. > So what actually is the buffer size and how does GetCurrentPadding behave? > > I don't expect MSDN's example to be fundamentally flawed. IMHO, they > designed their API such that it is the typical use case. > I believe that multiple buffers (of size GetBufferSize) are > involved (much like ALSA's periods) and that GetBuffer switches among > them -- exactly like they say about the exclusive mode ping pong. I don't think I understand your objection. I have been thinking of the buffer as a FIFO queue with a size limit. Then, GetCurrentPadding() returns "write - read" and GetBufferSize() returns the queue size limit. As far as I can tell, this is entirely consistent with how Windows behaves. You can especially see this with the GetBuffer() failure tests (see <dlls/mmdevapi/tests/render.c:509> for example). What behavior do you think Wine is going to do wrong because of this model? Thanks, Andrew

June 1, 2011

Re: winealsa.drv:mmdevapi input validation

by Andrew Eikum

Hi Joerg, On 06/01/2011 07:00 AM, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > because of bug #27087, I had a look at the emerging mmdevapi code. > Actually, it's the first time I read its documentation on MSDN. > Here are a few comments of mine while reading the code. Thanks for the careful review! > The handling of duration (-> buffer size) in > winealsa.drv/mmdevdrv.c:AudioClient_Initialize does not perform enough > validation. MSDN explicitly acknowledges that it could be 0. I > believe it should be related to ALSA's buffer, or a minimum of something > like 2*period_size. Or perhaps even 3*period size. Yes. I have yet to see an application specify a 0 buffer, but we should handle it this way instead of failing. This is important and I'll see to fixing it soon. > IIRC MSDN also documents a maximum of 2s for renderer and 0.5 for capture. These limitations might be true for exclusive mode, which isn't really implemented in Wine. Frankly, I see little reason to spend time implementing special handling of exclusive mode unless we find an app that does something that requires it. > AudioRenderClient_ReleaseBuffer does not check that written_frames is > what GetBuffer returned. Use with AUDCLNT_BUFFERFLAGS_SILENT and > you'll have memcpy overwrite arbitrary amounts of memory. > MSDN says: AUDCLNT_E_INVALID_SIZE Yeah, that should be checked more closely. > I believe Wine should currently include > if (AUDCLNT_STREAMFLAGS_EVENTCALLBACK&& SHAREMODE_EXCLUSIVE) > { FIXME("unsupported")& return E_ } > because none of the buffer allocation and parameter checking specific > to that mode that MSDN mentions in Initialize is implemented. Is it really better to outright fail on that combination instead of having a (probably quite good) chance at just working? I think it would be worth having a loud FIXME when exclusive mode is requested, though. > When it (the ping pong buffer logic) will be implemented, some better > logic to trigger SetEvent() will probably be needed, because I'm > unsure whether CreateTimerQueueTimer() provides the required stability > over time and possibly ignores a laptop's suspend/resume cycle or xruns. > In exclusive mode, reception of an event is a guarantee that > GetBuffer(GetBufferSize frames) will succeed. Hmm, perhaps. It would be nice to have an application that actually uses exclusive mode, to see how it behaves with this "non-exclusive" implementation. I have yet to see an application that uses exclusive mode, though... > I suggest the values returned by AudioClient_GetDevicePeriod be adjusted > to match what test.winehq shows. 10ms/3ms makes some sense, 10ms is > mentioned a lot in MSDN and blogs. 3ms is small yet has a chance to > work without glitch with MSDN's exclusive mode example. These were tuned basically by hand, to values that worked well for the small number of test applications I had. I could try tweaking them to 10ms/3ms and see if it still works. I agree that being consistent with Windows would be somewhat better arbitrary values than the ones we currently have. > I can't remember seeing input validation of broken avgbytes and blockalign in the > WAVEFORMAT parameter. I showed some tests some month ago that prove that > unlike Wine, winmm is not disturbed by bad values. How does mmdevapi react to them? I believe mmdevapi actually should validate those parameters, but I'm not certain. Tests would obviously clear that up. None of the things above (except the 0 size buffer one) are really in need of urgent fixing, so I probably won't get to them for a while. I have made notes about them, though. If you would like to send patches fixing them (with tests, etc of course), please do. Notice that it's likely that the other drivers have the same problems. Thanks again, Andrew

June 1, 2011

Re: ddraw: Add tests for Setcooperativelevel

by Stefan Dösinger

On Wednesday 01 June 2011 08:01:57 paulo lesgaz wrote: > Hi, > > any problem with the patch > > http://source.winehq.org/patches/data/74702 I guess Alexandre was waiting for a reply from me or Henri. The tests themselves look ok, but I wonder what your ultimate goal is - is there a game that has issues with DDSCL_CREATEDEVICEWINDOW? Also the tests we have are somewhat unsatisfying, we don't have a single test where DDSCL_CREATEDEVICEWINDOW is supposed to succeed.

June 1, 2011

Re: [4/4] ws2_32: Avoid a crash with an optional pointer in WSAAsyncGetServByName.

by Andrew Nguyen

On 06/01/2011 07:39 AM, Marvin wrote: > Hi, > > While running your changed tests on Windows, I think I found new failures. > Being a bot and all I'm not very good at pattern recognition, so I might be > wrong, but could you please double-check? > Full results can be found at > http://testbot.winehq.org/JobDetails.pl?Key=11362 > > Your paranoid android. > > > === WXPPROSP3 (32 bit sock) === > sock.c:501: Test failed: oob_server (218): unexpectedly at the OOB mark: 0 > sock.c:511: Test failed: oob_server (218): unexpectedly at the OOB mark: 0 The test failure is not related to my changes.

June 1, 2011

Re: [4/4] ws2_32: Avoid a crash with an optional pointer in WSAAsyncGetServByName.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=11362 Your paranoid android. === WXPPROSP3 (32 bit sock) === sock.c:501: Test failed: oob_server (218): unexpectedly at the OOB mark: 0 sock.c:511: Test failed: oob_server (218): unexpectedly at the OOB mark: 0

June 1, 2011

mmdevapi buffering

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, I have one big concern about MSDN's "Rendering a stream" (shared mode) example w.r.t wrap-around of buffer pointers and how Wine implements it. The example is at http://msdn.microsoft.com/en-us/library/dd316756(v=vs.85).aspx Suppose the write pointer is near the end of the buffer and the app is a little late, so the HW read pointer is not far away. |---------------------------------------------------^read------^write-----| What is GetCurrentPadding going to return? - the tiny write-read portion, says MSDN AFAICT - 0, because there's a new fresh buffer waiting to be filled? MSDN's example then computes avail = GetBufferSize() - GetCurrentPadding(); What is GetBuffer(avail) going to return?!? - If it's a pointer to the above buffer, it can only succeed if avail is tiny, otherwise there's a buffer overflow. However avail, as defined above and in MSDN's example, will be large. Wine tricks around that by returning a dynamic local tmp buffer larger than GetBufferSize (I wonder why a fixed one instead, twice GetBufferSize would not suffice?) Does native behave like that? - E_BUFFER_TOO_LARGE ? The MSDN example would exit. - Could it be a pointer to the beginning of the buffer? - Or it's a pointer to another buffer! That's not compatible with Wine's current local_buffer implementation... Isn't Wine's mmdevapi going to produces glitches? The problematic situation is when avail is small, as the following sleep for half a buffer size in MSDN's example will inevitably produce an xrun. So what actually is the buffer size and how does GetCurrentPadding behave? I don't expect MSDN's example to be fundamentally flawed. IMHO, they designed their API such that it is the typical use case. I believe that multiple buffers (of size GetBufferSize) are involved (much like ALSA's periods) and that GetBuffer switches among them -- exactly like they say about the exclusive mode ping pong. I suppose native's mixer (in shared mode) would mix these possibly incompletely filled buffers and everything is fine there. No need to deal with wrap-around! Why should Wine do something entirely different? Regards, Jörg Höhle

June 1, 2011

winealsa.drv:mmdevapi input validation

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, because of bug #27087, I had a look at the emerging mmdevapi code. Actually, it's the first time I read its documentation on MSDN. Here are a few comments of mine while reading the code. The handling of duration (-> buffer size) in winealsa.drv/mmdevdrv.c:AudioClient_Initialize does not perform enough validation. MSDN explicitly acknowledges that it could be 0. I believe it should be related to ALSA's buffer, or a minimum of something like 2*period_size. Or perhaps even 3*period size. IIRC MSDN also documents a maximum of 2s for renderer and 0.5 for capture. AudioRenderClient_ReleaseBuffer does not check that written_frames is what GetBuffer returned. Use with AUDCLNT_BUFFERFLAGS_SILENT and you'll have memcpy overwrite arbitrary amounts of memory. MSDN says: AUDCLNT_E_INVALID_SIZE I believe Wine should currently include if (AUDCLNT_STREAMFLAGS_EVENTCALLBACK && SHAREMODE_EXCLUSIVE) { FIXME("unsupported") & return E_ } because none of the buffer allocation and parameter checking specific to that mode that MSDN mentions in Initialize is implemented. When it (the ping pong buffer logic) will be implemented, some better logic to trigger SetEvent() will probably be needed, because I'm unsure whether CreateTimerQueueTimer() provides the required stability over time and possibly ignores a laptop's suspend/resume cycle or xruns. In exclusive mode, reception of an event is a guarantee that GetBuffer(GetBufferSize frames) will succeed. I suggest the values returned by AudioClient_GetDevicePeriod be adjusted to match what test.winehq shows. 10ms/3ms makes some sense, 10ms is mentioned a lot in MSDN and blogs. 3ms is small yet has a chance to work without glitch with MSDN's exclusive mode example. I can't remember seeing input validation of broken avgbytes and blockalign in the WAVEFORMAT parameter. I showed some tests some month ago that prove that unlike Wine, winmm is not disturbed by bad values. How does mmdevapi react to them? Regards, Jörg Höhle

June 1, 2011

Re: infosoft/tests: COM cleanup in infosoft.c.

by Michael Stefaniuc

Marvin wrote: > While running your changed tests on Windows, I think I found new failures. > Being a bot and all I'm not very good at pattern recognition, so I might be > wrong, but could you please double-check? > Full results can be found at > http://testbot.winehq.org/JobDetails.pl?Key=11350 This one seems to be a limitation in the WTB: the failing machines don't have the infosoft DLL. Can be checked in the wintest.exe results of those machines, they show all "infosoft missing". > Your paranoid android. > > > === WVISTAADM (32 bit infosoft) === > infosoft.c:183: Test failed: failed > infosoft.c:197: Test failed: failed > > === W2K8SE (32 bit infosoft) === > infosoft.c:183: Test failed: failed > infosoft.c:197: Test failed: failed > > === W7PRO (32 bit infosoft) === > infosoft.c:183: Test failed: failed > infosoft.c:197: Test failed: failed > > === W7PROX64 (32 bit infosoft) === > infosoft.c:183: Test failed: failed > infosoft.c:197: Test failed: failed > > === W7PROX64 (64 bit infosoft) === > infosoft.c:183: Test failed: failed > infosoft.c:197: Test failed: failed bye michael

June 1, 2011

Re: [4/5] ddraw: Extend the lifetime of the IDirect3D7::EnumDevices strings beyond function scope.

by Stefan Dösinger

Am 31.05.2011 um 18:00 schrieb Andrew Nguyen: > No, I simply defined the structure there to be consistent with the role > of the unused structure definitions I removed earlier. I can certainly > move the definition to ddraw.c. Yes, please. And yeah, the existing code could use some improvement in this regard too, but that is of course beyond the scope of this patch.

June 1, 2011