September 2011 - Wine-Devel

Re: winecfg: Remove driver selection from Audio tab.

by Michael Stefaniuc

Vitaliy Margolen wrote: > On 09/27/2011 12:17 PM, Michael Stefaniuc wrote: >> On 09/27/2011 03:20 AM, Vitaliy Margolen wrote: >>> Could we please wait another week (until after the wineconf) before >>> doing these whole sale changes? >>> >>> Please leave sound system and related winecfg code as-is. >> Why? >> >> Come to think of it this would be the right time to do it: >> - We are at the start of the commit period for 1.3.30 >> - People will be busy traveling to/from WineConf so a regression won't >> halt their work. >> - Andrew won't travel so he can fix regressions. >> - We can directly motivate Andrew to fix those regressions "Oh, >> regressions still open? No dinner for you tonight!". > > Exactly because of all the regressions introduced with such a huge change. > > I'm not a big fan of ripping things out and replacing them with new and > shiny just because no one understands how it all needs to be done in the New? mmdevapi was introduced with Vista a couple of years ago. Shiny? For an 1:1 (feature wise) backend replacement? A backend that has no UI component to it? Seriously? > first place. It would be much better to see the entire conversion to > mmdevapi in a separate tree stabilized before making it's way to main tree. Like in a development tree? The Wine 1.3.x tree is a development tree. Also how many people would you think would use that extra mmdevapi tree? One? Two? So you'll get the same untested code merged at a later time. > Wine is driven by wrong objectives. Most developers here don't give much > rip about regressions if the code they've put in is "right". This is Yes, sadly there are Wine developers that consider regressions they have introduced "just normal bugs" which they will fix eventually if they are interested. But there are a lot of other developers that fix the regressions they have introduced with a hight priority. I venture to say that most Wine developers prioritize regression bugs above normal bugs. And no, Andrew's code is *NOT* "right" by any means! If we preferred the "right" code we would have taken Maarten's code which nuked dsound out of orbit. Andrew's code is a minimally invasive (for a big amount of minimal) that replaces the winmm calls with mmdevapi. His patches didn't even conflict with my COM cleanup and the COM implementation in dsound is not only "wrong code" but also "bad and ugly". > exactly the wrong way to do it. Don't take it from me, take it from the > person who knows it much better and knows what he is talking about: > http://h30565.www3.hp.com/t5/Feature-Articles/Linus-Torvalds-s-Lessons-on-S… > > > > "[T]he thing that really matters is the users of the code. The code > itself is unimportant; the project is only as useful as people actually > find it." Exactly! And how did we care about our sound users all this years? By ignoring all their bugs? There are 67 open dsound bugs, the oldest is from 2004. Again this is an 1:1 backend replacement with feature parity on the "frontend". This is none of the stuff that Linus was targetting: "We replace a 90% working solution with a 60% working solution, but you don't need those 30% we took away", "We know it better", "You are not our targeted audience". The programs that use dsound need to work afterwards, period. Regressions are unavoidable but we do our very best to fix those. > You think that all people who'll find their program broken because of > this sound rewrite care about the code? Or that now dsound talks to > mmdevapi instead of winmm? Or remarks by Andrew that "he doesn't want to Those people will stay on the old Wine version that worked for them until we fix the regressions. On the other hand the people for which dsound never worked will most likely appreciate that we are finally working to fix their problem. > fix old code because it's going away anyway. And he'd rather fix new > code." a good excuse for their game to stop working on Wine? Uh? Logic error... if he doesn't fixes the old code he cannot break it. The new code on the other hand is new so he will fix the regressions in that. > I'd like you guys to think about what really is important users or code? Users. But you seem to prefer the old code... bye michael

Sept. 28, 2011

Re: [1/3] cmd: Avoid reading char by char from console (try 2)

by Alexandre Julliard

Frédéric Delanoy <frederic.delanoy(a)gmail.com> writes: > Yes, but it can fail for other reasons as well. > If it really is a console handle, but ReadConsoleW fails, isn't it > better to return failure early rather than trying to read from > (assumed) file handle h? It probably doesn't matter much, but you can always check last error or something like that. -- Alexandre Julliard julliard(a)winehq.org

Sept. 28, 2011

