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

Re: [PATCH] loader: Use a volatile pointer in wld_memset().

by Alexandre Julliard

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

Dec. 21, 2012

Re: [1/2] wineps.drv: Add an ability to select device resolution from UI.

by Alexandre Julliard

Dmitry Timoshkov <dmitry(a)baikal.ru> writes: > @@ -224,10 +229,13 @@ typedef struct > static INT_PTR CALLBACK PSDRV_PaperDlgProc(HWND hwnd, UINT msg, > WPARAM wParam, LPARAM lParam) > { > + static const WCHAR resW[] = {'%','d',' ','d','p','i',0}; > + static const WCHAR resxyW[] = {'%','d','x','%','d',' ','d','p','i',0}; The "dpi" string needs to be loaded from resources. > @@ -267,6 +275,59 @@ static INT_PTR CALLBACK PSDRV_PaperDlgProc(HWND hwnd, UINT msg, > } > SendDlgItemMessageA(hwnd, IDD_DUPLEX, CB_SETCURSEL, Cursel, 0); > } > + > + if (list_empty( &di->pi->ppd->Resolutions )) > + { > + WCHAR buf[64]; > + struct item_res_data *data; > + > + sprintfW(buf, resW, di->pi->ppd->DefaultResolution); > + SendDlgItemMessageW(hwnd, IDD_QUALITY, CB_ADDSTRING, 0, (LPARAM)buf); > + > + data = HeapAlloc(GetProcessHeap(), 0, sizeof(*data)); > + data->resx = data->resy = di->pi->ppd->DefaultResolution; > + SendDlgItemMessageW(hwnd, IDD_QUALITY, CB_SETITEMDATA, 0, (LPARAM)data); > + Cursel = 0; You don't need a data structure, you can store the values directly with something like MAKELONG. -- Alexandre Julliard julliard(a)winehq.org

Dec. 21, 2012

dsound: use event based threads, v2

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

Maarten, > + hres = IAudioClient_GetStreamLatency(device->client, &period); > + device->sleeptime = period_ms * 5 / 2; > + ret = WaitForSingleObject(dev->sleepev, dev->sleeptime); Although it's a minor point, as we're solely discussing the case of the timeout when mmdevapi doesn't call SetEvent, I wonder why you insist on using GetStreamLatency as the basis of your timeout computations instead of GetDevicePeriod. You know that wineXYZ.drv use the device period as the basis for their SetEvent, thus IMHO the timeout too should be based on it, instead of a GetStreamLatency we know next to nothing about. If you visit testbot jobs 23521 and 23514, you'll see that I've measured the event rates. Sadly, all testbot VMs report a GetStreamLatency of 10.6666ms too close to the DefaultPeriod of 10.0000ms to tell for sure which is used as the average period. W7 and w2k8 machines seem to use DefaultPeriod: render.c:185: Returned periods: 10.0000 ms 3.0000 ms render.c:333: Returned latency: 10.6666 ms render.c:626: event after Start average 10.004ms render.c:2206: event average 9.992ms sigma 0.447 Vista may use its reported StreamLatency as period: render.c:185: Returned periods: 10.0000 ms 3.0000 ms render.c:333: Returned latency: 10.6666 ms render.c:626: event after Start average 10.697ms render.c:2206: event average 10.736ms sigma 0.834 The one w2k8 machine is peculiar as it appears to use the 15.600ms (or 15.625ms?) timers well known from pre-Vista times: render.c:185: Returned periods: 10.0000 ms 3.0000 ms render.c:333: Returned latency: 10.6666 ms render.c:626: event after Start average 15.547ms render.c:2206: event average 15.625ms sigma 5.625 However, the rate is certainly not the 20-30ms StreamLatency that e.g. winealsa returns. Regards, Jörg Höhle

Dec. 21, 2012

mmdevapi: Avoid lock contention after SetEvent.

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

Andrew Eikum asked: >Is it true that SetEvent() causes a context switch? Looking at +tid log files, I've seen switches happen after SetEvent often enough. That motivated some changes to winmm notification in the last years, i.e. call SetEvent after all local work is finished and objects are in a sane state again. Moreover, the semantics of SetEvent is that it puts any thread waiting for the event into runnable state. If it's runnable, the OS may as well decide to run it. And that's exactly what happens. On a multi-core system, the signaled thread may start running immediately on another core. You don't want it to stop shortly later, blocking on wineXYZ*.drv's sole critical section when invoking mmdevapi. LeaveCriticalSection is another place where context switches happen, both according to log files and by design (see RtlUnWaitCriticalSection). So could moving SetEvent after LeaveCS disturb the dynamic behaviour of winealsa.drv? I argue that there's no harm, because mmdevapi's design is such that the app has nearly one period time after receiving the event to supply new samples. Thus if an app manages to sneak in a call unblocked by LeaveCS, e.g. GetPosition, before SetEvent, there should still be enough time left afterwards. In any case, lock contention by an app calling GetPosition during the period callback is presumably rare, while lock contention by SetEvent vs. GetCurrentPadding in the signaled thread, e.g. DSound is systematic. So let's avoid it. (Please don't skip that patch waiting for the lock-less one which will render it superfluous...). Regards, Jörg Höhle

Dec. 21, 2012

Re: oleaut32: Indentation fix (Try 2)

by Andrew Talbot

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

Dec. 20, 2012

Fwd: [PATCH 3/8] winemac: Implement GetDeviceCaps().

by Per Johansson

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

Dec. 20, 2012

Re: [PATCH 2/2] gdi32: Ensure a fixed-pitch full-width character has double width of a half-width character.

by Alexandre Julliard

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

Dec. 20, 2012

Re: dsound: use event based threads, v2

by Andrew Eikum

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 > > > >

Dec. 20, 2012

Re: [PATCH 3/3] usp10: Add support for format 2 pair adjustments.

by Alexandre Julliard

Dmitry Timoshkov <dmitry(a)baikal.ru> writes: > There are 9 elements of 2 bytes in size each, 18 bytes in total, > the compiler will align this structure to 4 bytes, making its > size 20 bytes. No it won't. -- Alexandre Julliard julliard(a)winehq.org

Dec. 20, 2012

Re: [PATCH 3/3] usp10: Add support for format 2 pair adjustments.

by Dmitry Timoshkov

Huw Davies <huw(a)codeweavers.com> wrote: > > There are 9 elements of 2 bytes in size each, 18 bytes in total, > > the compiler will align this structure to 4 bytes, making its > > size 20 bytes. > > The size is irrelevant - it's variable sized anyway. We just care > about the offsets of the elements. Anyway, in order to avoid any confusion or bugs in future with allocating an array of such structures or using sizeof() on them I'd suggest to explicitely specify alignment. -- Dmitry.

Dec. 20, 2012