February 2011 - Wine-Devel - list.winehq.org

Re: iphlpapi: Hint what to do when icmp does not work

by Charles Davis

On 2/11/11 11:12 AM, André Hentschel wrote: > Otherwise the user might try running wine as root or doesn't know that this error can be easily fixed when needed. > --- > dlls/iphlpapi/icmp.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/dlls/iphlpapi/icmp.c b/dlls/iphlpapi/icmp.c > index d8ee101..41d1940 100644 > --- a/dlls/iphlpapi/icmp.c > +++ b/dlls/iphlpapi/icmp.c > @@ -155,6 +155,7 @@ HANDLE WINAPI IcmpCreateFile(VOID) > int sid=socket(AF_INET,SOCK_RAW,IPPROTO_ICMP); > if (sid < 0) { > ERR_(winediag)("Failed to use ICMP (network ping), this requires special permissions.\n"); > + ERR_(winediag)("Try running \"sudo setcap cap_net_raw+epi /usr/bin/wine-preloader\".\n"); That only works on systems that support POSIX capabilities (Linux, and maybe OpenBSD). Chip

Feb. 11, 2011

Re: Resend: Who does c2man?

by Juan Lang

> It looks like Jon Griffiths is the one I need to contact. Is he still > around? He's often disconnected, but he does respond to email from time to time. --Juan

Feb. 11, 2011

Re: Resend: Who does c2man?

by Max TenEyck Woodbury

