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

Re: [PATCH 3/3] winecoreaudio.drv: Use AUHAL API instead of AudioQueue

by Ken Thomases

Hi, I'm concerned about the use of the AudioConverter API within the capture callback (ca_capture_cb). I have it embedded in my memory that it's not permitted to call a converter on the device thread. I can't find a hard and fast declaration of that in the documentation, although Technical Note TN2091: Device input using the HAL Output Audio Unit does say this: <https://developer.apple.com/library/mac/technotes/tn2091/_index.html> "If sample rate conversion is needed, it can be accomplished by buffering the input and converting the data on a separate thread with another AudioConverter." If what you're doing were allowed, it would seem they would have mentioned that and not suggested the use of "a separate thread". It also seems that the HAL Output Audio Unit would just do this itself and not have the restriction on the sample rate in the first place. I could have sworn that the API would actually generate errors if you tried to use it on the AUHAL's thread, but I may be thinking of the AUConverter audio unit, instead. Ah, after more searching, I believe it was for the AUConverter audio unit. There's an error, kAudioUnitErr_CannotDoInCurrentContext, that is generated if you try to use such a unit or if you try to make the AudioOutputUnit do the conversion itself. Besides my specific recollection that sample rate conversion is not allowed, there's the general requirement to avoid slow operations in those callbacks. The call to AudioConverterFillComplexBuffer() may be such a slow operation. So could the calls to malloc() in your ca_capture_cb() and feed_cb(). Could you not move the sample rate conversion to the application thread, at the point where the application calls in to get the captured data (or metadata about it)? In AudioClient_IsFormatSupported(), is it really the case that any format for which a stream description can be constructed is supported? Is there a need to try setting the format on an AudioConverter to see if it will accept it? There are still two mentions of AudioQueue in lines like: WARN("AudioQueue doesn't support per-channel volume control\n"); -Ken

June 30, 2014

Re: d3dx9: Implement D3DXCreateTorus() + tests

by Matteo Bruni

