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

Re: RFC: Remove auto-scan of ALSA devices from winealsa.drv (and get repeatable behaviour)

by Alexandre Julliard

Andrew Eikum <aeikum(a)codeweavers.com> writes: > On Tue, Jan 22, 2013 at 11:15:59AM +0100, Joerg-Cyril.Hoehle(a)t-systems.com wrote: >> Andrew Eikum was in favour of this too and since implemented winmm >> device notification upon change. Remember the December thread: >> http://www.winehq.org/pipermail/wine-devel/2012-December/098114.html >> > > Yeah, I still think it's a good idea. Assuming we remove enumeration > and add a good UI to manually add devices, there were two unanswered > objections raised in the last thread. > > 1) We still want to do this, but want a better way to do this (i.e. > work with upstream to make a good enumeration API) > > This still requires us to gut our existing enumeration, so I guess you > can think of it in two pieces: remove the existing enumeration, and > add "good" enumeration should that ever exist. So let's do step one > now and maybe step two if it becomes possible. > > 2) Each new prefix will require manual configuration > > This is true now, but it would be a larger pain if the user has to add > a device in addition to selecting it. Is there some existing mechanism > for users to customize prefixes when they're created? Should there be? I still fail to see how this would be an improvement. If the enumeration doesn't give good results in some cases the heuristics could be improved, and/or fixed upstream, but getting rid of it is not a solution. At least on my setup, the current code is working just fine, and offering the devices I expect, without any manual configuration. That's how it should work for everybody. -- Alexandre Julliard julliard(a)winehq.org

Jan. 22, 2013

Re: RFC: Remove auto-scan of ALSA devices from winealsa.drv (and get repeatable behaviour)

by Andrew Eikum

On Tue, Jan 22, 2013 at 11:15:59AM +0100, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > Andrew Eikum was in favour of this too and since implemented winmm > device notification upon change. Remember the December thread: > http://www.winehq.org/pipermail/wine-devel/2012-December/098114.html > Yeah, I still think it's a good idea. Assuming we remove enumeration and add a good UI to manually add devices, there were two unanswered objections raised in the last thread. 1) We still want to do this, but want a better way to do this (i.e. work with upstream to make a good enumeration API) This still requires us to gut our existing enumeration, so I guess you can think of it in two pieces: remove the existing enumeration, and add "good" enumeration should that ever exist. So let's do step one now and maybe step two if it becomes possible. 2) Each new prefix will require manual configuration This is true now, but it would be a larger pain if the user has to add a device in addition to selecting it. Is there some existing mechanism for users to customize prefixes when they're created? Should there be? Anyway, I'll put this on my TODO list and let you know when I've got something working. Andrew

Jan. 22, 2013

Re: macdrv: implement getting and setting the screen saver state. if(Style)

by Christian Costa

Well, 'grep "if(" * -r | wc -l' and 'grep "if (" * -r | wc -l'. :) 2013/1/22 Bruno Jesus <00cpxxx(a)gmail.com>: > Out of curiosity can you explain the shell wizardry you used to get the > stats? > > Thanks, > Bruno > > > On Tue, Jan 22, 2013 at 10:52 AM, Christian Costa <titan.costa(a)gmail.com> > wrote: >> >> 2013/1/22 <Joerg-Cyril.Hoehle(a)t-systems.com>: >> > Hi, >> > >> > Ken Thomases wrote: >> >>> + if(success != kIOReturnSuccess) >> >>Another style nitpick: please put a space between "if" and the >> >> condition. >> >> That applies to the "if(count2)" above, too. >> > >> > I find this over the top. Where is the rule? >> > Andrew Eikum's mmdevapi code -- which I don't consider very old in Wine >> > -- >> > uses exactly 0 space between if and (). >> > >> > So what is it that the Wine "project" can tell new contributors? >> > >> > The only rule I see is: "Look at the code surrounding the patch" and >> > its corollary: "if you create a new module, you're the first and define >> > the style others will have to follow." >> > >> >> Hi, >> >> Just some stats regard "if" before Dan beat me at it ;) : >> - without space => 39979 >> - with space => 144133 >> >> The space is better to make the distinction between C keyword and >> function name. Off course it's a personal taste. :) >> >> Bye, >> Christian >> >> > > > > -- > universe* god::bigbang (void); //and then it all began...

Jan. 22, 2013

Re: macdrv: implement getting and setting the screen saver state. if(Style)

by Christian Costa

2013/1/22 <Joerg-Cyril.Hoehle(a)t-systems.com>: > Hi, > > Ken Thomases wrote: >>> + if(success != kIOReturnSuccess) >>Another style nitpick: please put a space between "if" and the condition. >> That applies to the "if(count2)" above, too. > > I find this over the top. Where is the rule? > Andrew Eikum's mmdevapi code -- which I don't consider very old in Wine -- > uses exactly 0 space between if and (). > > So what is it that the Wine "project" can tell new contributors? > > The only rule I see is: "Look at the code surrounding the patch" and > its corollary: "if you create a new module, you're the first and define > the style others will have to follow." > Hi, Just some stats regard "if" before Dan beat me at it ;) : - without space => 39979 - with space => 144133 The space is better to make the distinction between C keyword and function name. Off course it's a personal taste. :) Bye, Christian

