January 2012 - Wine-Devel - list.winehq.org

Re: winecoreaudio.drv: Improve underrun handling

by Andrew Eikum

On Mon, Jan 30, 2012 at 11:03:52AM +0100, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > + sc = AudioQueueEnqueueBufferWithParameters(This->aqueue, > + This->public_buffer, 0, NULL, 0, 0, 0, NULL, &req_time, &start_time); > I advise against setting req_time. This can only fragilize the > system. What if req_time is miscomputed or there's a delay? > NULL=ASAP is exactly what we want. We don't want ALSA/dmix behavior, > where it'll silently skip over late frames in order to catch up > (dunno if MacOSX does this but in my eyes that doesn't matter). The AudioQueue API does do this, and that's the behavior I was trying to work around. See bug 29602. Other options like AudioQueueReset() or AudioQueueFlush() don't seem to do what we want, so this is the best solution I found. What do you mean by it doesn't matter? > >if(list_count(...) == 0) > I once wrote a patch to turn such a pattern into if(list_empty(...)) > Oops, I forgot that existed. I'll fix that in the next version. Andrew

Jan. 30, 2012

Re: [PATCH 2/3] winecoreaudio: Implement the lock-free callback design.

by Andrew Eikum

On Mon, Jan 30, 2012 at 11:37:17AM +0100, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > Fixes bug #29657 > While I like the idea of this patch and how you implemented it, I wonder if it's appropriate for code freeze. The main problem in that bug is that the CA driver can deadlock, or release memory out of order. When working on this patch, were you able to identify specifically where the contention is? The idea of the InTransition state was to handle the lock release in mid-call. Can we add a check for that state in the callbacks to solve this problem less intrusively? The first patch seems fine. Andrew

Jan. 30, 2012

Re: dinput: Support INFINITE as number of effect iterations (try 2)

by Marcel Hasler

2012/1/30 Vitaliy Margolen <wine-devel(a)kievinfo.com>: > > I'm afraid you'll have to dig up some documentation about what is the max > value here is then. The fact that it works doesn't mean it's correct. > > Vitaliy. > > The only official documentation I can find is in Documentation/input/ff.txt and it doesn't say anything about the max for 'value'. I had a look at the kernel source instead. input_handle_event() in input.c only checks whether 'value' is >= 0 for EV_FF (which is why the event is ignored for INFINITE), ml_ff_playback() in ff-memless.c sets 'state->count' (which is an 'int') to 'value' and ml_get_combo_effect() finally counts down 'state->count' until it reaches zero. So there really is no limit here (accept what's given by the data type). Marcel

Jan. 30, 2012

Re: mmdevapi: Correctly convert UINT32 to INT64

by Vitaliy Margolen

On 01/30/2012 03:08 AM, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > Michael Stefaniuc wrote: >>> But now I see basestd.h defines LONG32. >> LONG32 is kinda redundant. LONG is 32bit on all Windows >> versions on 16bit, 32bit and 64bit Windows. > Ah, thank you. However in our particular case, the value that > getbuf_last is derived from is UINT32, not UINT, so for symmetry, > I still recommend the explicit LONG32. UINT is the same as UNIT32: basetsd.h:84 typedef unsigned int UINT32, *PUINT32; windef.h:213 typedef unsigned int UINT, *PUINT; Vitaliy.

Jan. 30, 2012

comctl32: fix MSVC build in imagelist.c

by carlo.bramix＠libero.it

If I can say my opinion, I disagree on the usage of HeapAlloc/HeapFree for that known amount of memory. Instead, for granting compatibility with MSVC, I would continue to use a structure in stack memory by simply using _alloca() and something like this: LPBITMAPINFO bmi = (LPBITMAPINFO) _alloca(FIELD_OFFSET(BITMAPINFO, bmiColors[256])); or this: LPBITMAPINFO bmi = (LPBITMAPINFO) _alloca(sizeof(BITMAPINFO)+sizeof(RGBQUAD)*256); and leaving all other lines of code unchanged. Sincerely, Carlo Bramini. > Thomas Faber <thfabba at gmx.de> writes: > >> This removes the use of a Variable-Length Array in ImageList_CreateImage >> and replaces it with HeapAlloc/HeapFree, as it is done in several >> other functions in this file. >> Alas, FIELD_OFFSET is "not const enough" for ANSI C. > >Using sizeof instead would be preferable. > >-- >Alexandre Julliard >julliard at winehq.org >