On 02/11/2011 12:20 PM, GOUJON Alexandre wrote: > On 02/11/2011 05:19 PM, Max TenEyck Woodbury wrote: >> I have just been going over tools/c2man and have a fair number of >> questions about it. > > You can ask them here. This is why wine-devel exists. > There is also IRC (http://wiki.winehq.org/IRC) > > But keep in mind that everyone is very busy. So ask something useful. > If you are too lazy to read the source code, then don't be surprised if > you don't get any answer. > Umm, I HAVE been reading the source code. There is quite a lot of it and there are not a lot of descriptive comments. There are a few things I think need changing but some people might consider those changes controversial. Such things as links that go nowhere and mangled names. Also places where the parsing of the input does one thing while the comments say it does something slightly different (and the comments are what should be done). >> Please send me >> email with contact information, or indicate that you want this done on >> some forum. > > wine/tools$ git blame c2man.pl > Thanks, that was useful. Summary: Mike McCormack who was the original author around Aug 2000. Jon Griffiths who rewrote most of it from Mar 2003 thru Dec 2004. Francois Gouget who made some changes in Oct 2004. Stefan Stranz who added XML stuff in Jun 2009. A few others with small changes some time ago. It looks like Jon Griffiths is the one I need to contact. Is he still around? Max

Feb. 11, 2011

Re: [PATCH 01/13] dsound: New sample rate converter core functions.

by Krzysztof Nikiel

2011/2/11 <Joerg-Cyril.Hoehle(a)t-systems.com>: > Hi, > > The question how to split is interesting. I haven't found the answer > about how to split my future MIDI player rewrite either. Like yours, > it will replace old functions with something new. It looks like introducing bigger patches is a kind of mission impossible. > > Some guidelines are: > - Each patch should be self contained and a logical unit, e.g. > + a bug fix, possibly with test > e.g. ZeroMemory => FillMemory wBitsPerSample == 8 ? 128 : 0 > + a change in configuration (e.g. sampling rate 48000) > => split change of default & introduction new of registry key > + some refactoring in preparation for future work > + changes to logic & functionality > e.g. DSDDESC_DONTNEEDWRITELEAD, leadin > - It must compile. > - It should pass tests (a rule, not a dogma). > > In this light, [PATCH 08/13] (remove 2x HeapFree) looks wrong, unless > it fixed a bug in the old code. It should be part of something else. But the question is: remove everything unused in one patch or split the removing. > > Try and explain design changes. Your code is not only a resampler, it > also changes the way DSound performs mixing, such that > DSOUND_MixToTemporary becomes superfluous. > > > What's not so nice -- yet that has happened several times already -- > is to have a patch like "Now use the new functions added in the last 12 > patches instead of the old ones", because regression testing will > irremediably identify that patch as culprit and not help identify > which of the 12 introducing patches caused the "actual" regression via > a subtle change in behaviour or side effects. I would reduce it to two options: 1. Add first, remove last; 2. Remove/stubify first, add last. First option looks more reasonable to me. > > Conversely, if you think about how regression testing can work well, > you'll find recommendations for patches yourself. Do you mean bisecting it? I would just wish a good luck to anyone bisecting such patch. In this case, I would rather fix bugs more traditional way. > > About ordering: Move the patch introducing the new registry key > towards the end, and work with a constant prior to that. Doing so, > this patch becomes working code instead of being ground work not > testable until the resampler is in place. > > I suggest isolating > - the FIR generator + Makefile change. > - keeping patch 11 -- remove unused files & code last -- so as to > minimize size of patches that introduce code. > > As a side note, I'm wondering whether I'd insert resample.c into mixer.c. No way, at least I'm not going to realize such a crazy idea. You're welcome to try. > > + TRACE("resample quality: %d\n", dsb->quality); > + TRACE("resample firstep %d\n", dsb->firstep); > Logging is expensive (when on) and known to cause race > conditions. Hence I'd combine all these into one line. Those were used for debugging but now are no longer useful, can be removed. Thanks.

Feb. 11, 2011

Re: CD handling on MacOS - the big picture?

by Ken Thomases

On Feb 11, 2011, at 10:23 AM, <Joerg-Cyril.Hoehle(a)t-systems.com> <Joerg-Cyril.Hoehle(a)t-systems.com> wrote: > Mixed CDs occur in some games, but some audio CDs > contain "bonus content" as well. > > On MacOS, mixed CDs are mounted as two volumes > - the audio part, > - the data/binary section > not unlike Gnome which also displays 2 disc icons on the desktop. > > On Linux, Wine manages to map both to a single CD-ROM > drive letter like D:, such that > - D:\data\foo.exe works as well as > - MCICDA: open D: type cdaudio alias D; play D work (but see bug 20555), > which mimics native's behaviour. > > On MacOS, we're not there yet. I've no idea how that can be made to work. > How would Wine on MacOS join the pieces? I guess I'd say that Wine should create a drive letter only for the data/binary section. I suspect that that's already the case due to the logic in mountmgr.sys/diskarb.c. Then, if some code needs to find the whole-media device that contains that section, it could use something like the get_parent_device() function in dlls/ntdll/cdrom.c. From there, it might have to enumerate the child entries to find the audio part. I don't know how one checks for that. Playing with such a mixed CD and /Developer/Applications/Utilities/IORegistryExplorer.app might be helpful in figuring that out. Cheers, Ken

Feb. 11, 2011

Re: Resend: Who does c2man?

by GOUJON Alexandre

On 02/11/2011 05:19 PM, Max TenEyck Woodbury wrote: > I have just been going over tools/c2man and have a fair number of > questions about it. You can ask them here. This is why wine-devel exists. There is also IRC (http://wiki.winehq.org/IRC) But keep in mind that everyone is very busy. So ask something useful. If you are too lazy to read the source code, then don't be surprised if you don't get any answer. > Please send me > email with contact information, or indicate that you want this done on > some forum. wine/tools$ git blame c2man.pl

Feb. 11, 2011

[PATCH 01/13] dsound: New sample rate converter core functions.

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

Hi, The question how to split is interesting. I haven't found the answer about how to split my future MIDI player rewrite either. Like yours, it will replace old functions with something new. Some guidelines are: - Each patch should be self contained and a logical unit, e.g. + a bug fix, possibly with test e.g. ZeroMemory => FillMemory wBitsPerSample == 8 ? 128 : 0 + a change in configuration (e.g. sampling rate 48000) => split change of default & introduction new of registry key + some refactoring in preparation for future work + changes to logic & functionality e.g. DSDDESC_DONTNEEDWRITELEAD, leadin - It must compile. - It should pass tests (a rule, not a dogma). In this light, [PATCH 08/13] (remove 2x HeapFree) looks wrong, unless it fixed a bug in the old code. It should be part of something else. Try and explain design changes. Your code is not only a resampler, it also changes the way DSound performs mixing, such that DSOUND_MixToTemporary becomes superfluous. What's not so nice -- yet that has happened several times already -- is to have a patch like "Now use the new functions added in the last 12 patches instead of the old ones", because regression testing will irremediably identify that patch as culprit and not help identify which of the 12 introducing patches caused the "actual" regression via a subtle change in behaviour or side effects. Conversely, if you think about how regression testing can work well, you'll find recommendations for patches yourself. About ordering: Move the patch introducing the new registry key towards the end, and work with a constant prior to that. Doing so, this patch becomes working code instead of being ground work not testable until the resampler is in place. I suggest isolating - the FIR generator + Makefile change. - keeping patch 11 -- remove unused files & code last -- so as to minimize size of patches that introduce code. As a side note, I'm wondering whether I'd insert resample.c into mixer.c. + TRACE("resample quality: %d\n", dsb->quality); + TRACE("resample firstep %d\n", dsb->firstep); Logging is expensive (when on) and known to cause race conditions. Hence I'd combine all these into one line. Also keep in mind that your reactions here will influence acceptance of your code -- "Does this person sound like s/he's going to support and fix regressions in her/his code or is it 'take or leave, bye'?" Regards, Jörg Höhle

Feb. 11, 2011

Re: [PATCH 01/13] dsound: New sample rate converter core functions.

by Krzysztof Nikiel

2011/2/11 Maarten Lankhorst <m.b.lankhorst(a)gmail.com>: >> Well, previous version of this patch was rejected as "needs >> splitting", it's basically too big to be send as a single patch. It >> can be applied as several smaller chunks or rejected as a whole. I >> don't think there is any other option. > > Thanks for looking at the format conversion functions. The splitting can > probably be done in 4 parts: > 1. If you have any generic bugfixes, put them in the first few patches. Once > those are in you can look at the other parts. Each bugfix should probably be > its own separate patch. > 2. Single patch to remove the old behavior. At this point you probably only > want the mixing to work without requiring reconversion, and have a FIXME or > something to say rate conversion is unavailable. > 3. A bunch of patches implementing new rate conversion, one logical step at > a time. The only requirement is that you want to be able to build wine > without it failing, and if possible even running directsound, even if the > result is insane.. Thanks, now I have at least two reasonable advices how to proceed with the split. It looks like the actual split may be more difficult to make than the patch itself :P > > Having mkfir in the way you put it there will probably not work, since some > people will build wine out of tree. My guess is that you should put that in > tools. Since the contents of the file won't change often it may just be > enough to have the .c file there and have a note in the mkfir.h header that > it was generated by tools/mkfir.c, but I'm not AJ so he would have to > decide. Yes, 'tools' seems a good place to put it. I think if someone is really inclined to know where 'firtab.h' comes from it won't be a real problem to find it. I'm not sure what "out of tree" means but I think it does work out of tree. Anyway, if you like the unwrapped version, just visit the bugzilla and download the .zip pack. http://bugs.winehq.org/show_bug.cgi?id=14717 > > The reason for this kind of splitting is that even if the code is broken > between the removal of the old code and the last patch, it's a lot easier to > see bugs. Usually in a patch like this you will spot 2 or 3 bugs you > wouldn't have otherwise spotted because you look at the code in logical > parts. This also makes bisecting a lot easier. Otherwise you know what > commit caused the regression, but you have no idea what part of the code is. I expected it will be a problematic patch. It's quite big and the original code is rather messy, especially mixer.c. I really doubt the patch will be easy to read even when split as you say. > > Cheers, > Maarten >

Feb. 11, 2011

CD handling on MacOS - the big picture?

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

Hi, the last years have seen individual patches by K. Thomases and C. Davis to Mac OSX' CD handling, but I don't see the big picture. Mixed CDs occur in some games, but some audio CDs contain "bonus content" as well. On MacOS, mixed CDs are mounted as two volumes - the audio part, - the data/binary section not unlike Gnome which also displays 2 disc icons on the desktop. On Linux, Wine manages to map both to a single CD-ROM drive letter like D:, such that - D:\data\foo.exe works as well as - MCICDA: open D: type cdaudio alias D; play D work (but see bug 20555), which mimics native's behaviour. On MacOS, we're not there yet. I've no idea how that can be made to work. How would Wine on MacOS join the pieces? Regards, Jörg Höhle

Feb. 11, 2011

Resend: Who does c2man?

by Max TenEyck Woodbury

I have just been going over tools/c2man and have a fair number of questions about it. I can probably dig some of the answers out myself, but it would probably go faster and raise less fuss if I can talk to the person (people) who have worked on it previously. Please send me email with contact information, or indicate that you want this done on some forum. Max

Feb. 11, 2011

Re: [PATCH 01/13] dsound: New sample rate converter core functions.

by Maarten Lankhorst

Hi Krzysztof, Op 11-02-11 12:16, Krzysztof Nikiel schreef: > 2011/2/11 Dmitry Timoshkov<dmitry(a)codeweavers.com>: >> Krzysztof Nikiel<knik00(a)gmail.com> wrote: >> >>>> You can't send Makefile changes separately from added/removed >>>> files, a patch should not add dead code. >>> Could you explain "dead code", all 13 parts need to be applied, >>> otherwise the code will be broken. >> Wine should compile and be able to pass 'make test' after each separate patch. >> You can't add for instance dlls/dsound/resample.c in one patch, and add it to >> Makefile or use interfaces provided by it in some later patches. Every patch >> should be finished and self-containing. > Well, previous version of this patch was rejected as "needs > splitting", it's basically too big to be send as a single patch. It > can be applied as several smaller chunks or rejected as a whole. I > don't think there is any other option. Thanks for looking at the format conversion functions. The splitting can probably be done in 4 parts: 1. If you have any generic bugfixes, put them in the first few patches. Once those are in you can look at the other parts. Each bugfix should probably be its own separate patch. 2. Single patch to remove the old behavior. At this point you probably only want the mixing to work without requiring reconversion, and have a FIXME or something to say rate conversion is unavailable. 3. A bunch of patches implementing new rate conversion, one logical step at a time. The only requirement is that you want to be able to build wine without it failing, and if possible even running directsound, even if the result is insane.. Having mkfir in the way you put it there will probably not work, since some people will build wine out of tree. My guess is that you should put that in tools. Since the contents of the file won't change often it may just be enough to have the .c file there and have a note in the mkfir.h header that it was generated by tools/mkfir.c, but I'm not AJ so he would have to decide. The reason for this kind of splitting is that even if the code is broken between the removal of the old code and the last patch, it's a lot easier to see bugs. Usually in a patch like this you will spot 2 or 3 bugs you wouldn't have otherwise spotted because you look at the code in logical parts. This also makes bisecting a lot easier. Otherwise you know what commit caused the regression, but you have no idea what part of the code is. Cheers, Maarten

Feb. 11, 2011

Re: [PATCH 01/13] dsound: New sample rate converter core functions. (resend)

by Krzysztof Nikiel

2011/2/11 Vitaliy Margolen <wine-devel(a)kievinfo.com>: > On 02/11/2011 04:34 AM, Krzysztof Nikiel wrote: >> >> +typedef double sample_t; >> >> +static inline sample_t getsample(LPBYTE buf, INT bps) >> +{ >> + tmp = ((sample_t) (*((BYTE *) buf)) - 128.0); > > No need to type cast. "buf" already a pointer to the BYTE. You can use it > directly as *buf or buf[0], which is even more readable. Indeed, but I think it would be even better to change 'buf' type to void* as the actual size/type is defined by 'bps'. > >> +static inline void putsample(LPBYTE buf, INT bps, sample_t smp) >> + CLIPSAMPLE(smp, (double) 0x7fff); > > Typecasts are bad, especially that the type of "smp" is not "double". Please > use "32767.0" instead. So maybe something like (sample_t)0x7fff would be better or better yet move the typecast to the macro definition. > >> + DOUBLE amp[3]; > > Mixing types again. If you defined your own type then please use it. Or make > everything the same type whichever it is. I'm not quite sure what type it should be, 'sample_t' doesn't look quite good to me but maybe you're right. > > >> [PATCH 02/13] dsound: Resample filter table generator. (resend) > > Don't think compiler helpers should be going into dll directories. The more > correct place is tools. But it's up to AJ. Ah, I see. > > > And as others noted, your patch series is not split correctly. Each patch > will break Wine tree until the very last one. This is against the patch > requirements that code tree should compile after every commit. > > You can put your compile helper with it's make file first. Then add your > resampling code with a make file changes to compile it. But not use it yet > (it does break another rule for adding dead code, but that's the only way > you can split the entire series). After that make use of your new resampler, > removing all conflicting code, so Wine will work. Last commit can be a > cleanup for all remaining pieces that are no longer used. Sounds reasonable, but not quite easy to do. It looks like wine will use old code for some time until I try to do the correct split. Thanks. > > Vitaliy. >

Feb. 11, 2011

Testmail

by Detlef Riekenberg

Hi Just a test, as my patches do not show up in the mail archive. Detlef ___________________________________________________________ WEB.DE DSL Doppel-Flat ab 19,99 €/mtl.! Jetzt mit gratis Handy-Flat! http://produkte.web.de/go/DSL_Doppel_Flatrate/2

Feb. 11, 2011

Re: [PATCH] dsound: More accurate volume calculation.

by Krzysztof Nikiel

Sure, I'm working on it. 2011/2/11 Vitaliy Margolen <wine-devel(a)kievinfo.com>: > On 02/11/2011 02:38 AM, Krzysztof Nikiel wrote: >> >> I have changed this when creating the new resampler patch but it's not >> really related to the main patchset. > > The patch is wrapped. Please send it as attachment. Also please no HTML when > sending patches. > > Vitaliy >

Feb. 11, 2011

Re: [PATCH] dsound: More accurate volume calculation.

by Vitaliy Margolen

On 02/11/2011 02:38 AM, Krzysztof Nikiel wrote: > I have changed this when creating the new resampler patch but it's not > really related to the main patchset. The patch is wrapped. Please send it as attachment. Also please no HTML when sending patches. Vitaliy

Feb. 11, 2011

Re: [PATCH 01/13] dsound: New sample rate converter core functions. (resend)

by Vitaliy Margolen

Nice job! If it will make Wine's sound better - it would be awesome. Haven't read the entire patch series. But few nit picks so far. On 02/11/2011 04:34 AM, Krzysztof Nikiel wrote: > +typedef double sample_t; > > +static inline sample_t getsample(LPBYTE buf, INT bps) > +{ > + tmp = ((sample_t) (*((BYTE *) buf)) - 128.0); No need to type cast. "buf" already a pointer to the BYTE. You can use it directly as *buf or buf[0], which is even more readable. > +static inline void putsample(LPBYTE buf, INT bps, sample_t smp) > + CLIPSAMPLE(smp, (double) 0x7fff); Typecasts are bad, especially that the type of "smp" is not "double". Please use "32767.0" instead. > + DOUBLE amp[3]; Mixing types again. If you defined your own type then please use it. Or make everything the same type whichever it is. > [PATCH 02/13] dsound: Resample filter table generator. (resend) Don't think compiler helpers should be going into dll directories. The more correct place is tools. But it's up to AJ. And as others noted, your patch series is not split correctly. Each patch will break Wine tree until the very last one. This is against the patch requirements that code tree should compile after every commit. You can put your compile helper with it's make file first. Then add your resampling code with a make file changes to compile it. But not use it yet (it does break another rule for adding dead code, but that's the only way you can split the entire series). After that make use of your new resampler, removing all conflicting code, so Wine will work. Last commit can be a cleanup for all remaining pieces that are no longer used. Vitaliy.

Feb. 11, 2011

Re: msxml3: Ignore IActiveScript interface for IXMLHTTPRequest

by Nikolay Sivov

On 2/11/2011 15:56, Jacek Caban wrote: > On 2/11/11 12:02 PM, Nikolay Sivov wrote: >> On 2/11/2011 13:30, Alistair Leslie-Hughes wrote: >>> Hi, >>> >>> -------------------------------------------------- >>> From: "Nikolay Sivov" <bunglehead(a)gmail.com> >>> Sent: Friday, February 11, 2011 9:27 PM >>> To: "Alistair Leslie-Hughes" <leslie_alistair(a)hotmail.com> >>> Cc: <wine-devel(a)winehq.org> >>> Subject: Re: msxml3: Ignore IActiveScript interface for IXMLHTTPRequest >>> >>>> On 2/11/2011 12:25, Alistair Leslie-Hughes wrote: >>>>> Hi, >>>>> >>>>> On 11/02/2011 8:12 PM, Nikolay Sivov wrote: >>>>>> On 2/11/2011 10:51, Alistair Leslie-Hughes wrote: >>>>>>> Hi, >>>>>>> >>>>>>> >>>>>>> Changelog: >>>>>>> msxml3: Ignore IActiveScript interface for IXMLHTTPRequest >>>>>> What's a purpose of this? >>>>> To remove the FIXME from appearing in the logs. >>>> What application is that? Does it appear running in browser >>>> scripting by any chance? >>> Yes, it was a website that uses XMLHTTPRequest to load files off the >>> local >>> filesystem. >> My point is that browser scripting engine shouldn't query for that >> interface in a first place probably, so it's interesting to figure >> out why it's queried instead of silence it. Is it possible to >> reproduce that with wine-gecko or I need IE? >> >> P.S. cc-ing Jacek, as he knows probably why such dead queries happen. > > It's queried by mshtml security manager and the query is right. It's > just the way to check if created object (usually via ActiveXObject > call in jscript) is a script engine. Once script engine owns an > objects, it queries for a few more ifaces for which returning > E_NOINTERFACE is perfectly valid and expected behavior. I think the > right fix in this case is silencing the FIXME (downgrading to WARN or > TRACE). FIXME in QueryInterface implementation is a good thing for > objects that implement a lot different ifaces and it's likely that > lack of some will cause app failure or objects that are unlikely to be > queried for unimplemented ifaces. I think it's not the case here. Ok, thanks for explanation. So it's fine to silence it in this case or course. > > Cheers, > Jacek >

Feb. 11, 2011

Re: msxml3: Ignore IActiveScript interface for IXMLHTTPRequest

by Jacek Caban

On 2/11/11 12:02 PM, Nikolay Sivov wrote: > On 2/11/2011 13:30, Alistair Leslie-Hughes wrote: >> Hi, >> >> -------------------------------------------------- >> From: "Nikolay Sivov" <bunglehead(a)gmail.com> >> Sent: Friday, February 11, 2011 9:27 PM >> To: "Alistair Leslie-Hughes" <leslie_alistair(a)hotmail.com> >> Cc: <wine-devel(a)winehq.org> >> Subject: Re: msxml3: Ignore IActiveScript interface for IXMLHTTPRequest >> >>> On 2/11/2011 12:25, Alistair Leslie-Hughes wrote: >>>> Hi, >>>> >>>> On 11/02/2011 8:12 PM, Nikolay Sivov wrote: >>>>> On 2/11/2011 10:51, Alistair Leslie-Hughes wrote: >>>>>> Hi, >>>>>> >>>>>> >>>>>> Changelog: >>>>>> msxml3: Ignore IActiveScript interface for IXMLHTTPRequest >>>>> What's a purpose of this? >>>> To remove the FIXME from appearing in the logs. >>> What application is that? Does it appear running in browser >>> scripting by any chance? >> Yes, it was a website that uses XMLHTTPRequest to load files off the >> local >> filesystem. > My point is that browser scripting engine shouldn't query for that > interface in a first place probably, so it's interesting to figure out > why it's queried instead of silence it. Is it possible to reproduce > that with wine-gecko or I need IE? > > P.S. cc-ing Jacek, as he knows probably why such dead queries happen. It's queried by mshtml security manager and the query is right. It's just the way to check if created object (usually via ActiveXObject call in jscript) is a script engine. Once script engine owns an objects, it queries for a few more ifaces for which returning E_NOINTERFACE is perfectly valid and expected behavior. I think the right fix in this case is silencing the FIXME (downgrading to WARN or TRACE). FIXME in QueryInterface implementation is a good thing for objects that implement a lot different ifaces and it's likely that lack of some will cause app failure or objects that are unlikely to be queried for unimplemented ifaces. I think it's not the case here. Cheers, Jacek

Feb. 11, 2011

Re: [PATCH 01/13] dsound: New sample rate converter core functions. (resend)

by Owen Rudge

> It looks like it may be too big to be accepted. Anyway, I will try to > resend it with 4 space indent for new files. > Hopefully these patches won't get wrapped this time. Your patches still appear to be being wrapped by your mail client. Try sending them as an attachment instead of inline (i.e., use the --attach parameter to git format-patch). -- Owen Rudge http://www.owenrudge.net/

Feb. 11, 2011

Re: [PATCH 01/13] dsound: New sample rate converter core functions.

by Krzysztof Nikiel

2011/2/11 Ricardo Filipe <ricardojdfilipe(a)gmail.com>: > > > 2011/2/11 Krzysztof Nikiel <knik00(a)gmail.com> >> >> 2011/2/11 Dmitry Timoshkov <dmitry(a)codeweavers.com>: >> > Krzysztof Nikiel <knik00(a)gmail.com> wrote: >> > >> >> > You can't send Makefile changes separately from added/removed >> >> > files, a patch should not add dead code. >> >> >> >> Could you explain "dead code", all 13 parts need to be applied, >> >> otherwise the code will be broken. >> > >> > Wine should compile and be able to pass 'make test' after each separate >> > patch. >> > You can't add for instance dlls/dsound/resample.c in one patch, and add >> > it to >> > Makefile or use interfaces provided by it in some later patches. Every >> > patch >> > should be finished and self-containing. >> >> Well, previous version of this patch was rejected as "needs >> splitting", it's basically too big to be send as a single patch. It >> can be applied as several smaller chunks or rejected as a whole. I >> don't think there is any other option. >> >> > > still, what dmitry told you still holds. wine needs to be able to compile > and pass the tests after each patch is applied. at least the makefile > changes should be merged with the previous patches that needs them (1 and > 2). the rest seems like fine splitting, although i don't know the code. > what i'm not sure is, if resampler is still not called after the first > patches, if it is fine to send those first patches... i think you also need > to call resampler for the patch to be accepted, even if it is just as a > stub, incomplete version. It's not that easy, I don't think it would be just incomplete, it would be broken. The original code is rather messy, especially mixer.c, no wonder no one wanted to touch it to fix the really poor sample rate conversion.

Feb. 11, 2011

Re: [PATCH 01/13] dsound: New sample rate converter core functions.

by Ricardo Filipe

2011/2/11 Krzysztof Nikiel <knik00(a)gmail.com> > 2011/2/11 Dmitry Timoshkov <dmitry(a)codeweavers.com>: > > Krzysztof Nikiel <knik00(a)gmail.com> wrote: > > > >> > You can't send Makefile changes separately from added/removed > >> > files, a patch should not add dead code. > >> > >> Could you explain "dead code", all 13 parts need to be applied, > >> otherwise the code will be broken. > > > > Wine should compile and be able to pass 'make test' after each separate > patch. > > You can't add for instance dlls/dsound/resample.c in one patch, and add > it to > > Makefile or use interfaces provided by it in some later patches. Every > patch > > should be finished and self-containing. > > Well, previous version of this patch was rejected as "needs > splitting", it's basically too big to be send as a single patch. It > can be applied as several smaller chunks or rejected as a whole. I > don't think there is any other option. > > > still, what dmitry told you still holds. wine needs to be able to compile and pass the tests after each patch is applied. at least the makefile changes should be merged with the previous patches that needs them (1 and 2). the rest seems like fine splitting, although i don't know the code. what i'm not sure is, if resampler is still not called after the first patches, if it is fine to send those first patches... i think you also need to call resampler for the patch to be accepted, even if it is just as a stub, incomplete version.

Feb. 11, 2011

Re: [PATCH 01/13] dsound: New sample rate converter core functions.

by Krzysztof Nikiel

2011/2/11 Dmitry Timoshkov <dmitry(a)codeweavers.com>: > Krzysztof Nikiel <knik00(a)gmail.com> wrote: > >> > You can't send Makefile changes separately from added/removed >> > files, a patch should not add dead code. >> >> Could you explain "dead code", all 13 parts need to be applied, >> otherwise the code will be broken. > > Wine should compile and be able to pass 'make test' after each separate patch. > You can't add for instance dlls/dsound/resample.c in one patch, and add it to > Makefile or use interfaces provided by it in some later patches. Every patch > should be finished and self-containing. Well, previous version of this patch was rejected as "needs splitting", it's basically too big to be send as a single patch. It can be applied as several smaller chunks or rejected as a whole. I don't think there is any other option.

Feb. 11, 2011

Re: msxml3: Ignore IActiveScript interface for IXMLHTTPRequest

by Nikolay Sivov

On 2/11/2011 13:30, Alistair Leslie-Hughes wrote: > Hi, > > -------------------------------------------------- > From: "Nikolay Sivov" <bunglehead(a)gmail.com> > Sent: Friday, February 11, 2011 9:27 PM > To: "Alistair Leslie-Hughes" <leslie_alistair(a)hotmail.com> > Cc: <wine-devel(a)winehq.org> > Subject: Re: msxml3: Ignore IActiveScript interface for IXMLHTTPRequest > >> On 2/11/2011 12:25, Alistair Leslie-Hughes wrote: >>> Hi, >>> >>> On 11/02/2011 8:12 PM, Nikolay Sivov wrote: >>>> On 2/11/2011 10:51, Alistair Leslie-Hughes wrote: >>>>> Hi, >>>>> >>>>> >>>>> Changelog: >>>>> msxml3: Ignore IActiveScript interface for IXMLHTTPRequest >>>> What's a purpose of this? >>> To remove the FIXME from appearing in the logs. >> What application is that? Does it appear running in browser scripting >> by any chance? > Yes, it was a website that uses XMLHTTPRequest to load files off the > local > filesystem. My point is that browser scripting engine shouldn't query for that interface in a first place probably, so it's interesting to figure out why it's queried instead of silence it. Is it possible to reproduce that with wine-gecko or I need IE? P.S. cc-ing Jacek, as he knows probably why such dead queries happen. > > > Best Regards > Alistair Leslie-Hughes > > >

Feb. 11, 2011

Re: msxml3: Ignore IActiveScript interface for IXMLHTTPRequest

by Alistair Leslie-Hughes

Hi, -------------------------------------------------- From: "Nikolay Sivov" <bunglehead(a)gmail.com> Sent: Friday, February 11, 2011 9:27 PM To: "Alistair Leslie-Hughes" <leslie_alistair(a)hotmail.com> Cc: <wine-devel(a)winehq.org> Subject: Re: msxml3: Ignore IActiveScript interface for IXMLHTTPRequest > On 2/11/2011 12:25, Alistair Leslie-Hughes wrote: >> Hi, >> >> On 11/02/2011 8:12 PM, Nikolay Sivov wrote: >>> On 2/11/2011 10:51, Alistair Leslie-Hughes wrote: >>>> Hi, >>>> >>>> >>>> Changelog: >>>> msxml3: Ignore IActiveScript interface for IXMLHTTPRequest >>> What's a purpose of this? >> To remove the FIXME from appearing in the logs. > What application is that? Does it appear running in browser scripting by > any chance? Yes, it was a website that uses XMLHTTPRequest to load files off the local filesystem. Best Regards Alistair Leslie-Hughes

Feb. 11, 2011

Re: msxml3: Ignore IActiveScript interface for IXMLHTTPRequest

by Nikolay Sivov

On 2/11/2011 12:25, Alistair Leslie-Hughes wrote: > Hi, > > On 11/02/2011 8:12 PM, Nikolay Sivov wrote: >> On 2/11/2011 10:51, Alistair Leslie-Hughes wrote: >>> Hi, >>> >>> >>> Changelog: >>> msxml3: Ignore IActiveScript interface for IXMLHTTPRequest >> What's a purpose of this? > To remove the FIXME from appearing in the logs. What application is that? Does it appear running in browser scripting by any chance? > > Best Regards > Alistair Leslie-Hughes >

Feb. 11, 2011