Re: [1/3] cmd: Avoid reading char by char from console (try 2)

by Frédéric Delanoy

2011/9/28 Alexandre Julliard <julliard(a)winehq.org>: > Frédéric Delanoy <frederic.delanoy(a)gmail.com> writes: > >> @@ -181,13 +181,21 @@ WCHAR *WCMD_parameter (WCHAR *s, int n, WCHAR **where, WCHAR **end) { >> * the LF (or CRLF) from the line. >> */ >> >> -WCHAR *WCMD_fgets (WCHAR *s, int noChars, HANDLE h) { >> - >> - DWORD bytes; >> - BOOL status; >> +WCHAR *WCMD_fgets (WCHAR *s, int noChars, HANDLE h) >> +{ >> + DWORD bytes, charsRead, dummy; >> + BOOL status, is_console_handle; >> WCHAR *p; >> >> p = s; >> + is_console_handle = !!GetConsoleMode(h, &dummy); >> + if (is_console_handle) { >> + status = ReadConsoleW(h, s, noChars, &charsRead, NULL); > > ReadConsole will fail if it's not a console, the check is redundant. Yes, but it can fail for other reasons as well. If it really is a console handle, but ReadConsoleW fails, isn't it better to return failure early rather than trying to read from (assumed) file handle h? Frédéric

Sept. 28, 2011

Re: winecfg: Remove driver selection from Audio tab.

by Alex Bradbury

On 28 September 2011 01:33, Vitaliy Margolen <wine-devel(a)kievinfo.com> wrote: > You think that all people who'll find their program broken because of this > sound rewrite care about the code? Or that now dsound talks to mmdevapi > instead of winmm? Or remarks by Andrew that "he doesn't want to fix old code > because it's going away anyway. And he'd rather fix new code." a good excuse > for their game to stop working on Wine? > > I'd like you guys to think about what really is important users or code? Do correct me if I'm wrong here, but users who don't want regressions in their favourite apps/games should be using the stable release. It doesn't seem fair to complain about regressions being ignored unless 1.4 releases with a significant number of them. Alex

Sept. 28, 2011

Re: [1/3] cmd: Avoid reading char by char from console (try 2)

by Alexandre Julliard

Frédéric Delanoy <frederic.delanoy(a)gmail.com> writes: > @@ -181,13 +181,21 @@ WCHAR *WCMD_parameter (WCHAR *s, int n, WCHAR **where, WCHAR **end) { > * the LF (or CRLF) from the line. > */ > > -WCHAR *WCMD_fgets (WCHAR *s, int noChars, HANDLE h) { > - > - DWORD bytes; > - BOOL status; > +WCHAR *WCMD_fgets (WCHAR *s, int noChars, HANDLE h) > +{ > + DWORD bytes, charsRead, dummy; > + BOOL status, is_console_handle; > WCHAR *p; > > p = s; > + is_console_handle = !!GetConsoleMode(h, &dummy); > + if (is_console_handle) { > + status = ReadConsoleW(h, s, noChars, &charsRead, NULL); ReadConsole will fail if it's not a console, the check is redundant. -- Alexandre Julliard julliard(a)winehq.org

Sept. 28, 2011

mmdevapi performance analysis

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

Hi, While the 3D guys work on performance improvements, we audio guys kill it. Luckily gfx data is shuffled around in MB/s, audio only in hundred kB/s, so hopefully it doesn't weight that much. Let's have a look at how data wanders around with dsound, winmm and mmdevapi. waveOutWrite with mmdevapi: copy to mmdevapi ALSA/OSS buffer copy some in case of mmdevapi buffer wrap around write buffer to ALSA/OSS waveOutWrite old driver model: write header to ALSA/OSS waveOutWrite involving MSACM with mmdevapi: convert PCM to MSACM, writing to new buffer copy to mmdevapi ALSA/OSS buffer copy some in case of buffer wrap around write to ALSA/OSS Here, if msacm/waveform.c had the courage to delay conversion until GetBuffer time, some copying could be avoided as msacm would write directly into mmdevapi's buffer. Being overly protective ("I should convert before GetBuffer because afterwards I might not have enough time") ends up wasting CPU cycles, hence paradoxically increasing the probability of being late thus causing underruns. The cause for the impedance mismatch between winmm and mmdevapi is what I call "owner inversion". The app supplies winmm with a pointer to its data buffers. As mmdevapi does the same, some transfer is needed between both. Arguably, mmdevapi is not the best target for winmm in Wine... DSOUND_WaveQueue newest "reimplement rendering on mmdevapi" in 1.3.30: write to dsound secondary buffer mix secondary to primary buffer copy to mmdevapi buffer copy some more in case of mmdevapi buffer wrap around write buffer to ALSA/OSS It looks like the new code misses the opportunity to equate device buffer with primary buffer. Instead we've got a cascade of buffers. Why is dsound's primary (mixing target) not mmdevapi's buffer? What's the goal? Ideally, 1. ALSA's mmdevdrv Get/ReleaseBuffer use snd_pcm_mmap_begin/commit, avoiding copying from mmdevapi to ALSA; 2. mmdevapi's buffer becomes DSound's primary, avoiding copying from dsound to mmdevapi. This applies even to CoreAudio where there's not really an mmdevapi buffer. The result: DSound's mixer talks straight with the HW. This should be no surprise. mmdevapi's shared mode mixer probably has roots in dsound's. BTW, what prevents equating dsound's secondary buffers with mmdevapi's shared mode streams and let the underlying device perform the mixing? Regards, Jörg Höhle

Sept. 28, 2011

Re: Update on testbot

by Hans Leidekker

On Sun, 2011-09-18 at 16:05 +0200, Maarten Lankhorst wrote: > I moved the windows 7 x64 vm with sp1 applied to the base pool now. This means > that all submitted patches will be run on that vm as well. The original win7 vm > is still available under its own name. Since all tests pass on that according > to winehq tests I'm keeping it in the base pool. I hope to see all tests on > the new vm fixed as well. They mostly appear to be related to the ie9 update, > but msctf:inputprocessor and msi:install fail as well. The msi:install test is fixed now as well as a failure in wininet:http on ie9. The msi:install failure was caused by pending renames on that machine, which is good for testing variety but you may want to check that out nonetheless.

Sept. 28, 2011

Re: mscoree: Add ICorDebug interface support (try 3)

by Vincent Povirk

It's OK to call CLRMetaHost methods directly with a NULL ICLRMetaHost instance. It's a singleton, and none of the methods use the interface. You might want to check whether you get a new ICorDebug interface every time you request one. I suspect you do. On Wed, Sep 28, 2011 at 1:21 AM, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > Hi, > Changed to use ICLRRuntimeInfo::GetInterface, suggest by Vincent. > Moved the #undef to the header. > > > Changelog: > mscoree: Add ICorDebug interface support > > > Best Regards > Alistair Leslie-Hughes > > > > > >

Sept. 28, 2011

Re: mscoree: Add ICorDebug interface support (try 3)

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 http://testbot.winehq.org/JobDetails.pl?Key=14599 Your paranoid android. === W2K3R2SESP2 (32 bit debugging) === debugging.c:82: Test failed: CreateDebuggingInterfaceFromVersion returned 80070057

Sept. 28, 2011

re: dsound: Place storage class before type qualifier

by Dan Kegel

Hi Andrew, doesn't seem to apply here: $ patch -p1 < 79377.patch patching file dlls/dsound/dsound_main.c Hunk #1 FAILED at 319. 1 out of 1 hunk FAILED -- saving rejects to file dlls/dsound/dsound_main.c.rej ---------- Forwarded message ---------- From: <buildbot(a)kegel.com> Date: Tue, Sep 27, 2011 at 2:50 PM Subject: Re: 79377: dsound: Place storage class before type qualifier To: dank(a)kegel.com, wine-tests-results(a)winehq.org This is an experimental automated build and test service. Please feel free to ignore this email while we work the kinks out. For more info about this message, see http://wiki.winehq.org/BuildBot The Buildbot has detected a failed build on builder runtests-default while building Wine. Full details are available at: http://buildbot.kegel.com/builders/runtests-default/builds/128 (though maybe not for long, as I'm still reinstalling the buildbot periodically while experimenting) BUILD FAILED: failed git Errors: error: patch failed: dlls/dsound/dsound_main.c:319 error: dlls/dsound/dsound_main.c: patch does not apply

Sept. 28, 2011