Hi Gediminas, 2014-06-30 19:29 GMT+02:00 Gediminas Jakutis <gediminas(a)varciai.lt>: > --- > dlls/d3dx9_36/d3dx9_36.spec | 2 +- > dlls/d3dx9_36/mesh.c | 129 +++++++++++++++++++++++++++++++ > dlls/d3dx9_36/tests/mesh.c | 183 ++++++++++++++++++++++++++++++++++++++++++++ > include/d3dx9shape.h | 2 + > 4 files changed, 315 insertions(+), 1 deletion(-) > +HRESULT WINAPI D3DXCreateTorus(struct IDirect3DDevice9 *device, + float innerradius, float outerradius, UINT sides, UINT rings, struct ID3DXMesh **mesh, ID3DXBuffer **adjacency) The "usual" style is 8 spaces for line continuations (that also applies to some other places in the patch). + float sin; + float cos; Not really an issue but I'd prefer if these wouldn't shadow the functions from math.h. + if (!device || innerradius < 0.0f || outerradius < 0.0f || sides < 3 || rings < 3 || !mesh) + { + return D3DERR_INVALIDCALL; + } Not strictly required but you can either avoid the curly braces (here and below) or add a WARN for the error return condition. Probably a WARN here is nicer. + rotation_step = D3DX_PI / sides * -2.0f; + rotation_curr = 0; Please use 0.0f there instead. + for (i = 0; i < sides; ++i) + { + sin = sinf(rotation_curr); + cos = cosf(rotation_curr); + vertices[i].position.x = innerradius * cos + outerradius; + vertices[i].position.y = 0.0f; + vertices[i].position.z = innerradius * -sin; + vertices[i].normal.x = cos; + vertices[i].normal.y = 0.0f; + vertices[i].normal.z = -sin; + + rotation_curr += rotation_step; + } I might be mistaken but I think you can remove the "-" from the previous "-2.0f" and from both "-sin" here. + for (i = 0; i < numfaces - sides * 2; ++i) + { + faces[i][0] = i % 2 ? i / 2 + sides : i / 2; + faces[i][1] = (i / 2 + 1) % sides ? i / 2 + 1 : i / 2 + 1 - sides; + if ((i + 1 >= sides * 4) && !((i + 1) % sides) && !((i + 1) / sides % 2)) + { + faces[i][2] = (i + 1) / 2; + } + else if (i) + { + faces[i][2] = (i + 1) % (sides * 2) ? (i + 1) / 2 + sides : (i / sides) * sides; + } + else + { + faces[i][2] = sides; + } + } Can't the last "else" just be dropped (and the condition from the previous "else" removed)? I.e. the first "else" for the i == 0 case is equivalent to the second one, AFAICS. Actually the first two branches also look to be the same so this should be enough for the third vertex of each face: faces[i][2] = (i + 1) % (sides * 2) ? (i + 1) / 2 + sides : (i + 1) / 2; + for (j = 0; i < numfaces; ++i, ++j) + { + faces[i][0] = i % 2 ? j / 2 : sides * (rings - 1) + j / 2; This should be equivalent: faces[i][0] = i % 2 ? j / 2 : i / 2; I don't mind all that much for this one though. + faces[i][1] = (i / 2 + 1) % sides ? i / 2 + 1 : i / 2 + 1 - sides; + faces[i][2] = i == numfaces - 1 ? 0 : (j + 1) / 2; + } I guess this separate loop for the last "strip" of the mesh is there to avoid a bunch of "% numvert" in the previous loop. That's okay for me. + hr = D3DXCreateTorus(NULL, 0.0f, 0.0f, 3, 3, &torus, NULL); + ok(hr == D3DERR_INVALIDCALL, "Got result %x, expected %x (D3DERR_INVALIDCALL)\n",hr,D3DERR_INVALIDCALL); Missing spaces after the commas. The notes regarding vertices and faces generation also apply to compute_torus in the test obviously. For some reason your patch has DOS-style CRLF line terminators and that made "git apply" fail for me. Testbot didn't complain though so probably that's not an issue. Generally it's preferred to generate patches with "git format-patch" or even better directly "git send-email".

June 30, 2014

Re: Wine freetext search no longer working

by Jeremy Newman

OK, I'll recover the missing files from the old machine. Should happen later today. -N On 06/29/2014 06:44 AM, Hugh McMaster wrote: > As of 29 June 2014, the freetext source code search option at http://source.winehq.org/search is not working. > > The error message is "Unexpected return value 2 from Glimpse". > > A quick search on the internet reveals the following from http://lxr.sourceforge.net/en/troubleshooting.shtml: > > Unexpected return value x from Glimpse > Glimpse was unable to complete the free-text search query. > If x is 2, files are inaccessible. These usually are the auxiliary caches, which may be missing or have incorrect access rights. The relevant directory is defined by the 'glimpsedir' or 'glimpsedirbase' configuration parameters. > > There is no problem with the identifier search option. > > -- > Hugh McMaster > >

June 30, 2014

Re: [PATCH] riched20: Don't create too many IRichEditOle interfaces for a RichEdit. (try 3)

by Jactry Zeng

