[speex-dev] [PATCH] Make SSE Run Time option. Add Win32 SSE code

[Jean-Marc Valin]

Hi,

Thanks for the patch. I think it's a good idea, although I can't apply
it as is. The reason is that in its current form, the SSE version is not
tested enough and isn't very clean in some aspects. For example, the
order 10 filter is hard-coded and patched to work also for order 8 (less
efficiently). Also, I think this should really go into 1.1.x (to become
1.2). I have already found a faster implementation, which is not yet in
CVS BTW.

About your SPEEX_ASM flags, I'm not sure I see the difference between
SPEEX_ASM_MMX_SSE and SPEEX_ASM_MMX_SSE_FP. Also, you're saying that the
current code makes use of SSE2, which I don't think is the case, since I
developed it on a Pentium III, which only supports SSE1. I don't think
SSE2 is important at all, since most of the SSE2 instructions are for
double precision (which Speex doesn't use at all). 

Last thing, I see in your windows version a bunch of commented pops and
pushes. Those are definitely needed. You compiler may happen to produce
code without that, but there's no guarantee you won't run into problems
later because suddenly, the compiler assumes that whatever was there
before is still there.

        Jean-Marc

Le ven 09/01/2004 à 00:18, Aron Rosenberg a écrit :
> All,
> 
>          Attached is a patch that does two things. First it makes the use 
> of the current SSE code a run time option through the use 
> of  speex_decoder_ctl() and speex_encoder_ctl
> It does this twofold. First there is a modification to the configure.in 
> script which introduces a check based upon platform. It will compile in the 
> sse assembly if you are on an i?86 based platform by making a special 
> define. Second, it adds a new ctl value called SPEEX_SET_ASM_FLAG  which 
> takes in an integer. The values are defined as:
> 
> #define SPEEX_SET_ASM_FLAG              200
> #define SPEEX_ASM_MMX_NONE              0
> #define SPEEX_ASM_MMX_BASIC             1
> #define SPEEX_ASM_MMX_SSE               2
> #define SPEEX_ASM_MMX_SSE_FP    4
> 
> The current Speex SSE code requires full SSE2 support which corresponds to 
> SPEEX_ASM_MMX_SSE_FP. None of the other defines are actively used, but they 
> are included since they represent different Intel/AMD processors. For 
> example, an AMD Duran only supports SPEEX_ASM_MMX_BASIC while Pentium 3's 
> and above support full SPEEX_ASM_MMX_SSE_FP
> 
> 
> The second part of the patch adds the equivalent MS Windows assembler for 
> the same sections that currently have GCC x86 assembler code.
> 
> Notes about implementation: We took the easiest route when hacking in the 
> flag support which was to add a global flag for the entire library at 
> runtime and extern it in all the various files.
> Jean-Marc: We looked at adding the flag into the state structures, however 
> they were not passed all the way down into the filters.c files and it would 
> have been a massive change to make it pass all the needed data. The 
> approach we took should be ok since on a given machine you would have the 
> same settings. The decoder_ctl and encoder_ctl set the same global flag 
> variable.
> 
> The way we setup the asm flags var should allow you to add the ARM assembly 
> in the exact same manor. You would add a check in the configure.in for the 
> platform and define a _USE_ARM and place the code in the same functions as 
> we did. You would then add a SPEEX_ASM_ARM 8  or something and let the 
> application decide to turn it on.
> 
> 
> Other Notes: This patch obsoletes ltp_sse.h and filters_sse.h  . However 
> the patch does not remove them. This is thge updated version of the patch 
> we sent in November.
> 
> 
> Comments are welcome.  BTW, we have been shipping our Video Conferencing 
> product which only uses the Speex codec for 6 months now and have gotten 
> rave reviews (PC Magazine Editors choice) for the audio and video quality. 
> We use Speex in Windows, Mac OS-X, and Linux as we have clients for each 
> platform. Keep up the great work! Check us out at 
> http://www.sightspeed.com  and please try our beta version (Mac and Windows 
> Clients available now) at http://www.sightspeed.com/page.php?page=beta


-- 
Jean-Marc Valin, M.Sc.A., ing. jr.
LABORIUS (http://www.gel.usherb.ca/laborius)
Université de Sherbrooke, Québec, Canada


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: Ceci est une partie de message numériquement signée.
Url : http://lists.xiph.org/pipermail/speex-dev/attachments/20040109/be8389a8/signature.pgp