random flakiness

2011-08-19 Thread Dan Kegel
When running a test bot, one notices flaky things that
otherwise happen too seldom to be noticed.

For instance, today, the buildbot crashed once during wineboot:

wine: created the configuration directory
'/home/dank/wineslave.dir/sandbox/slave/runtests/build/wineprefix'
fixme:storage:create_storagefile Storage share mode not implemented.
err:mscoree:LoadLibraryShim error reading registry key for installroot
...
err:mscoree:LoadLibraryShim error reading registry key for installroot
fixme:iphlpapi:NotifyAddrChange (Handle 0x83e8ec, overlapped 0x83e8d0): stub
wine: Unhandled page fault on write access to 0x at address
0x7d8eb051 (thread 0010), starting debugger...
err:process:__wine_kernel_init boot event wait timed out

When I got home, the crash dialog was still up.
ps says the app that crashed was:
C:\windows\system32\rundll32.exe setupapi,InstallHinfSection
DefaultInstall 128
\\?\unix/home/dank/wineslave.dir/sandbox/slave/runtests/build/tools/wine.inf

There have been several other random little failures, e.g.
http://bugs.winehq.org/show_bug.cgi?id=28108,
http://bugs.winehq.org/show_bug.cgi?id=28133

I guess I'll just grit my teeth and ignore these until they happen
more than once, then try to file useful bugs about them.
Who knows, maybe it's my system.
- Dan




Re: mshtml: Use wininet to get and parse the proxy

2011-08-19 Thread Dmitry Timoshkov
André Hentschel  wrote:

> +if(!InternetCrackUrlA(pi->lpszProxy, 0, 0, &UrlComponents)) return;
> +heap_free(pi);

If InternetCrackUrlA() fails pi is leaked.

-- 
Dmitry.




Re: [PATCH 01/10] cmd: Use CSTR_* instead of hardcoded values as result of CompareStringW

2011-08-19 Thread Dan Kegel
2011/8/19 Frédéric Delanoy :
> The last [10/10] patch was the cause. I sent an updated version.

Hmm.  You sent 10/10 before 1/10.  My patch series recognizer
rejected the series.  It would be hard to fix.  Let's see if we can
live with the rule "patch series must be sent in order".

> I wonder how/whether the various (resend) or (try N) are handled for
> revised patch series

My recognizer does not notice those designations, nor the time or
order of arrival.
It only looks at the 'Date:' and 'From:' fields, and the '[%d/%d]'
part of the subject line.
(The exact regular expression I use is \[\D*(\d+)/(\d+)\D*\] .)

Humans prefer (resend) for unchanged resends, and (try N+1) for
changed resends, I think.
- Dan




Re: [1/2] cmd: fixing an issue with extra whitespaces after cd parameter (retry2)