Hi, Another try was sent: http://source.winehq.org/patches/data/105376 2014-06-26 19:10 GMT+08:00 Jactry Zeng <jactry92(a)gmail.com>: > Hi Huw, > 2014-06-26 15:54 GMT+08:00 Huw Davies <huw(a)codeweavers.com>: > > > > > > > I'm sure you can think about how to simplify this a bit. > > The clue that it needs simplification is that you've > > got two AddRef()s in there. > > > > Hint: The code is trying to do two things: > > > > 1. If needed, assign an interface to reOle > > 2. AddRef and return. > > > > > --- a/dlls/riched20/richole.c > > > +++ b/dlls/riched20/richole.c > > > @@ -126,12 +126,13 @@ IRichEditOle_fnRelease(IRichEditOle *me) > > > > > > TRACE ("%p ref=%u\n", This, ref); > > > > > > - if (!ref) > > > + if (ref == 1) > > > { > > > TRACE ("Destroying %p\n", This); > > > This->txtSel->reOle = NULL; > > > ITextSelection_Release(&This->txtSel->ITextSelection_iface); > > > > IOleClientSite_Release(&This->clientSite->IOleClientSite_iface); > > > + This->editor->reOle = NULL; > > > > No, this is wrong. This function should remain unchanged. > > > > The editor is in charge of the reOle reference, so it > > needs to release it when the editor is destroyed - > > ie in ME_DestroyEditor(). > > > > I will try again. > Thanks for your review. :) > > > -- > Regards, > Jactry Zeng > > -- Regards, Jactry Zeng

June 30, 2014

Re: RFC: patch 105133

by Henri Verbeet

On 30 June 2014 16:12, Jérôme Gardou <jerome.gardou(a)reactos.org> wrote: > Oh... I thought it at least tried to apply the patch and run the test suite > with the patched wine binary. We'd like that in the future, but currently it only runs the tests on Windows.

June 30, 2014

Re: RFC: patch 105133

by Jérôme Gardou

Le 30.06.2014 16:06, Henri Verbeet a écrit : > On 30 June 2014 16:01, Jérôme Gardou <jerome.gardou(a)reactos.org> wrote: >> According to the patch status page, the following patch is still marked as >> "new". >> http://source.winehq.org/patches/data/105133 >> Is there any prioritisation for the testbot regarding the jobs it processes >> ? >> > There is, but since that patch doesn't touch the tests the testbot is > not really involved. Oh... I thought it at least tried to apply the patch and run the test suite with the patched wine binary.

June 30, 2014

Re: RFC: patch 105133

by Henri Verbeet

On 30 June 2014 16:01, Jérôme Gardou <jerome.gardou(a)reactos.org> wrote: > According to the patch status page, the following patch is still marked as > "new". > http://source.winehq.org/patches/data/105133 > Is there any prioritisation for the testbot regarding the jobs it processes > ? > There is, but since that patch doesn't touch the tests the testbot is not really involved.

June 30, 2014

RFC: patch 105133

by Jérôme Gardou

Hello wine team. According to the patch status page, the following patch is still marked as "new". http://source.winehq.org/patches/data/105133 Is there any prioritisation for the testbot regarding the jobs it processes ? Also, that put aside, I'd like to discuss the approach and the problem there as I understand it: with "modern" stubs, allocations and frees are made per argument. If there is no prioritisation on how this is done, the "freeer" of one argument can rely on data which were freed before in the loop and overwritten, thus causing the mayhem in the heap that can be witnessed here (see https://bugs.winehq.org/show_bug.cgi?id=36743) The patch tries to overcome the problem and frees the arguments marked as "MustFree" before anything else. I believe this is the right approach, but a comment from someone more proficient as me in RPC matters would be greatly appreciated. Regards Jérôme

June 30, 2014

Re: [PATCH 3/5] d3d8: Check the device status in present calls.

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 https://testbot.winehq.org/JobDetails.pl?Key=7768 Your paranoid android. === w8 (32 bit device) === device.c:2397: Test failed: Expected foreground window 000301BA, got 0002004C. device.c:3039: Test failed: Expected wndproc 0x412780, got 0x412780.

June 30, 2014

Re: [PATCH 2/5] d3d8: Improve d3d8_device_TestCooperativeLevel().

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 https://testbot.winehq.org/JobDetails.pl?Key=7767 Your paranoid android. === w8 (32 bit device) === device.c:2397: Test failed: Expected foreground window 000301BA, got 0002004C. device.c:3039: Test failed: Expected wndproc 0x412780, got 0x412780.

June 30, 2014