Jan. 22, 2013

macdrv: implement getting and setting the screen saver state. if(Style)

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

Hi, Ken Thomases wrote: >> + if(success != kIOReturnSuccess) >Another style nitpick: please put a space between "if" and the condition. > That applies to the "if(count2)" above, too. I find this over the top. Where is the rule? Andrew Eikum's mmdevapi code -- which I don't consider very old in Wine -- uses exactly 0 space between if and (). So what is it that the Wine "project" can tell new contributors? The only rule I see is: "Look at the code surrounding the patch" and its corollary: "if you create a new module, you're the first and define the style others will have to follow." Regards, Jörg Höhle

Jan. 22, 2013

RFC: Remove auto-scan of ALSA devices from winealsa.drv (and get repeatable behaviour)

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

Hi, please view the winmm:wave test results from Francois Gouget's machines fg-acer64-t32 fg-acer64-wow32 fg-acer64-t64 fg-acer64-wow64 http://test.winehq.org/data/dabde6a04f6d02233bc5074a8eba613b2c4adc68/index_… Is their configuration the same except for 32/64bit? We see 3 types of results 2x: 2 devices, no ALSA "default" device(!), one HDA-An, one HDA-HDMI 1x: 3 devices incl. ALSA "default", all tests successful 1x: 3 devices incl. ALSA "default", dev. 1 HDA-An yields MMSYSERR_ALLOCATED Is their configuration truly the same? Is that yet another aretefact of ALSA device enumeration? It could well be. Again, I argue in favor of removing the enumeration code. I want users to get repeatable results. I don't want Wine's winmm to map device 0 as ALSA "default" on one run and "HDMI-An" on another run. Andrew Eikum was in favour of this too and since implemented winmm device notification upon change. Remember the December thread: http://www.winehq.org/pipermail/wine-devel/2012-December/098114.html Of course, the winecfg/GUI issue need be addressed. Regards, Jörg Höhle

Jan. 22, 2013

winmm/tests: Allow more margin in the test_asyncWAVE() playback check.

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

Francois, thank you for helping remove timing-caused flakiness from the tests. I believe you forgot to test your patch in Wine(!). The test + ok(400 <= p2 && p2 <= 600, "%ums is not in the expected 400-600ms range\n", p2); should fail in every version of Wine. Ironically, the reason is mentioned in your patch: /* Give Wine's asynchronous thread time to start up. Furthermore, * it uses 3 buffers per second, so that the positions reported * will be 333ms, 667ms etc. at best. */ IIRC, the first non-null position reported by Wine is 667ms, because it successfully queues two 333ms buffers to waveOut. I suggest adding a todo_wine plus an additional ok(0<p2 && strcmp(buf,"2000")) that Wine won't fail. And I'd decrease the sleep to e.g. 200 and 100 <= p2 && p2 <= 300 All those times sum up, the winmm test run long enough already. MCIWave should use winmm:waveOutGetPosition while playing, however the MCI devices hardly have any decent thread-safety and the construct if(status == MODE_PLAYING) waveOutGetPosition(hWave, &pos) else pos = stopped_pos; is unsafe, as the player may have exited meanwhile and closed hWave. It's a compatibility bug since day one of mciwave in git history that the position increments 3 times per second only. Yet so far, no app seems to have been bothered. Regards, Jörg Höhle

Jan. 22, 2013

Re: user32/tests: Test standard scrollbar's initialization (try 2).

by Dmitry Timoshkov

Sergey Guralnik <serhio(a)etersoft.ru> wrote: > Subject: user32/tests: Test standard scrollbar's initialization (try 2). If it matters the test looks good to me. -- Dmitry.

Jan. 22, 2013

Re: [PATCH] windowscodecs: Add support for DIB format.

by Dmitry Timoshkov

Vincent Povirk <madewokherd(a)gmail.com> wrote: > Yes, it is possible that I wrote more code than was necessary to > support TGA because I knew WIC better and because I thought it'd be > cool (in theory the decoder can now be used outside the context of > d3dx9, for instance on windows with the builtin image viewer - not all > that useful but still pretty cool imo). Adding Wine specific extensions should be slightly more justified IMHO. > For DIB it is less code because all of the logic involved is already > used to support BMP or ICO, and all of the real work for it was done > at the same time as adding ICO support. I'd argue that it's even less code to load a DIB since gdi32 has full built-in support for it. Taking into account all the overhead that COM adds (although in d3dx9 case it's already there) writing a new WIC codec is not always justified. -- Dmitry.

Jan. 22, 2013

Re: macdrv: implement getting and setting the screen saver state, version 4

by Ken Thomases

Hi, On Jan 21, 2013, at 6:21 PM, C.W. Betts wrote: > This version falls through in SPI_SETSCREENSAVEACTIVE. Sadly, you used C++ style for the fall-through comment. :-/ Also, you have some trailing whitespace and the patch no longer applies cleanly after today's commits of mine. It should be easy to fix that though. There aren't any serious conflicts. -Ken

Jan. 22, 2013