[Speex-dev] Speex on TI C6x, Problem with TI C5x Patch

[Stuart Cording]

Hi Jean-Marc,

I think some of this is already covered, but I'll comment anyway.

> > 1) We need our own "fixed_xx.h" header file. I don't know why, and 
>haven't
> > had time to investigate, but there is a definite improvement when I use 
>the
> > attached fixed_c55x.h file which has turned all the maths into inline
> > functions.
>
>Did you try with fixed_generic.h or just with fixed_debug.h?
>fixed_debug.h uses int and short directly, so I know it won't work with
>the C5x. However, I think fixed_generic.h should work and has all the
>operators defined as macros anyway, so inlining isn't a problem.

I was working with fixed_generic.h, but from what I have seen Jim already 
found the fix in the PSHR32?

> > Some optimisation or something is probably possible here to
> > reduce code size and inline the functions, as by default the C55x 
>compiler
> > does not seem to inline them (perhaps due to debugging mode). This can 
>be
> > enabled with a C55X_ASM definition following the ARM fixed point math
> > definition convention, and some it could be converted to assembler in 
>the
> > future.
>
>The assembly definitions for the operators are only useful if you have
>gcc-like inline assembly. Otherwise, the explicit register loads will
>make it worse.

We supprt gcc-like inline assembler - I'll try out some optimisation from 
the compiler first and see if how the assembler comes out. If it looks like 
it can be written better I'll attempt to write some assembler.

> > 2) Proper definitions for the speex types are required in the 
>speex_types.h
> > file - I did this and you can enable it via a __C55X__ definition. File
> > attached. My definition follows the convention of other defines in this
> > file. It could be covered by the C55X_ASM define above, but the content 
>of
> > this file is not going to have assembler in it. I leave it up to you if
> > think this is wrong or right - just tell me and I'll follow!
>
>I didn't include the C5x in speex_types.h because I was under the
>impression that autoconf would be used, but I can add it.

Thanks.

> > 3) There seem to be further int/long on a C55X issues in nb_celp.c for 
>the
> > nb_encoder_ctl and nb_decoder_ctl functions. I think that all the
> > (*(int*)ptr) or *((int*)ptr) should be (*(long*)ptr) for a C55X. I don't
> > know why. What I presume was happening was that the data passed to the
> > function in void *ptr was being lost in the upper or lower half of a 
>32-bit
> > word. So it didn't matter what you passed as a setting for 
>SPEEX_SET_QUALITY
> > and SPEEX_SET_COMPLEXITY, the (*(int*)ptr) always = 0. For quality this
> > doesn't matter I don't think due to the default settings, but the
> > st->complexity always ended up as zero, and the decoded bit-stream ends 
>up
> > sounding very ropy.
>
>Actually, I don't see why long/int would be a problem here since you're
>also passing an (int*) and the values are never higher than 32767.

I think you are correct - I made a mistake in my main application (took your 
advise and used #define int long) and think I confused myself in the 
process. nb_celp.c is fine - I am the problem :o)


Thanks for the feedback,

Stuart