December 2012 - Wine-devel - list.winehq.org

Re: [PATCH] loader: Use a volatile pointer in wld_memset().
by Alexandre Julliard 21 Dec '12

21 Dec '12

Charles Davis <cdavis5x(a)gmail.com> writes: > This prevents Clang from optimizing the loop into a memset(3) call. Something like -fno-builtin would be preferable. -- Alexandre Julliard julliard(a)winehq.org

1 0

mmdevapi: Avoid lock contention after SetEvent.
by Joerg-Cyril.Hoehle＠t-systems.com 21 Dec '12

21 Dec '12

Hi, what's up with this patch? I wrote: >SetEvent is one of those calls with a high probability that the >Linux scheduler will switch to another thread, e.g. the one receiving >the event. As the critical section is still hold, it'll prevent the >receiver from invoking mmdevapi, e.g. GetCurrentPadding, without two >more thread switches (forth and back). We don't need that. Now that This->event is constant between IAC_Start and IAC_Release, thanks to my "SetEventHandle is allowed once only" patch, it's safe to move SetEvent(This->event) outside of the CS. Unlike the rejected patch about a CreateTimerQueue race condition internal to Wine (and only relevant in lock-less mode), this one is between the driver and the client app that blocks when calling e.g. GetCurrentPadding, what almost all apps will do, including our DSound http://source.winehq.org/source/dlls/dsound/mixer.c#L679 Regards, Jörg Höhle

2 2

Re: oleaut32: Indentation fix (Try 2)
by Andrew Talbot 20 Dec '12

20 Dec '12

Sorry, I should have labelled this patch as "Try 2". -- Andy.

1 0

Fwd: [PATCH 3/8] winemac: Implement GetDeviceCaps().
by Per Johansson 20 Dec '12

20 Dec '12

