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

Krzysztof Nikiel knik00 at gmail.com
Fri Feb 11 10:34:36 CST 2011


2011/2/11 Maarten Lankhorst <m.b.lankhorst at 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
>



More information about the wine-devel mailing list