Jan. 30, 2012

Re: dinput: Support INFINITE as number of effect iterations (try 2)

by Vitaliy Margolen

On 01/29/2012 04:31 PM, Marcel Hasler wrote: > The limit 0x7fff mentioned in input.h doesn't concern the number of > iterations, just durations. I couldn't find anything on an upper limit > for 'value'. I've tested 0x7fffffff and it works just fine. I'm afraid you'll have to dig up some documentation about what is the max value here is then. The fact that it works doesn't mean it's correct. Vitaliy.

Jan. 30, 2012

Rules for code freeze

by Alexandre Julliard

Folks, We are now in code freeze, so all patches that are not obviously harmless need to have a justification as to why the patch should be applied. In most cases, that simply means pointing to the corresponding bug that is fixed by the patch. Patches that seem risky will be marked pending or deferred. If you still think they should go in, you should convince me that they fix something important that is worth the risk. Patches that only change the tests can't break apps so they will be accepted freely, at least until we get closer to release. Patches that require translation updates will only be accepted for the next two weeks, to give translators time to catch up afterwards. I'm planning to make rc releases every week until we run out of bugs. The release schedule may be a bit more random than usual because of travel constraints (not to mention skiing constraints ;-) -- Alexandre Julliard julliard(a)winehq.org

Jan. 30, 2012

Re: comctl32: fix MSVC build in imagelist.c

by Alexandre Julliard

Thomas Faber <thfabba(a)gmx.de> writes: > This removes the use of a Variable-Length Array in ImageList_CreateImage > and replaces it with HeapAlloc/HeapFree, as it is done in several > other functions in this file. > Alas, FIELD_OFFSET is "not const enough" for ANSI C. Using sizeof instead would be preferable. -- Alexandre Julliard julliard(a)winehq.org

Jan. 30, 2012

mmdevapi: Correctly convert UINT32 to INT64

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

Michael Stefaniuc wrote: >>But now I see basestd.h defines LONG32. >LONG32 is kinda redundant. LONG is 32bit on all Windows >versions on 16bit, 32bit and 64bit Windows. Ah, thank you. However in our particular case, the value that getbuf_last is derived from is UINT32, not UINT, so for symmetry, I still recommend the explicit LONG32. Regards, Jörg Höhle

Jan. 30, 2012

winecoreaudio.drv: Improve underrun handling

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

Andrew, The good news is that your underrun patch prevents mmdevapi from writing less and less frames in mmdevapi/tests/render:test_worst_case http://test.winehq.org/data/f0cfa7cf46d7c832fb84555529d1a1b7c49c46c0/mac_jc… >render.c:1631: Released 86436=98x882 -0 frames at 44100 worth 1960ms in 2035ms >render.c:1631: Released 71442=81x882 -0 frames at 44100 worth 1620ms in 2037ms >render.c:1631: Released 56448=64x882 -0 frames at 44100 worth 1280ms in 2039ms That is abnormal behavior. + sc = AudioQueueEnqueueBufferWithParameters(This->aqueue, + This->public_buffer, 0, NULL, 0, 0, 0, NULL, &req_time, &start_time); I advise against setting req_time. This can only fragilize the system. What if req_time is miscomputed or there's a delay? NULL=ASAP is exactly what we want. We don't want ALSA/dmix behavior, where it'll silently skip over late frames in order to catch up (dunno if MacOSX does this but in my eyes that doesn't matter). If you need to memorize a time, I believe it's best stored in our AQBuffer struct. >if(list_count(...) == 0) I once wrote a patch to turn such a pattern into if(list_empty(...)) Regards, Jörg

Jan. 30, 2012