2011-08-19 Thread Frédéric Delanoy
On Fri, Aug 19, 2011 at 02:46, Nowres Rafid  wrote:
>
   else {
+pos = string;
 /* Remove any double quotes, which may be in the
middle, eg. cd "C:\Program Files"\Microsoft is ok */
-pos = string;

> That's a gratuitous change

 while (*command) {
   if (*command != '"') *pos++ = *command;
   command++;
 }
 *pos = 0x00;

+/* Remove any whitespace at the end of command
+This was because "cd .. " wasn't working */
+pos--;
+while ( *pos == ' ' || *pos == '\t' )
+*pos-- = 0x00;
+

You don't need to alter the string more than necessary.
I would do sthg like

 while (*command) {
   if (*command != '"') *pos++ = *command;
   command++;
 }
+pos--;
+while (*pos == ' ' || *pos == '\t') pos--;
 *pos = 0x00;




Re: [2/2] cmd/tests: fixing an issue with extra whitespaces after cd parameter (retry)

2011-08-19 Thread Frédéric Delanoy
You should use @space@ elements to make the tests clearer.

Also, you shouldn't use (retry) in your patch mail subject, but rather (try N)




Re: [PATCH 01/10] cmd: Use CSTR_* instead of hardcoded values as result of CompareStringW

2011-08-19 Thread Frédéric Delanoy
2011/8/19 Dan Kegel :
> Hi Frédéric,
> your patch series seems to have failed the cmd tests:
>
> batch.c:259: Test failed: unexpected char 0x53 position 0 in line 301
> (got 'Syntax error', wanted '1')
> batch.c:259: Test failed: unexpected char 0x31 position 0 in line 302
> (got '1', wanted '3')
> batch.c:259: Test failed: unexpected char 0x33 position 0 in line 303
> (got '3', wanted '5')
>
> See http://buildbot.kegel.com/builders/runtests/builds/86
>
> I'll change the buildbot so it tests each subset of a patch series,
> and pinpoint the one that broke things.

The last [10/10] patch was the cause. I sent an updated version.

I wonder how/whether the various (resend) or (try N) are handled for
revised patch series

Frédéric




Re: What gstreamer plugins are actually needed?

2011-08-19 Thread Maarten Lankhorst
On 08/20/2011 12:15 AM, Scott Ritchie wrote:
> I'm trying to cleanup a bit of a packaging mess whereby Wine currently
> pulls in 250 or so gstreamer plugins.
>
> Which ones are actually useful for applications?  Do we even know?
>
> Thanks,
> Scott Ritchie
>
>
wine-gstreamer only needs decoding and parser filters, and 
gstreamer-plugins-base, but honestly I'd leave in as many as possible because 
you never know which one wine wants. I honestly don't know about the subsets, 
it seems people file bugs about tons of exotic codecs because older games have 
a wide variety of those..

~Maarten




Re: Buildbot status

2011-08-19 Thread Dan Kegel
Hi Seth,
yum, yes, please send me your changes.  I'd be happy to add gentoo
prerequisite handling.

I don't think the build slave needs any incoming connections at all,
it always connects to the master, not the other way around.

What graphics card do you have?
- Dan

On Fri, Aug 19, 2011 at 3:32 PM, Seth Shelnutt  wrote:
> Dan,
>
> Great job with this. I've got a 64bit E6300 I can setup as a build slave.
> It's up and on 24/7 folding (gpu) so the cpu sites mostly idle. I've already
> adapted your scripts to work with gentoo/funtoo. I just replaced the apt-get
> with emerge commands. I can send those changes to you if you want? I don't
> know if you'd want to implement os detection based installation of the
> prereqs or its easier to just leave it alone.
>
> The only thing about setting up a build slave would be I'd have to run it
> off of port 53. My ISP (cox) blocks all inbound ports except 22,23 and 53.
> Port 53 is reserved for DNS gateway, but since I don't run one that isn't an
> issue on mine end. Do you have anything against running on port 53?
>
>
> Thanks,
>
> Seth Shelnutt
>
> On Thu, Aug 18, 2011 at 7:29 PM, Dan Kegel  wrote:
>>
>> http://buildbot.kegel.com/ caught a couple of real problems today.
>>
>> Remember the original patchwatcher?  It died because only
>> one person knew how to run it, and it was too hard to set up.
>> I'm trying to not make that mistake this time.
>> To that end, I've made the process easier, and would like as
>> many people as possible to try running buildbot slaves (and, at least
>> temporarily, masters!) so the knowledge is spread around.
>> It's probably ready for other people to try running one.
>> If you're interested, see
>>
>> http://code.google.com/p/winezeug/source/browse/trunk/buildbot/readme-wine.txt
>>
>> Changes since last post
>> - made cleaner script and doc for adding a new slave
>> - rewrote patch series recognizer (old one was broken and opaque)
>>
>> to-do list:
>> - make it start automatically after reboot
>> - sandbox it
>> - turn on build result emails
>> - add a bit of parallelism into the test runner, so e.g. tests that
>> don't need the display or network run in parallel with those that do,
>> probably in a separate wineprefix
>> - add valgrind slave
>> - add windows slave (like testbot, but with real graphics and sound)
>> - add x86_64 slave
>> - move master to winehq.org once it seems ready
>>
>>
>
>




Re: Buildbot status

2011-08-19 Thread Seth Shelnutt
Dan,

Great job with this. I've got a 64bit E6300 I can setup as a build slave.
It's up and on 24/7 folding (gpu) so the cpu sites mostly idle. I've already
adapted your scripts to work with gentoo/funtoo. I just replaced the apt-get
with emerge commands. I can send those changes to you if you want? I don't
know if you'd want to implement os detection based installation of the
prereqs or its easier to just leave it alone.

The only thing about setting up a build slave would be I'd have to run it
off of port 53. My ISP (cox) blocks all inbound ports except 22,23 and 53.
Port 53 is reserved for DNS gateway, but since I don't run one that isn't an
issue on mine end. Do you have anything against running on port 53?


Thanks,

Seth Shelnutt

On Thu, Aug 18, 2011 at 7:29 PM, Dan Kegel  wrote:

> http://buildbot.kegel.com/ caught a couple of real problems today.
>
> Remember the original patchwatcher?  It died because only
> one person knew how to run it, and it was too hard to set up.
> I'm trying to not make that mistake this time.
> To that end, I've made the process easier, and would like as
> many people as possible to try running buildbot slaves (and, at least
> temporarily, masters!) so the knowledge is spread around.
> It's probably ready for other people to try running one.
> If you're interested, see
>
> http://code.google.com/p/winezeug/source/browse/trunk/buildbot/readme-wine.txt
>
> Changes since last post
> - made cleaner script and doc for adding a new slave
> - rewrote patch series recognizer (old one was broken and opaque)
>
> to-do list:
> - make it start automatically after reboot
> - sandbox it
> - turn on build result emails
> - add a bit of parallelism into the test runner, so e.g. tests that
> don't need the display or network run in parallel with those that do,
> probably in a separate wineprefix
> - add valgrind slave
> - add windows slave (like testbot, but with real graphics and sound)
> - add x86_64 slave
> - move master to winehq.org once it seems ready
>
>
>



What gstreamer plugins are actually needed?

2011-08-19 Thread Scott Ritchie
I'm trying to cleanup a bit of a packaging mess whereby Wine currently
pulls in 250 or so gstreamer plugins.

Which ones are actually useful for applications?  Do we even know?

Thanks,
Scott Ritchie




Re: mshtml: Use the last colon in proxy url as port separator

2011-08-19 Thread Juan Lang
> just sent a patch doing that.

Looks good to me, thanks!
--Juan




Re: mshtml: Use the last colon in proxy url as port separator

2011-08-19 Thread André Hentschel
Am 18.08.2011 22:50, schrieb Juan Lang:
>> Is the ProxyServer specified as an URL? If yes then use the proper API
>> to dissect the URL instead of cobbling something together.
> 
> Just to follow up in this idea, you could use InternetCrackUrl.
> mshtml delay loads wininet, but it also loads urlmon, which also loads
> wininet, so in effect wininet is always loaded and available.
> 
> But a larger question is, why load the proxy settings from the
> registry only?  If you query wininet for them instead, then the
> environment variable http_proxy is also checked.  There was bug 5625
> about this, which I set to fixed based on commit
> 80f02b82d68902f32578a7bcf6cfbaa715b724ce.  I may have been mistaken.
> Under what circumstances is the network.proxy.http gecko setting still
> being used?  Maybe for embedded content, e.g. IMG tags?  If these were
> set by querying wininet instead, then at least we wouldn't have
> potentially different proxy settings for some HTTP requests than for
> others.
> --Juan

just sent a patch doing that.
I'm not sure for what winegecko uses it, but that function was added by 
intention and jacek already modified it a bit, so there must be a reason.

-- 

Best Regards, André Hentschel




Re: [PATCH 01/10] cmd: Use CSTR_* instead of hardcoded values as result of CompareStringW

2011-08-19 Thread Dan Kegel
2011/8/19 Frédéric Delanoy :
>> Incidentally, that 'Syntax error' probably should go to stderr;
>> and since the cmd tests ignore stderr, that might fix your failure.
>
> They should eventually go to stderr, but I wouldn't change them right now.
> The current tests can run successfully only if the # lines match in
> the output / expected files, so putting them to stderr instead might/will 
> break
> things.

Do our tests pass on windows cmd?  That outputs errors to stderr, I think.

> I don't know how your code works, and it's probably done already, but
> in certain cases if tests fail, some test files/dirs can be left in
> the test directory, so a preliminary cleanup is probably a wise idea.

The buildbot always does clean builds/tests.
- Dan




Re: [PATCH 01/10] cmd: Use CSTR_* instead of hardcoded values as result of CompareStringW

2011-08-19 Thread Frédéric Delanoy
2011/8/19 Dan Kegel :
> Hi Frédéric,
> your patch series seems to have failed the cmd tests:
>
> batch.c:259: Test failed: unexpected char 0x53 position 0 in line 301
> (got 'Syntax error', wanted '1')
> batch.c:259: Test failed: unexpected char 0x31 position 0 in line 302
> (got '1', wanted '3')
> batch.c:259: Test failed: unexpected char 0x33 position 0 in line 303
> (got '3', wanted '5')
>
> See http://buildbot.kegel.com/builders/runtests/builds/86
>
> I'll change the buildbot so it tests each subset of a patch series,
> and pinpoint the one that broke things.
>
> Incidentally, that 'Syntax error' probably should go to stderr;
> and since the cmd tests ignore stderr, that might fix your failure.

They should eventually go to stderr, but I wouldn't change them right now.
The current tests can run successfully only if the # lines match in
the output / expected files, so putting them to stderr instead might/will break
things.
Also, they are (only?) generated for perfectly valid constructs due to
parsing bugs in current code.

I don't know how your code works, and it's probably done already, but
in certain cases if tests fail, some test files/dirs can be left in
the test directory, so a preliminary cleanup is probably a wise idea.

Frédéric




Re: regedit: adding confirmation message before importing from file

2011-08-19 Thread Nowres Rafid
On Wed, 2011-08-17 at 08:19 -0700, Dan Kegel wrote:
> On Wed, Aug 17, 2011 at 8:17 AM, Dan Kegel  wrote:
> > Hi Nowres,
> > did you run the regedit tests,
> >   cd programs/regedit/tests
> >   make test
> > after that change?  Here it fails with
> >  regedit.c:384: Test failed: regedit not available, skipping regedit tests
> > ( log at http://buildbot.kegel.com/builders/runtests/builds/2 )
> > You might need to fully implement the /S switch for regedit and use it
> > from the tests before you can add the confirmation dialog to regedit.
> 
> Ah, of course, the tests already use /S, since they pass on windows.
> So you just need to make your dialog not show up when /S is given.

I updated the patch.
can you please check it?
>From 938512a27114da8698bdc2d4ca9afd55bd034113 Mon Sep 17 00:00:00 2001
From: Nowres Rafid 
Date: Thu, 18 Aug 2011 00:56:37 +
Subject: regedit: asking for confirmation before importing a file

---
 programs/regedit/regedit.c  |   51 --
 programs/regedit/regedit.rc |7 ++
 programs/regedit/resource.h |3 ++
 3 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/programs/regedit/regedit.c b/programs/regedit/regedit.c
index cc0fc3a..7cd167b 100644
--- a/programs/regedit/regedit.c
+++ b/programs/regedit/regedit.c
@@ -18,10 +18,14 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
  */
 
-#include 
-#include 
 #include 
+#include 
+#include 
+#include 
+
+#include "resource.h"
 #include "regproc.h"
+#include "wine/unicode.h"
 
 static const char *usage =
 "Usage:\n"
@@ -40,7 +44,6 @@ static const char *usage =
 "	file. Exports the whole registry if no key is specified.\n"
 "/D - deletes specified registry key\n"
 "/S - silent execution, can be used with any other switch.\n"
-"	Default. The only existing mode, exists for compatibility with Windows regedit.\n"
 "/V - advanced mode, can be used with any other switch.\n"
 "	Ignored, exists for compatibility with Windows regedit.\n"
 "/L - location of system.dat file. Can be used with any other switch.\n"
@@ -122,7 +125,7 @@ static void get_file_name(CHAR **command_line, CHAR *file_name)
 (*command_line) += pos;
 }
 
-static BOOL PerformRegAction(REGEDIT_ACTION action, LPSTR s)
+static BOOL PerformRegAction(REGEDIT_ACTION action, LPSTR s, BOOL silent)
 {
 switch (action) {
 case ACTION_ADD: {
@@ -142,6 +145,7 @@ static BOOL PerformRegAction(REGEDIT_ACTION action, LPSTR s)
 if (strcmp(filename, "-") == 0)
 {
 reg_file = stdin;
+import_registry_file(reg_file);
 }
 else
 {
@@ -159,6 +163,7 @@ static BOOL PerformRegAction(REGEDIT_ACTION action, LPSTR s)
 getAppName(), filename, GetLastError());
 exit(1);
 }
+
 reg_file = fopen(realname, "r");
 if (reg_file == NULL)
 {
@@ -166,8 +171,34 @@ static BOOL PerformRegAction(REGEDIT_ACTION action, LPSTR s)
 fprintf(stderr, "%s: Can't open file \"%s\"\n", getAppName(), filename);
 exit(1);
 }
+
+if (silent == FALSE)
+{
+if (MessageBoxW(NULL, MAKEINTRESOURCEW(IDS_IMPORT_BOX_TEXT),
+MAKEINTRESOURCEW(IDS_APP_TITLE),
+MB_YESNO | MB_ICONEXCLAMATION) == IDYES)
+{
+if (import_registry_file(reg_file))
+{
+MessageBoxW(NULL, MAKEINTRESOURCEW(IDS_IMPORT_SUCCESS_BOX_TEXT),
+MAKEINTRESOURCEW(IDS_APP_TITLE),
+MB_OK | MB_ICONINFORMATION);
+}
+else
+{
+MessageBoxW(NULL, MAKEINTRESOURCEW(IDS_IMPORT_FAIL_BOX_TEXT),
+MAKEINTRESOURCEW(IDS_APP_TITLE),
+MB_OK | MB_ICONERROR);
+}
+}
+}
+else
+{
+import_registry_file(reg_file);
+}
+
 }
-import_registry_file(reg_file);
+
 if (realname)
 {
 HeapFree(GetProcessHeap(),0,realname);
@@ -252,6 +283,7 @@ BOOL ProcessCmdLine(LPSTR lpCmdLine)
 REGEDIT_ACTION action = ACTION_UNDEF;
 LPSTR s = lpCmdLine;/* command line pointer */
 CHAR ch = *s;   /* current character */
+BOOL silent = FALSE;
 
 while (ch && ((ch == '-') || (ch == '/'))) {
 char chu;
@@ -268,8 +30

re: [PATCH 01/10] cmd: Use CSTR_* instead of hardcoded values as result of CompareStringW

2011-08-19 Thread Dan Kegel
Hi Frédéric,
your patch series seems to have failed the cmd tests:

batch.c:259: Test failed: unexpected char 0x53 position 0 in line 301
(got 'Syntax error', wanted '1')
batch.c:259: Test failed: unexpected char 0x31 position 0 in line 302
(got '1', wanted '3')
batch.c:259: Test failed: unexpected char 0x33 position 0 in line 303
(got '3', wanted '5')

See http://buildbot.kegel.com/builders/runtests/builds/86

I'll change the buildbot so it tests each subset of a patch series,
and pinpoint the one that broke things.

Incidentally, that 'Syntax error' probably should go to stderr;
and since the cmd tests ignore stderr, that might fix your failure.




Only use D3DLOCK_DISCARD if dynamic memory (was Re: d3dx9: CloneMesh test and improvements)

2011-08-19 Thread Michael Mc Donnell
On Fri, Aug 19, 2011 at 2:20 PM, Stefan Dösinger  wrote:
> Am 19.08.2011 um 14:04 schrieb Michael Mc Donnell:
>> Ok I see that in dlls/d3d9/tests/buffers.c. So it will fail to lock on
>> Win7 if D3DLOCK_DISCARD is specified and the vertex buffer wasn't
>> created with D3DUSAGE_DYNAMIC?
> Yes, unless Microsoft changed this again.
>
>> Am I correct in D3DLOCK_DISCARD makes a copy of the vertex buffer so
>> that the video card can keep reading the old one, and the old vertex
>> buffer memory is released after the video card releases its lock [1]?
> Sort of. It doesn't copy the old data, it just gives you a fresh, 
> uninitialized block of memory. So if you read from the pointer you get from a 
> DISCARD lock you'll get garbage.
>
>> In that case it does not make sense to use D3DLOCK_DISCARD at all in
>> CloneMesh, as the video card has not begun to use the new vertex
>> buffer, and it will just add memory overhead. The easiest fix would be
>> to just pass 0 instead of D3DLOCK_DISCARD. I've done that in the
>> updated patch.
> Right, the buffer is newly created in CloneMesh. Just not using DISCARD is 
> the best fix then.
>
>>> I noticed a few more D3DLOCK_DISCARD locks in the mesh.c code.
>>
>> I guess the other methods should check the mesh options for
>> D3DXMESH_DYNAMIC, D3DXMESH_VB_DYNAMIC, and D3DXMESH_IB_DYNAMIC [2]
>> before using D3DLOCK_DISCARD? I'll write patches for those.
> Most likely yes. I haven't looked at the details. If the buffer is new in 
> those situations as well just don't pass any flags, otherwise pass DISCARD if 
> you lock a dynamic buffer and are going to rewrite the entire buffer contents.

All of the places with D3DLOCK_DISCARD in mesh.c were working on newly
created meshes or vertex buffers, so they should all pass 0. I've
attached patches for it.
From 161a8d36437b2b950642f8cca4012b2792d4b385 Mon Sep 17 00:00:00 2001
From: Michael Mc Donnell 
Date: Fri, 19 Aug 2011 16:48:18 +0200
Subject: d3dx9: Use 0 instead of D3DLOCK_DISCARD in OptimizeInPlace

It does not make sense to use D3DLOCK_DISCARD for locking a newly created
vertex buffer because it will be allocated, freed, and then allocated again.
Furthermore, D3DLOCK_DISCARD should only be used in conjuction with dynamic
memory, which the current implementation does not check for. Using the flag
D3DLOCK_DISCARD with static memory causes failure on Windows7 (see dlls/d3d9/
tests/buffer.c).
---
 dlls/d3dx9_36/mesh.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/dlls/d3dx9_36/mesh.c b/dlls/d3dx9_36/mesh.c
index 9a0fd03..ed42883 100644
--- a/dlls/d3dx9_36/mesh.c
+++ b/dlls/d3dx9_36/mesh.c
@@ -1293,7 +1293,7 @@ static HRESULT WINAPI ID3DXMeshImpl_OptimizeInplace(ID3DXMesh *iface, DWORD flag
 hr = IDirect3DVertexBuffer9_Lock(This->vertex_buffer, 0, 0, (void**)&orig_vertices, D3DLOCK_READONLY);
 if (FAILED(hr)) goto cleanup;
 
-hr = IDirect3DVertexBuffer9_Lock(vertex_buffer, 0, 0, (void**)&new_vertices, D3DLOCK_DISCARD);
+hr = IDirect3DVertexBuffer9_Lock(vertex_buffer, 0, 0, (void**)&new_vertices, 0);
 if (FAILED(hr)) {
 IDirect3DVertexBuffer9_Unlock(This->vertex_buffer);
 goto cleanup;
-- 
1.7.6

From a3d069449d7b4d54b2847b8de6af447f65e709c5 Mon Sep 17 00:00:00 2001
From: Michael Mc Donnell 
Date: Fri, 19 Aug 2011 17:21:30 +0200
Subject: d3dx9: Use 0 instead of D3DLOCK_DISCARD in load_skin_mesh_from_xof

It does not make sense to use D3DLOCK_DISCARD for locking newly created
buffers because they will be allocated, freed, and then allocated again.
---
 dlls/d3dx9_36/mesh.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/dlls/d3dx9_36/mesh.c b/dlls/d3dx9_36/mesh.c
index ed42883..2462422 100644
--- a/dlls/d3dx9_36/mesh.c
+++ b/dlls/d3dx9_36/mesh.c
@@ -2966,7 +2966,7 @@ static HRESULT load_skin_mesh_from_xof(IDirectXFileData *filedata,
 hr = D3DXCreateMeshFVF(mesh_data.num_tri_faces, total_vertices, options, mesh_data.fvf, device, &d3dxmesh);
 if (FAILED(hr)) goto cleanup;
 
-hr = d3dxmesh->lpVtbl->LockVertexBuffer(d3dxmesh, D3DLOCK_DISCARD, &vertices);
+hr = d3dxmesh->lpVtbl->LockVertexBuffer(d3dxmesh, 0, &vertices);
 if (FAILED(hr)) goto cleanup;
 
 out_ptr = vertices;
@@ -3008,7 +3008,7 @@ static HRESULT load_skin_mesh_from_xof(IDirectXFileData *filedata,
 }
 d3dxmesh->lpVtbl->UnlockVertexBuffer(d3dxmesh);
 
-hr = d3dxmesh->lpVtbl->LockIndexBuffer(d3dxmesh, D3DLOCK_DISCARD, (void**)&indices);
+hr = d3dxmesh->lpVtbl->LockIndexBuffer(d3dxmesh, 0, (void**)&indices);
 if (FAILED(hr)) goto cleanup;
 
 index_in_ptr = mesh_data.indices;
@@ -3037,7 +3037,7 @@ static HRESULT load_skin_mesh_from_xof(IDirectXFileData *filedata,
 
 if (mesh_data.material_indices) {
 DWORD *attrib_buffer = NULL;
-hr = d3dxmesh->lpVtbl->LockAttributeBuffer(d3dxmesh, D3DLOCK_DISCARD, &attrib_buffer);
+hr = d3dxmesh->lpVtbl->LockAttributeBuffer(d3dxmesh, 0, &attrib_buffer);

The sad state of audio GetPosition

2011-08-19 Thread Joerg-Cyril . Hoehle
Hi,

here's just for fun an interpretation of some test data:

A native machine passes all my tests in shared mode.

In exclusive mode, that machine behaves like PulseAudio in Wine now :-)
render.c:797: BufferSize 21846 frames
render.c:799: Test failed: BufferSize 21846 too small for duration at rate 44100
render.c:807: Clock Frequency 44100
render.c:872: Test failed: Position 21829 too far after 100ms
render.c:882: padding 0 past sleep #2

It ate 495ms worth of samples (the completely prefilled buffer - mysterious 17)
within 100ms after Start then returned a local underrun (padding 0)!
Clearly a bug in MS-Windows or some driver.

If you start with an underrun like my loop does, then
that machine returns data that appears valid:
render.c:1033: padding 1324 position 19648 slept 470ms iteration 0


Testbot shows how MS "fixed" a bug between Vista and w2k8R2/w7 in exclusive 
mode:
render.c:1086: Test failed: GetCurrentPadding returned 576, should be 0
render.c:1092: Test failed: Position 90720 at end vs. 91296 played frames
Somewhat I found the old behavior more consistent.
After all, 90720 + 576 = 91296

What's noteworthy is that only a multiple of period frames appears to
get written.  Unlike shared mode, the remainder will be left in the
buffer, even if you wait for an underrun to occur (like my tests do).

IOW, native players using exclusive mode are well-advised to add a few
silence frames at the end of the samples, or they won't be heard.
At least, that's what I infer from
render.c:1092: Test failed: Position 90720 at end vs. 91296 played frames
since I've never put my hands on a native system new enough to have mmdevapi.


Testbot confirms again that you must not trust timers in vmware,
which I already experienced with the MCI tests.
render.c:1040: hpctime 828 pcpos 843
render.c:1033: padding 16560 position 50832 slept 870ms iteration 4
render.c:1034: Test failed: Position 50832 too far after 870ms
render.c:1035: Test failed: Position delta 7392 not regular
render.c:1040: hpctime 937 pcpos 952

Within 109ms its speaker position advanced by 7392 frames,
whereas 5232 would be normal at 48000 in that time frame.

Other than that, data looks good.

It will be no fun to correct the tests to accept testbot's daily results on 
test.winehq.
Currently I lean towards
if (winetest_debug > 0)
position-tests
such that testbot's patch watcher skips over position-based
tests which we would know pass on native.

Regards,
Jörg Höhle




The sad state of audio GetPosition

2011-08-19 Thread Joerg-Cyril . Hoehle
Maarten Lankhorst wrote:
[nice to hear from you]

>>  IMHO AudioClient_Stop must not map to 
>> snd_pcm_drop.  It is more like snd_pcm_pause. Or perhaps simply lead 
>> ALSA into an underrun.
>afaict pause, with reset mapped to drop,
Indeed.  But I believe I need a fallback because ALSA says that
pause "works only on the hardware which supports" it.


>I can't remember why pause didn't work, but if it works go for it.
I was solely thinking aloud that pause is TRT, not tried out yet.

However, I received test results from a "Windows 7 Ultimate" machine.
It exhibits a similar bug -- in exclusive mode only:
render.c:948: Test failed: Position 18191 too far after 100ms
Shared mode works as my tests expect it (<= 48000/10 frames).


>> +snd_pcm_status_alloca(&status);
>HeapAlloc(GetProcessHeap(), HEAP_ZERO_FLAG, snd_pcm_status_sizeof())
> or something like that if available please..
Really? I don't want to go through the overhead of memory allocation
when all I need is a stupid small amount of stack allocated memory.


>> +if(!This->initted){
>> +return AUDCLNT_E_NOT_INITIALIZED;
>Unneeded part.
Can't you obtain a handle to that COM object prior to
calling Initialize which sets This->fmt?

>Follow that flow..
I beg your pardon?


>> +if(0){
>> +avail_frames = snd_pcm_status_get_avail(status);
>> +delay_frames = snd_pcm_status_get_delay(status);
>if 0 is bad...

I tried out pcm_status because somebody in alsa-devel mentioned that
it allows to grab avail + delay in one (sync'ed?).
However, I found delay to be always 0 inside status!?!

Also, I found out that I need to call avail_update and delay
in a particular order, otherwise I get stale values from an old call
prior to the last sleep...


>> +if(avail_frames <= This->bufsize_alsa + MAX_LATE_SECONDS * 
>> This->fmt->nSamplesPerSec
>> +   && delay_frames > 0)
>Isn't delay_frames < 0 the definition of underrun?
Indeed.
There are potentially N distinct underruns:
 - the front end -- what snd_pcm_avail_update knows about;
 - intermittent buffers (USB);
 - the speaker -- what snd_pcm_delay knows about.
There could be a short front-end buffer underrun that
goes unnoticed by the speaker if the TCP or USB in
between buffers enough data *and* is able to speed up.

>no point in adding MAX_LATE_SECONDS

That is some form of guard against broken values.  E.g. people reporting
in alsa-devel that PA sometimes complains about avail ~ MAXINT and such weird 
values.

>Getting an avail update again? Why?
The theory is:
position = written_frames(into ALSA) - delay
and translates to:
This->written_frames - This->held_frames - delay

However sometimes I can't trust delay.
I still need to figure out when.
 - IIRC after an underrun, snd_pcm_delay yields error X.
 - or was it before starting?
 - ...

The upper bound on position is always:
This->written_frames(ReleaseBuffer) - This->held_frames - ALSA_padding
 (what ALSA's front end has not yet processed, in absence of underrun).

Perhaps that would be robust:
1. Compute upper bound
2. position = clamp(0, delay > 0 ? written-delay : written, upper_bound);
2b. except when not yet started ...
2c. except while stopped ...

I was even considering:
3. if position < This->previous_position stick to previous...

Yet perhaps it's better to allow intermittent garbage values
than to stick to garbage!

OTOH, the delay values I see in the logs are subject to such variation (with PA)
that I'm considering going with a clock instead, or perhaps:
 - query delay once per tick (e.g. 10ms) => last_pos
 - when asked, compute position from last_pos + time since tick * rate

The last_pos slot may be needed anyway once stopped in pause mode.

Regards,
Jörg Höhle




re: shlwapi: Put correct return values in SetFromTimeInterval

2011-08-19 Thread Dan Kegel
(The buildbot worked its way back to the day after Alexandre went on vacation.
Even though the patches are old, git hasn't changed, so they ought to
apply still.)

It didn't apply here:

patching file dlls/shlwapi/string.c
patching file dlls/shlwapi/tests/string.c
Hunk #1 FAILED at 622.
Hunk #2 FAILED at 639.
Hunk #3 FAILED at 649.
3 out of 3 hunks FAILED -- saving rejects to file
dlls/shlwapi/tests/string.c.rej

Log at
http://buildbot.kegel.com/builders/runtests/builds/80
for the next day or so.




Re: About cmd patches

2011-08-19 Thread Dan Kegel
On Fri, Aug 19, 2011 at 2:15 AM, Nowres Rafid  wrote:
>> That means that your patch series
>> [1/2] cmd: fixing an error with redirection operators
>> [2/2] cmd/tests: fixing an error with redirection operators (updating
>> test results)
>>
>> is incorrect, since tests marked todo_wine will fail after the first
>> patch.
>
> i queued these patch in my email client, but when I sent them they were sent
> in bad order.

It doesn't matter what order they were sent in, I think.
There's no way that applying one of those patches would
yield working tests.

If you want to do a two patch series, you can, but every
patch in the series has to pass tests on its own.
That may mean sending new tests first with extra todo's,
then removing the todo's in the second patch which fixes them.

In this case, you weren't adding any new tests, so there's no
first test patch in the series; you really should just send
a single merged patch.
- Dan




Re: The sad state of audio GetPosition

2011-08-19 Thread Maarten Lankhorst
On 08/19/2011 01:30 PM, joerg-cyril.hoe...@t-systems.com wrote:
> Reece,
>
> I wrote the text in a hurry and forgot that Wine results
> are most interesting to me with my patch applied.
>
> Please use my patch with something like:
> WINETEST_DEBUG=3 WINEDEBUG=warn+alsa wine mmdevapi_test.exe render
>
>> render.c:897: Test failed: Position 24000 too far after 200ms
> That's not PA's fault.  IMHO AudioClient_Stop must not map
> to snd_pcm_drop.  It is more like snd_pcm_pause. Or perhaps
> simply lead ALSA into an underrun.  I've not made up my mind
> yet as the models (mmdevapi vs. ALSA) are different w.r.t. buffering.
afaict pause, with reset mapped to drop, but for historic reasons
that didn't work (surprise! love from dmix). Even worse,
snd_pcm_drain would deadlock if called twice.

I can't remember why pause didn't work, but if it works go for it.
>
> As you can see, the patch is nowhere final.
>
> #From 60689763bd21513bd9b8dbd2df3abc5f2586f1f2 Mon Sep 17 00:00:00 2001
> #From: =?UTF-8?q?J=C3=B6rg=20H=C3=B6hle?= 
> Date: Wed, 17 Aug 2011 21:04:34 +0200
> Subject: winealsa: Play with GetPosition.
>
> ---
>  dlls/winealsa.drv/mmdevdrv.c |   52 ++---
>  1 files changed, 43 insertions(+), 9 deletions(-)
>
> diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c
> index 3e3edc3..51e9b81 100644
> --- a/dlls/winealsa.drv/mmdevdrv.c
> +++ b/dlls/winealsa.drv/mmdevdrv.c
> @@ -2289,26 +2289,60 @@ static HRESULT WINAPI 
> AudioClock_GetPosition(IAudioClock *iface, UINT64 *pos,
>  UINT64 *qpctime)
>  {
>  ACImpl *This = impl_from_IAudioClock(iface);
> -UINT32 pad;
> -HRESULT hr;
> +int err;
> +snd_pcm_uframes_t avail_frames;
> +snd_pcm_sframes_t delay_frames, pad_frames;
> +snd_pcm_status_t *status;
>  
>  TRACE("(%p)->(%p, %p)\n", This, pos, qpctime);
>  
>  if(!pos)
>  return E_POINTER;
> +snd_pcm_status_alloca(&status);
HeapAlloc(GetProcessHeap(), HEAP_ZERO_FLAG, snd_pcm_status_sizeof()) or 
something like that if available please..
>  EnterCriticalSection(&This->lock);
>  
> -hr = IAudioClient_GetCurrentPadding(&This->IAudioClient_iface, &pad);
> -if(FAILED(hr)){
> +if(!This->initted){
>  LeaveCriticalSection(&This->lock);
> -return hr;
> +return AUDCLNT_E_NOT_INITIALIZED;
>  }
Unneeded part. Follow that flow..

>  
> -if(This->dataflow == eRender)
> -*pos = This->written_frames - pad;
> -else if(This->dataflow == eCapture)
> -*pos = This->written_frames + pad;
> +if((err = snd_pcm_status(This->pcm_handle, status)) < 0){
> +LeaveCriticalSection(&This->lock);
> +ERR("ALSA status error: %d (%s)\n",
> +err, snd_strerror(err));
> +return E_FAIL;
> +}
> +if(0){
> +avail_frames = snd_pcm_status_get_avail(status);
> +delay_frames = snd_pcm_status_get_delay(status);
> +}else{
> +avail_frames = snd_pcm_avail_update(This->pcm_handle);
> +err = snd_pcm_delay(This->pcm_handle, &delay_frames);
> +if(err < 0){ /* e.g. in STATE_PREPARED */
> +ERR("ALSA delay error: %d (%s)\n",
> +err, snd_strerror(err));
> +delay_frames = 0;
> +}
> +}

if 0 is bad...
> +pad_frames = This->bufsize_alsa - avail_frames;
> +#define MAX_LATE_SECONDS 5  /* huge USB or network latency */
> +if(avail_frames <= This->bufsize_alsa + MAX_LATE_SECONDS * 
> This->fmt->nSamplesPerSec
> +   && delay_frames > 0)
> +*pos = This->written_frames - This->held_frames - delay_frames;
Isn't delay_frames < 0 the definition of underrun? no point in adding 
MAX_LATE_SECONDS

> +else if(pad_frames > 0)
> +/* delay may be slightly < 0 past reset */
> +*pos = This->written_frames - This->held_frames - pad_frames;
> +else
> +*pos = This->written_frames - This->held_frames;
> +/* FIXME: if(This->dataflow == eCapture) */

> +ERR("avail %lu, delay %ld, sum %ld, alsa %lu, written %lu, held %u: 
> %lu\n",
> +avail_frames, delay_frames, avail_frames+delay_frames, 
> This->bufsize_alsa, (ulong)This->written_frames, This->held_frames, 
> (ulong)*pos);
> +avail_frames = snd_pcm_avail_update(This->pcm_handle);
> +err = snd_pcm_delay(This->pcm_handle, &delay_frames);
> +ERR("avail %lu, delay %ld, sum %ld, alsa %lu, written %lu, held %u: 
> %lu\n",
> +avail_frames, delay_frames, avail_frames+delay_frames, 
> This->bufsize_alsa, (ulong)This->written_frames, This->held_frames, 
> (ulong)*pos);
>  
>  LeaveCriticalSection(&This->lock);
>  
Getting an avail update again? Why?




Re: d3dx9: CloneMesh test and improvements

2011-08-19 Thread Stefan Dösinger
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1


Am 19.08.2011 um 14:04 schrieb Michael Mc Donnell:
> Ok I see that in dlls/d3d9/tests/buffers.c. So it will fail to lock on
> Win7 if D3DLOCK_DISCARD is specified and the vertex buffer wasn't
> created with D3DUSAGE_DYNAMIC?
Yes, unless Microsoft changed this again.

> Am I correct in D3DLOCK_DISCARD makes a copy of the vertex buffer so
> that the video card can keep reading the old one, and the old vertex
> buffer memory is released after the video card releases its lock [1]?
Sort of. It doesn't copy the old data, it just gives you a fresh, uninitialized 
block of memory. So if you read from the pointer you get from a DISCARD lock 
you'll get garbage.

> In that case it does not make sense to use D3DLOCK_DISCARD at all in
> CloneMesh, as the video card has not begun to use the new vertex
> buffer, and it will just add memory overhead. The easiest fix would be
> to just pass 0 instead of D3DLOCK_DISCARD. I've done that in the
> updated patch.
Right, the buffer is newly created in CloneMesh. Just not using DISCARD is the 
best fix then.

>> I noticed a few more D3DLOCK_DISCARD locks in the mesh.c code.
> 
> I guess the other methods should check the mesh options for
> D3DXMESH_DYNAMIC, D3DXMESH_VB_DYNAMIC, and D3DXMESH_IB_DYNAMIC [2]
> before using D3DLOCK_DISCARD? I'll write patches for those.
Most likely yes. I haven't looked at the details. If the buffer is new in those 
situations as well just don't pass any flags, otherwise pass DISCARD if you 
lock a dynamic buffer and are going to rewrite the entire buffer contents.

-BEGIN PGP SIGNATURE-
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)

iQIcBAEBAgAGBQJOTlUYAAoJEN0/YqbEcdMwi3kP/AwlIBwuD8XpvAnpVJMmFyM8
lZCJ5zrEbBypYPSin3lEv7yiA+tw2YQwD4ODoZHRZCLKi7Gy07EI2FLAX34KAhxa
ESNXanQuMWaglkj2QfV7aJan8plHdKhTA6HZl3VFCGSLHNjbGWzDXpO60TKTJU3F
jU0Iqnyxq2KSdDU0R6gMsIHKF6wLBkVMOheGTXO2Mc2VwBMt85js+T9+tSUKvKTR
ncVhvnygGLhyB0QfprFvN70If+BKQIicseJm6zO9nIt8/qqmtd6t9QYneEY+HIJI
rZMpHCwxRKmGnCJuJfXnZDtRqUTsT2S7iZuIDUi7xH+It79uWvFEP+Cs/bB2ssqF
kyw1kmTVSi99vbmiQRn4wBGv9C32L9C64SUwNH/nQg7E0iO9zdQxLE3oU64v4lVs
OaTYuIghLxrDaR9+28yLjt97oBvxPLvrhuve2ASpevz+6Fa3n40o1OTBi3ch+Q70
xOBe1Qh9lX40C56b2XVx+j5XfqrNFl1BS/Tluvcs66mgcdTKeZp4ATjx5Js9c1eT
/JsfBiARrGEZ1pVYLRbUY1oky+sq64/sSFsjs8QMerfhms1kTk8oziTTvSw0uT5H
UCF3MadXFwrFnif/kPQ4hfBKr29E28lB/JSp/tb7Rxtim7Se2F8a6I5kmrIwjRoQ
nWko6eM0FzSMR+OiZbsz
=SQxW
-END PGP SIGNATURE-




Re: d3dx9: CloneMesh test and improvements

2011-08-19 Thread Michael Mc Donnell
On Thu, Aug 18, 2011 at 8:05 PM, Stefan Dösinger  wrote:
> Hi,
>
> I noticed one small issue:
> On Saturday 13 August 2011 12:22:03 Michael Mc Donnell wrote:
>> +static HRESULT convert_vertex_buffer(ID3DXMesh *mesh_dst, ID3DXMesh
> *mesh_src)
>> ...
>> +    hr = mesh_dst->lpVtbl->LockVertexBuffer(mesh_dst, D3DLOCK_DISCARD,
> (void**)&vb_dst);
> On paper, D3DLOCK_DISCARD is only valid on D3DUSAGE_DYNAMIC buffers. Wine and
> Windows <= WinVista don't enforce this. Win7 does, and it broke some apps that
> way.

Ok I see that in dlls/d3d9/tests/buffers.c. So it will fail to lock on
Win7 if D3DLOCK_DISCARD is specified and the vertex buffer wasn't
created with D3DUSAGE_DYNAMIC?

Am I correct in D3DLOCK_DISCARD makes a copy of the vertex buffer so
that the video card can keep reading the old one, and the old vertex
buffer memory is released after the video card releases its lock [1]?

In that case it does not make sense to use D3DLOCK_DISCARD at all in
CloneMesh, as the video card has not begun to use the new vertex
buffer, and it will just add memory overhead. The easiest fix would be
to just pass 0 instead of D3DLOCK_DISCARD. I've done that in the
updated patch.

> I noticed a few more D3DLOCK_DISCARD locks in the mesh.c code.

I guess the other methods should check the mesh options for
D3DXMESH_DYNAMIC, D3DXMESH_VB_DYNAMIC, and D3DXMESH_IB_DYNAMIC [2]
before using D3DLOCK_DISCARD? I'll write patches for those.

[1] 
http://msdn.microsoft.com/en-us/library/bb147263(v=vs.85).aspx#Using_Dynamic_Vertex_and_Index_Buffers
[2] http://msdn.microsoft.com/en-us/library/bb205370(v=vs.85).aspx
From e52cb6d31e7c8fbb3dd7e93fbf64adf90fad807e Mon Sep 17 00:00:00 2001
From: Michael Mc Donnell 
Date: Wed, 17 Aug 2011 12:50:09 +0200
Subject: d3dx9: Implemented non-equal declaration support in CloneMesh.

Don't use D3DLOCK_DISCARD it only adds memory overhead in CloneMesh.
---
 dlls/d3dx9_36/mesh.c |  457 ++
 1 files changed, 425 insertions(+), 32 deletions(-)

diff --git a/dlls/d3dx9_36/mesh.c b/dlls/d3dx9_36/mesh.c
index 9a0fd03..26c726f 100644
--- a/dlls/d3dx9_36/mesh.c
+++ b/dlls/d3dx9_36/mesh.c
@@ -65,6 +65,27 @@ typedef struct ID3DXMeshImpl
 D3DXATTRIBUTERANGE *attrib_table;
 } ID3DXMeshImpl;
 
+const UINT d3dx_decltype_size[] =
+{
+   /* D3DDECLTYPE_FLOAT1*/ sizeof(FLOAT),
+   /* D3DDECLTYPE_FLOAT2*/ sizeof(D3DXVECTOR2),
+   /* D3DDECLTYPE_FLOAT3*/ sizeof(D3DXVECTOR3),
+   /* D3DDECLTYPE_FLOAT4*/ sizeof(D3DXVECTOR4),
+   /* D3DDECLTYPE_D3DCOLOR  */ sizeof(D3DCOLOR),
+   /* D3DDECLTYPE_UBYTE4*/ 4 * sizeof(BYTE),
+   /* D3DDECLTYPE_SHORT2*/ 2 * sizeof(SHORT),
+   /* D3DDECLTYPE_SHORT4*/ 4 * sizeof(SHORT),
+   /* D3DDECLTYPE_UBYTE4N   */ 4 * sizeof(BYTE),
+   /* D3DDECLTYPE_SHORT2N   */ 2 * sizeof(SHORT),
+   /* D3DDECLTYPE_SHORT4N   */ 4 * sizeof(SHORT),
+   /* D3DDECLTYPE_USHORT2N  */ 2 * sizeof(USHORT),
+   /* D3DDECLTYPE_USHORT4N  */ 4 * sizeof(USHORT),
+   /* D3DDECLTYPE_UDEC3 */ 4, /* 3 * 10 bits + 2 padding */
+   /* D3DDECLTYPE_DEC3N */ 4,
+   /* D3DDECLTYPE_FLOAT16_2 */ 2 * sizeof(D3DXFLOAT16),
+   /* D3DDECLTYPE_FLOAT16_4 */ 4 * sizeof(D3DXFLOAT16),
+};
+
 static inline ID3DXMeshImpl *impl_from_ID3DXMesh(ID3DXMesh *iface)
 {
 return CONTAINING_RECORD(iface, ID3DXMeshImpl, ID3DXMesh_iface);
@@ -260,6 +281,401 @@ static HRESULT WINAPI ID3DXMeshImpl_CloneMeshFVF(ID3DXMesh *iface, DWORD options
 return iface->lpVtbl->CloneMesh(iface, options, declaration, device, clone_mesh);
 }
 
+static FLOAT scale_clamp_ubyten(FLOAT value)
+{
+value = value * UCHAR_MAX;
+
+if (value < 0.0f)
+{
+return 0.0f;
+}
+else
+{
+if (value > UCHAR_MAX) /* Clamp at 255 */
+return UCHAR_MAX;
+else
+return value;
+}
+}
+
+static FLOAT scale_clamp_shortn(FLOAT value)
+{
+value = value * SHRT_MAX;
+
+/* The tests show that the range is SHRT_MIN + 1 to SHRT_MAX. */
+if (value <= SHRT_MIN)
+{
+return SHRT_MIN + 1;
+}
+else if (value > SHRT_MAX)
+{
+ return SHRT_MAX;
+}
+else
+{
+return value;
+}
+}
+
+static FLOAT scale_clamp_ushortn(FLOAT value)
+{
+value = value * USHRT_MAX;
+
+if (value < 0.0f)
+{
+return 0.0f;
+}
+else
+{
+if (value > USHRT_MAX) /* Clamp at 65535 */
+return USHRT_MAX;
+else
+return value;
+}
+}
+
+static INT simple_round(FLOAT value)
+{
+int res = (INT)(value + 0.5f);
+
+return res;
+}
+
+static void convert_float4(BYTE *dst, CONST D3DXVECTOR4 *src, D3DDECLTYPE type_dst)
+{
+BOOL fixme_once = FALSE;
+
+switch (type_dst)
+{
+case D3DDECLTYPE_FLOAT1:
+{
+FLOAT *dst_ptr = (FLOAT*)dst;
+*dst_ptr = src->x;
+break;
+}
+case D3DDECLTYPE_FLOAT2:
+{
+D3DXVECTOR2 *dst_ptr = (D3DXVECTOR2*)dst;
+dst_ptr->x = src-

The sad state of audio GetPosition

2011-08-19 Thread Joerg-Cyril . Hoehle
Reece,

I wrote the text in a hurry and forgot that Wine results
are most interesting to me with my patch applied.

Please use my patch with something like:
WINETEST_DEBUG=3 WINEDEBUG=warn+alsa wine mmdevapi_test.exe render

>render.c:897: Test failed: Position 24000 too far after 200ms
That's not PA's fault.  IMHO AudioClient_Stop must not map
to snd_pcm_drop.  It is more like snd_pcm_pause. Or perhaps
simply lead ALSA into an underrun.  I've not made up my mind
yet as the models (mmdevapi vs. ALSA) are different w.r.t. buffering.


As you can see, the patch is nowhere final.

#From 60689763bd21513bd9b8dbd2df3abc5f2586f1f2 Mon Sep 17 00:00:00 2001
#From: =?UTF-8?q?J=C3=B6rg=20H=C3=B6hle?= 
Date: Wed, 17 Aug 2011 21:04:34 +0200
Subject: winealsa: Play with GetPosition.

---
 dlls/winealsa.drv/mmdevdrv.c |   52 ++---
 1 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c
index 3e3edc3..51e9b81 100644
--- a/dlls/winealsa.drv/mmdevdrv.c
+++ b/dlls/winealsa.drv/mmdevdrv.c
@@ -2289,26 +2289,60 @@ static HRESULT WINAPI 
AudioClock_GetPosition(IAudioClock *iface, UINT64 *pos,
 UINT64 *qpctime)
 {
 ACImpl *This = impl_from_IAudioClock(iface);
-UINT32 pad;
-HRESULT hr;
+int err;
+snd_pcm_uframes_t avail_frames;
+snd_pcm_sframes_t delay_frames, pad_frames;
+snd_pcm_status_t *status;
 
 TRACE("(%p)->(%p, %p)\n", This, pos, qpctime);
 
 if(!pos)
 return E_POINTER;
+snd_pcm_status_alloca(&status);
 
 EnterCriticalSection(&This->lock);
 
-hr = IAudioClient_GetCurrentPadding(&This->IAudioClient_iface, &pad);
-if(FAILED(hr)){
+if(!This->initted){
 LeaveCriticalSection(&This->lock);
-return hr;
+return AUDCLNT_E_NOT_INITIALIZED;
 }
 
-if(This->dataflow == eRender)
-*pos = This->written_frames - pad;
-else if(This->dataflow == eCapture)
-*pos = This->written_frames + pad;
+if((err = snd_pcm_status(This->pcm_handle, status)) < 0){
+LeaveCriticalSection(&This->lock);
+ERR("ALSA status error: %d (%s)\n",
+err, snd_strerror(err));
+return E_FAIL;
+}
+if(0){
+avail_frames = snd_pcm_status_get_avail(status);
+delay_frames = snd_pcm_status_get_delay(status);
+}else{
+avail_frames = snd_pcm_avail_update(This->pcm_handle);
+err = snd_pcm_delay(This->pcm_handle, &delay_frames);
+if(err < 0){ /* e.g. in STATE_PREPARED */
+ERR("ALSA delay error: %d (%s)\n",
+err, snd_strerror(err));
+delay_frames = 0;
+}
+}
+pad_frames = This->bufsize_alsa - avail_frames;
+#define MAX_LATE_SECONDS 5  /* huge USB or network latency */
+if(avail_frames <= This->bufsize_alsa + MAX_LATE_SECONDS * 
This->fmt->nSamplesPerSec
+   && delay_frames > 0)
+*pos = This->written_frames - This->held_frames - delay_frames;
+else if(pad_frames > 0)
+/* delay may be slightly < 0 past reset */
+*pos = This->written_frames - This->held_frames - pad_frames;
+else
+*pos = This->written_frames - This->held_frames;
+/* FIXME: if(This->dataflow == eCapture) */
+
+ERR("avail %lu, delay %ld, sum %ld, alsa %lu, written %lu, held %u: %lu\n",
+avail_frames, delay_frames, avail_frames+delay_frames, 
This->bufsize_alsa, (ulong)This->written_frames, This->held_frames, 
(ulong)*pos);
+avail_frames = snd_pcm_avail_update(This->pcm_handle);
+err = snd_pcm_delay(This->pcm_handle, &delay_frames);
+ERR("avail %lu, delay %ld, sum %ld, alsa %lu, written %lu, held %u: %lu\n",
+avail_frames, delay_frames, avail_frames+delay_frames, 
This->bufsize_alsa, (ulong)This->written_frames, This->held_frames, 
(ulong)*pos);
 
 LeaveCriticalSection(&This->lock);
 
-- 
1.7.0.4





Re: Buildbot status

2011-08-19 Thread Jerome Leclanche
Compiler warnings - clang/gcc would be nice I guess. I have a terrible
10-line shell script to generate reports, I sent two to the mailing
list in the past.

As for hosting a buildbot, I can probably do that, I have an ubuntu
x64 8-core i7 machine on a static IP with 98% uptime. Can you ping me
on gtalk/email with details?

(Im not back home for a couple more days though)

Jerome

On Fri, Aug 19, 2011 at 4:36 AM, Dan Kegel  wrote:
> On Thu, Aug 18, 2011 at 5:19 PM, Jerome Leclanche  wrote:
>> Any chances for an automated warning report as well?
>
> What kind of warnings are you interested in - compile or runtime?
>
>> Also, ideally, there would be one build with all --enables and one
>> with all --disables for all this. Theres often been patches breaking
>> build --without-cups or something. Is this possible?
>
> Sure.
>
> It'll happen sooner if you submit a patch to implement it and/or host
> a slave for it :-)
> - Dan
>




Re: mscoree: COM cleanup for the ICLRRuntimeInfo iface.

2011-08-19 Thread Michael Stefaniuc
For those interested in coccinelle

Michael Stefaniuc wrote:
> On 08/18/2011 03:29 PM, Dan Kegel wrote:
>> Seems to cause a test failure here?
> Argh, indeed!
> - (IUnknown*)&runtimes[i]
> + &runtimes->ICLRRuntimeInfo_iface
> 
> The array subscript got dropped and that wasn't a manual change. Need to
> check if I'm hitting a bug in coccinelle or just a "feature".
It was a feature aka I got bitten by the "ptr_to_array" isomorphism.
Disabling that in the respective rule fixed it.

bye
michael

>> make[1]: Entering directory
>> `/home/dank/wineslave.dir/sandbox/slave/runtests/build/dlls/mscoree/tests'
>> ../../../tools/runtest -q -P wine -M mscoree.dll -T ../../.. -p
>> mscoree_test.exe.so metahost.c && touch metahost.ok
>> metahost.c:135: Test failed: got unexpected version L"v1.1.4322"
>> fixme:mscoree:CLRMetaHost_GetRuntime Unrecognized version L"v2.0.0"
>> Command exited with non-zero status 1
>> 0.04user 0.03system 0:00.09elapsed 82%CPU (0avgtext+0avgdata 
>> 27104maxresident)k
>> 0inputs+0outputs (0major+5905minor)pagefaults 0swaps
>> make[1]: *** [metahost.ok] Error 1
>> ../../../tools/runtest -q -P wine -M mscoree.dll -T ../../.. -p
>> mscoree_test.exe.so mscoree.c && touch mscoree.ok
>> mscoree.c:140: Test failed: version is L"v1.1.4322" , expected L"v2.0.50727"
>> mscoree.c:145: Test failed: version is L"v1.1.4322" , expected L"v2.0.50727"
>> fixme:mscoree:CreateConfigStream ((null), 0x32fbb4): stub
>> fixme:mscoree:CreateConfigStream
>> (L"Z:\\home\\dank\\wineslave.dir\\sandbox\\slave\\runtests\\build\\dlls\\mscoree\\tests\\conf.xml",
>> (nil)): stub
>> fixme:mscoree:CreateConfigStream ((null), (nil)): stub
>> fixme:mscoree:CreateConfigStream (L"nonexist.xml", 0x32fbb4): stub
>> fixme:mscoree:CreateConfigStream
>> (L"Z:\\home\\dank\\wineslave.dir\\sandbox\\slave\\runtests\\build\\dlls\\mscoree\\tests\\conf.xml",
>> 0x32fbb4): stub
>> Command exited with non-zero status 2
>> 0.05user 0.02system 0:00.09elapsed 81%CPU (0avgtext+0avgdata 
>> 27296maxresident)k
>> 0inputs+8outputs (0major+5969minor)pagefaults 0swaps
>> make[1]: *** [mscoree.ok] Error 2
>> make[1]: Target `test' not remade because of errors.
>>
>> (Log at http://buildbot.kegel.com:8010/builders/runtests/builds/32 for
>> a day or so.)




Re: About cmd patches

2011-08-19 Thread Nowres Rafid

On 19/08/2011 03:14, Dan Kegel wrote:

Hi Nowres,
it's awesome that you're digging into cmd like this!

The rule with Wine is that
   Each patch you submit shall pass tests.

That means that your patch series
[1/2] cmd: fixing an error with redirection operators
[2/2] cmd/tests: fixing an error with redirection operators (updating
test results)

is incorrect, since tests marked todo_wine will fail after the first patch.

Better to send things like that as a single merged patch in the future.

Thanks,
Dan

Ok Dan,

i queued these patch in my email client, but when I sent them they were 
sent in bad order.


Nowres,




Re: winemenubuilder: appbundle patch 20110618

2011-08-19 Thread Per Johansson
Hi Steven, list,

I tried both AppleScript and shell script versions and AppleScript doesn't 
really add anything, so it's probably better to go with shell script and 
generate the bundle manually.
I'll see if I can help smoothing it out.
-- 
Pelle Johansson

17 aug 2011 kl. 00.06 skrev Steven Edwards:

> You might be interested in this. It allows icons and localized
> resources. I've just not gotten around to doing all the polish.
> 
> 
> -- Forwarded message --
> From: Steven Edwards 
> Date: Sat, Jun 18, 2011 at 5:43 AM
> Subject: winemenubuilder: appbundle patch 20110618
> To: Wine Developers List 
> 
> 
> Hi,
> I've finally gotten back around to hacking the *.app bundle support in
> winemenubuilder and come close to having something that mostly works.
> There are still things that need to be cleaned up and further
> abstracted, and a bit more logic to work out. I wanted to go ahead and
> send this out in case anyone else wants to play around with it, or if
> for some reason I am unable to finish hackin on it.
> 
> Some notes:
> 
> - The first icon/bundle always seems to get the wrong icon, and uses
> the default wine icon. I need to take some time to understand the code
> a bit better and see why this is.
> 
> - Bundles are created in $HOME/Applications/wine and icon's are
> generated using the icns code. Support for building bundles for
> shortcuts on $HOME/Desktop has not been hacked on yet.
> 
> - There are still plenty of bugs to resolve such as making sure we are
> actually using the correct icon, and properly setting various values
> in the app *.plist. There are a couple of alternative ideas I looked
> in to, such as doing some sort of spawn/fork to invoke
> /usr/libexec/PlistBuddy rather than using the CoreFoundation API, but
> I worry about creating race conditions and other performance problems
> when installing software that creates many icons, such as Microsoft
> Office.
> 
> - I've tried to make this un-invasive to the existing xdg code path
> (as one can run a FreeDesktop on Darwin/OS X) but have not had a
> chance to actually test supporting both menu systems at the same time.
> 
> I welcome your feedback.
> Thanks
> --
> Steven Edwards
> 
> "There is one thing stronger than all the armies in the world, and
> that is an idea whose time has come." - Victor Hugo
> 
> 
> 
> -- 
> Steven Edwards
> 
> "There is one thing stronger than all the armies in the world, and
> that is an idea whose time has come." - Victor Hugo
>