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

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