Forgot to include list somehow... On Mon, Dec 17, 2012 at 9:16 PM, Alexandre Julliard <julliard(a)winehq.org> wrote: > > My main reason for using it is to test the Mac builds before pushing > commits, so not being able to build the Mac driver on it is a serious > problem (and no, it can't be upgraded apparently). I'll let you guys decide on 10.5 or 10.6, but I'd like to note that all intel Mac should be able to run 10.6, as per Apple's website <http://support.apple.com/kb/SP575>. Regards, -- Per Johansson

1 0

Re: [PATCH 2/2] gdi32: Ensure a fixed-pitch full-width character has double width of a half-width character.
by Alexandre Julliard 20 Dec '12

20 Dec '12

Akihiro Sagawa <sagawa.aki(a)gmail.com> writes: > @@ -5866,10 +5867,22 @@ static DWORD get_glyph_outline(GdiFont *incoming_font, UINT glyph, UINT format, > return GDI_ERROR; > } > > + if (!font->orientation) { > + TEXTMETRICW tm; > + double advance = (ft_face->glyph->metrics.horiAdvance + 63) >> 6; > + get_text_metrics(incoming_font, &tm); > + if (!(tm.tmPitchAndFamily & TMPF_FIXED_PITCH) && > + (tm.tmAveCharWidth * 2) != (INT)(advance * widthRatio) && > + (tm.tmAveCharWidth * 2) == (INT)((advance + 1.0) * widthRatio)) { > + TRACE("add fullwidth character padding\n"); > + padding = 1 << 6; > + } > + } Is it really limited to orientation==0? A test case with a different orientation would be useful. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: dsound: use event based threads, v2
by Andrew Eikum 20 Dec '12

20 Dec '12

Looks good to me, no problems with any of the drivers. I think it'd be good to get it in on Monday, as release day is tomorrow, but that's not my call. Andrew On Wed, Dec 19, 2012 at 10:30:04AM +0100, Maarten Lankhorst wrote: > Use a thread instead of a timer for greater precision. > > Changes since v1: > - Add WARN on failure of Start or GetStreamLatency > - Set lower bound of sleep time to 5 ms > - Change upper bound of sleep time to 2.5x period > - Fixup comment > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst(a)ubuntu.com> > --- > I like how the most useful part of the changelog always gets stripped out. > What's the point of even adding this info here? > > Obligatory "Yes I know the error handling I added isn't perfect, and I did fix that in my failure rework patch" here.. > > --- > dlls/dsound/dsound.c | 21 +++++++++++---------- > dlls/dsound/dsound_private.h | 7 ++++--- > dlls/dsound/mixer.c | 39 +++++++++++++++++++++++---------------- > dlls/dsound/primary.c | 27 ++++++++++++++++++++++++--- > 4 files changed, 62 insertions(+), 32 deletions(-) > > diff --git a/dlls/dsound/dsound.c b/dlls/dsound/dsound.c > index ac85ba2..381d0e1 100644 > --- a/dlls/dsound/dsound.c > +++ b/dlls/dsound/dsound.c > @@ -670,14 +670,13 @@ ULONG DirectSoundDevice_Release(DirectSoundDevice * device) > TRACE("(%p) ref was %u\n", device, ref + 1); > if (!ref) { > int i; > - timeKillEvent(device->timerID); > - timeEndPeriod(DS_TIME_RES); > > - /* The kill event should have allowed the timer process to expire > - * but try to grab the lock just in case. Can't hold lock because > - * secondarybuffer_destroy also grabs the lock */ > - RtlAcquireResourceShared(&(device->buffer_list_lock), TRUE); > - RtlReleaseResource(&(device->buffer_list_lock)); > + SetEvent(device->sleepev); > + if (device->thread) { > + WaitForSingleObject(device->thread, INFINITE); > + CloseHandle(device->thread); > + } > + CloseHandle(device->sleepev); > > EnterCriticalSection(&DSOUND_renderers_lock); > list_remove(&device->entry); > @@ -813,6 +812,7 @@ HRESULT DirectSoundDevice_Initialize(DirectSoundDevice ** ppDevice, LPCGUID lpcG > > device->mmdevice = mmdevice; > device->guid = devGUID; > + device->sleepev = CreateEventW(0, 0, 0, 0); > > hr = DSOUND_ReopenDevice(device, FALSE); > if (FAILED(hr)) > @@ -869,9 +869,10 @@ HRESULT DirectSoundDevice_Initialize(DirectSoundDevice ** ppDevice, LPCGUID lpcG > ZeroMemory(&device->volpan, sizeof(device->volpan)); > > hr = DSOUND_PrimaryCreate(device); > - if (hr == DS_OK) > - device->timerID = DSOUND_create_timer(DSOUND_timer, (DWORD_PTR)device); > - else > + if (hr == DS_OK) { > + device->thread = CreateThread(0, 0, DSOUND_mixthread, device, 0, 0); > + SetThreadPriority(device->thread, THREAD_PRIORITY_TIME_CRITICAL); > + } else > WARN("DSOUND_PrimaryCreate failed: %08x\n", hr); > > *ppDevice = device; > diff --git a/dlls/dsound/dsound_private.h b/dlls/dsound/dsound_private.h > index 62656a5..1a3900c 100644 > --- a/dlls/dsound/dsound_private.h > +++ b/dlls/dsound/dsound_private.h > @@ -68,9 +68,9 @@ struct DirectSoundDevice > > GUID guid; > DSCAPS drvcaps; > - DWORD priolevel; > + DWORD priolevel, sleeptime; > PWAVEFORMATEX pwfx, primary_pwfx; > - UINT timerID, playing_offs_bytes, in_mmdev_bytes, prebuf; > + UINT playing_offs_bytes, in_mmdev_bytes, prebuf; > DWORD fraglen; > LPBYTE buffer; > DWORD writelead, buflen, state, playpos, mixpos; > @@ -97,6 +97,7 @@ struct DirectSoundDevice > IAudioStreamVolume *volume; > IAudioRenderClient *render; > > + HANDLE sleepev, thread; > struct list entry; > }; > > @@ -226,7 +227,7 @@ void DSOUND_AmpFactorToVolPan(PDSVOLUMEPAN volpan) DECLSPEC_HIDDEN; > void DSOUND_RecalcFormat(IDirectSoundBufferImpl *dsb) DECLSPEC_HIDDEN; > DWORD DSOUND_secpos_to_bufpos(const IDirectSoundBufferImpl *dsb, DWORD secpos, DWORD secmixpos, float *overshot) DECLSPEC_HIDDEN; > > -void CALLBACK DSOUND_timer(UINT timerID, UINT msg, DWORD_PTR dwUser, DWORD_PTR dw1, DWORD_PTR dw2) DECLSPEC_HIDDEN; > +DWORD CALLBACK DSOUND_mixthread(void *ptr) DECLSPEC_HIDDEN; > > /* sound3d.c */ > > diff --git a/dlls/dsound/mixer.c b/dlls/dsound/mixer.c > index 84e029b..282808e 100644 > --- a/dlls/dsound/mixer.c > +++ b/dlls/dsound/mixer.c > @@ -820,7 +820,7 @@ static void DSOUND_PerformMix(DirectSoundDevice *device) > DSOUND_PrimaryStop(device); > } > > - } else { > + } else if (device->state != STATE_STOPPED) { > > DSOUND_WaveQueue(device, TRUE); > > @@ -843,22 +843,29 @@ static void DSOUND_PerformMix(DirectSoundDevice *device) > /* **** */ > } > > -void CALLBACK DSOUND_timer(UINT timerID, UINT msg, DWORD_PTR dwUser, > - DWORD_PTR dw1, DWORD_PTR dw2) > +DWORD CALLBACK DSOUND_mixthread(void *p) > { > - DirectSoundDevice * device = (DirectSoundDevice*)dwUser; > - DWORD start_time = GetTickCount(); > - DWORD end_time; > - TRACE("(%d,%d,0x%lx,0x%lx,0x%lx)\n",timerID,msg,dwUser,dw1,dw2); > - TRACE("entering at %d\n", start_time); > - > - RtlAcquireResourceShared(&(device->buffer_list_lock), TRUE); > - > - if (device->ref) > - DSOUND_PerformMix(device); > + DirectSoundDevice *dev = p; > + TRACE("(%p)\n", dev); > > - RtlReleaseResource(&(device->buffer_list_lock)); > + while (dev->ref) { > + DWORD ret; > > - end_time = GetTickCount(); > - TRACE("completed processing at %d, duration = %d\n", end_time, end_time - start_time); > + /* > + * Some audio drivers are retarded and won't fire after being > + * stopped, add a timeout to handle this. > + */ > + ret = WaitForSingleObject(dev->sleepev, dev->sleeptime); > + if (ret == WAIT_FAILED) > + WARN("wait returned error %u %08x!\n", GetLastError(), GetLastError()); > + else if (ret != WAIT_OBJECT_0) > + WARN("wait returned %08x!\n", ret); > + if (!dev->ref) > + break; > + > + RtlAcquireResourceShared(&(dev->buffer_list_lock), TRUE); > + DSOUND_PerformMix(dev); > + RtlReleaseResource(&(dev->buffer_list_lock)); > + } > + return 0; > } > diff --git a/dlls/dsound/primary.c b/dlls/dsound/primary.c > index 3ac928b..dd810fd 100644 > --- a/dlls/dsound/primary.c > +++ b/dlls/dsound/primary.c > @@ -152,6 +152,8 @@ HRESULT DSOUND_ReopenDevice(DirectSoundDevice *device, BOOL forcewave) > REFERENCE_TIME prebuf_rt; > WAVEFORMATEX *wfx = NULL; > HRESULT hres; > + REFERENCE_TIME period; > + DWORD period_ms; > > TRACE("(%p, %d)\n", device, forcewave); > > @@ -192,14 +194,15 @@ HRESULT DSOUND_ReopenDevice(DirectSoundDevice *device, BOOL forcewave) > prebuf_rt = (10000000 * (UINT64)prebuf_frames) / device->pwfx->nSamplesPerSec; > > hres = IAudioClient_Initialize(device->client, > - AUDCLNT_SHAREMODE_SHARED, AUDCLNT_STREAMFLAGS_NOPERSIST, > - prebuf_rt, 0, device->pwfx, NULL); > + AUDCLNT_SHAREMODE_SHARED, AUDCLNT_STREAMFLAGS_NOPERSIST | > + AUDCLNT_STREAMFLAGS_EVENTCALLBACK, prebuf_rt, 0, device->pwfx, NULL); > if(FAILED(hres)){ > IAudioClient_Release(device->client); > device->client = NULL; > WARN("Initialize failed: %08x\n", hres); > return hres; > } > + IAudioClient_SetEventHandle(device->client, device->sleepev); > > hres = IAudioClient_GetService(device->client, &IID_IAudioRenderClient, > (void**)&device->render); > @@ -234,6 +237,24 @@ HRESULT DSOUND_ReopenDevice(DirectSoundDevice *device, BOOL forcewave) > return hres; > } > > + /* Now kick off the timer so the event fires periodically */ > + hres = IAudioClient_Start(device->client); > + if (FAILED(hres)) > + WARN("starting failed with %08x\n", hres); > + > + hres = IAudioClient_GetStreamLatency(device->client, &period); > + if (FAILED(hres)) { > + WARN("GetStreamLatency failed with %08x\n", hres); > + period_ms = 10; > + } else > + period_ms = (period + 9999) / 10000; > + TRACE("period %u ms fraglen %u prebuf %u\n", period_ms, device->fraglen, device->prebuf); > + > + if (period_ms < 3) > + device->sleeptime = 5; > + else > + device->sleeptime = period_ms * 5 / 2; > + > return S_OK; > } > > @@ -379,7 +400,7 @@ HRESULT DSOUND_PrimaryPlay(DirectSoundDevice *device) > TRACE("(%p)\n", device); > > hr = IAudioClient_Start(device->client); > - if(FAILED(hr)){ > + if(FAILED(hr) && hr != AUDCLNT_E_NOT_STOPPED){ > WARN("Start failed: %08x\n", hr); > return hr; > } > -- > 1.8.0 > > > >

1 0

Re: [PATCH 3/3] usp10: Add support for format 2 pair adjustments.
by Dmitry Timoshkov 20 Dec '12

20 Dec '12

Huw Davies <huw(a)codeweavers.com> wrote: > typedef struct { > + WORD PosFormat; > + WORD Coverage; > + WORD ValueFormat1; > + WORD ValueFormat2; > + WORD ClassDef1; > + WORD ClassDef2; > + WORD Class1Count; > + WORD Class2Count; > + WORD Class1Record[1]; > +} GPOS_PairPosFormat2; Shouldn't this structure be 2 bytes aligned? -- Dmitry.

3 6

winepulse test data review
by Joerg-Cyril.Hoehle＠t-systems.com 20 Dec '12

20 Dec '12

Hi, Maarten Lankhorst wrote: >Actually in winepulse, sleep / 12000 (8 ms I guess?) ms would work, no sleep at all works too, Yes, I wanted to go no further than 8ms below the 10ms period limit. > it was just the mixing of various levels of sleep that was failing. Not sure why though, > might be something about the scheduling throwing it off. There's a perfectly simple explanation of this that applies to both winepulse and winecoreaudio. It goes as follows. IOW, there's a bug in winecoreaudio. It may explain why the test stutters on my MacOS 10.5.8 machine, regardless of how accurate or "improved" GetPosition is. winecoreaudio's IAC_ReleaseBuffer sends packets off as soon as it receives data, unlike winealsa/wineoss. Consider the sleep sequence 0 5 8 0 5 ms... + The transition 8 -> 0ms sleep is fine, as data is sent 2ms after the prior packet - The transition 0 -> 5 is most problematic, as it means that 2 ReleaseBuffer will be 15ms apart, even though only 10ms of data was sent. I believe winepulse is affected by this too, like winecoreaudio. The stable sequences "always 0" or "always 8ms sleep" are paced regularly, so underruns are avoided as long as the OS schedules the threads soon enough. Winealsa and wineOSS are not affected, as they write data on the 10ms period boundaries, -- like native presumably does. winecoreaudio solution paths: - write packets on period tick, not at release time - or add silence lead-in - or delay first packet until period tick only (if small) - or ... I think it should be possible to produce reliable continuous sound in winepulse and winecoreaudio yet maintain their current "send packet" codebase instead of transitioning to a separate buffer that winealsa and wineoss use. Regards, Jörg Höhle

3 2

Re: msxml3: Add custom handling for DISPID_VALUE for node map
by Jacek Caban 20 Dec '12

20 Dec '12

On 12/19/12 08:24, Nikolay Sivov wrote: > Add custom handling for DISPID_VALUE for node map If this has to be done for all IDispatchEx objects with DISPID_VALUE, it may be worth considering changing dispex.c. Something like: if(id == DISPID_VALUE && wFlags == DISPATCH_METHOD) wFlags |= DISPATCH_PROPERTYGET; before ITypeInfo::Invoke call should do the trick and replace both this patch and already committed node list patch. Cheers, Jacek

2 3

Re: [PATCH 1/2] oleaut32/tests: Add test for ITypeInfo2_fnGetContainingTypeLib (Try 3, resend)
by Nikolay Sivov 20 Dec '12

20 Dec '12

On 12/20/2012 14:50, Tatyana Fokina wrote: Hi, Tatyana. Main problem is that your test will crash if run without patch 2/2, that's not how it should be done. You need to merge these patches together to keep tests working or send implementation fix first, so next test patch will work. > + hr = ITypeInfo2_GetContainingTypeLib(ti2, (ITypeLib **)&tl, &Index); > + ok(hr == S_OK, " ITypeInfo2_GetContainingTypeLib failed on Index\n"); > + if(tl) ITypeLib_Release(tl); You don't need a cast here. > + hr = ITypeInfo2_GetContainingTypeLib(ti2, NULL, &Index); > + ok(hr == S_OK, " ITypeInfo2_GetContainingTypeLib failed on ITypeLib = NULL\n"); Please test index value here and above to make sure it really returns something.

1 0