Speex dev - Apr 2006 - Major internal changes, TI DSP build change

> You found it.  The SHL32 (not SHR32) line fixes the problem.  It must be 
> doing a 16-bit shift, then extending the result (which is reasonable).  As 
> it happens, that it the same macro which gave us trouble last May 
> (25th/26th), when the C55 build was more subtlely broken.
Yes, that's what I finally remembered. I think I've fixed all
occurrences (by adding EXTEND32) in svn. Could you check if that's the
case? 
> I will test the C54 build and send a patch tomorrow with updates to the
.pjt
> files (adding window.c) and bits.c, as I described earlier.
Good, thanks.

	Jean-Marc

>> You found it.  The SHL32 (not SHR32) line fixes the problem.  It must
be
>> doing a 16-bit shift, then extending the result (which is reasonable). 
>> As
>> it happens, that it the same macro which gave us trouble last May
>> (25th/26th), when the C55 build was more subtlely broken.
>
> Yes, that's what I finally remembered. I think I've fixed all
> occurrences (by adding EXTEND32) in svn. Could you check if that's the
> case?
>
>> I will test the C54 build and send a patch tomorrow with updates to the
>> .pjt
>> files (adding window.c) and bits.c, as I described earlier.
Build 11169 in SVN works correctly.  I have attached a zip file (renamed 
.txt) with a patch to bits.c to make the byteswapping for TI DSPs 
consistent.  I also added a switch so that byte swapping could be enabled or 
disabled in config.h.  There are also updates to the .pjt files to add 
window.c, and updates in the test main files in the TI directory to account 
for the reduction in delay from 10ms to 5ms.  There is also an added file: 
the C6x build needs speex/speex_config_types.h, so I created a speex 
subdirectory under TI with this file.  The subversion patch generation tool 
would not handle an added file.

I first made a patch against build 11146, applied this against 11169, made a 
couple of other changes, and made a new patch agains 11169.  The second 
patch is larger because some line ends got changed in a couple of the files 
when the first patch was applied.  I am sorry for the clutter, but its my 
first time with the patch tool.

I have one lingering concern.  The C6x encoder does not produce the same 
results as the C55x and C54x.  The waveform reproduction is less accurate as 
measured by a sample by sample comparison.  In the test programs, the SNR is 
11.10 in the C55x and C54x, and 10.79 in the C6x build.  I figured that the 
encoders might not produce bit-exact results because of differences in their 
wordlengths, but at one time they were the same (1.1.8, I think).  The 
decoders do produce the same results.  Do you think that this is anything to 
be worried about?

Finally, in the simulator I measured the peak MIPs for the C55x as 29.4, 
where it was 41.5 in Speex 1.1.8.  I was expecting some improvement from the 
continuing work you have been doing on the fixed point build, but this is 
really impressive, almost too good to be true.  This is all straight 
compiled C, after all (though the compiler has been updated also).

- Jim 
-------------- next part --------------
PKQX?4??`h?Qti_dsp_11169.patch?<ks?J???*????????
??J?]?@g??J?`????7'?}??!?	d???[???
??kz?{?["mgL?^??=????Y?h?;??j??g{?\.?)??{???m?!?j?????U,W?n]???\??????'?????R?_????E~????l????=?
iu;????A?l

]na?????1????????sG?$??C?	:h???
9?_??=??q?????7/???:m?t?
S??????P&??I?*A??J?/I?j\?7?=&?D<?????x?<???c#;8S"????
?nl' 3????o???]?*????a??%?W~;\NL???Z?*
@?!
?@?b*?s0 "?V? C?>\?[?s`?&???.??7
??dfy??#??r?5??#+?mJ??&?[C]?B???{?P? ???u??B0_??y??)p	@s?@j??????
??"zAR?????H?
?
???<B6?v??Bg>?s6
?p?+g????[?????lL
7 
Eh???2w/???'?8??i??!?CyS????	?y???/y?z?t??????????7?Yv?X?\?m?bm?.?t?????o?
?y???
?h????2??O????
f?S??B^??Z??}@}?d??p??8?7????m????q???s?h??2>??g?|?@?????o??????dR???<'????????Nz?M?P_??$,?)???(NE??2@Mc???7g????q{?F
??E?_e?^so?7???8???Z??[?W??"D
?????2wf?p??p??M?,E	64???$?K???,????k,????~?Z

???
??rn??aP??5??r????B?J?GQ?????
?
?I?X?f?0????????????X^??u?{??`?9?&v??(??W?Jp??W?K????x???t?a???????h'Mw?j??
U_}[	?W1*:?H??\f??"?????L??'M??)y???
?j?m%??A(o??g_OHod?R9"?M??~@??U???.[?;{F_I?i???
?2??.?vF????	?=???vxwW???????????Q?w=????g	???
L?dZ?
??M???????????B???????G??x?p?A>H???7?ldh;?w??PO???ow0eTJ????_???:????{??2H?G??S????7
??k??????!??g????t?H?
??o??v???w?
>5{F??i?#???@?D"\t???q??=??C??Sr?<o?7`?p?????}i???vg?????p???f?j??L?????3???>1????mF??
Y?-??>???'?!??
2uA???'?)L<kNKd??????`w"2???y??

 ?D$???G?B(?
???Z?Y:X?????av;g?CH???GU6E?O??k???????%??B??	????v?1
k?4z???a6???-??|J??1_??X\ ~|m??3???N???v_
??s	?=)?0???g???w???????\@?`?3??0r?s??,??=???T??*?6?S4NS???g?????jMl?
L),???-???9?4???f?>(?????B?0E]???C?????h???M??R?$??[???u??
$??)%Q??JM?ZTY???v??E?k?u;???m?M?-?f]W-???????}?.L^?8I????S\'Va
|?<k4?*??d08?

? BI???bT?(QV??l4
??H??"l?'c????
4V??{lle????????,?H3?OV?`RTU??
??$??jD??:
??
,???8WU???l?
?#??"pdJ?A???	?)?W4???*?W????7??j????5q7j~?????????8c??02
\?????p?^??]?=x??l#???-jb-W?`1W????
?s.<??t???D>R????????71?.??=?-
??7?Sl????dj???a;??WX?;??F????I???o(?U`_??_?`?I8&?B2?~p??
?????0?????????????_??x	?(??6,
??!O1??/???
?ql?????x?b?.@?y??*??a?|?B6???
l?vw???|?"?3?f??G??(?{3?????MS?omg??&Q??6?3*?????3??|?c{???G?U
?b?Ie?????'???C?'Bx??
??~??6?OM???_|???????P???5?/?u?c????C???S?:??
?a??*??????:?i???rl????n??
9????5<?????
V*?p????????FC?????uP?ndV+f?f2
2X?G^???wlP?B???
????eUr???(B?D??k"???tY??o??I?= ????S
??
?^?????$???????8)?????????0EYy
#?v?$?
?#CY?p???{9
?2M???1?????n????
????R<zU:?V?h	?NQ_DeB?v?????;? ???.?%e????D?s?i?8??sf2?X?
?g?2K?V
???A?,?w??G?!,???
{QQW?D??0uNc??l'?????z%?c?2??yM?<?rr%"??	???^????\?c:??Ig?
????V??u.?x?L{??`8??
?????)v?[?????5?l;??????U?j????_?U??OI]RfJ??????%)???0?#[?l;?????8?a;!N?{?~Q?6???j<?>d"??Us?
?\??_?#?|6'???WU-^
?(??????????S>?3?|? ?"???)???\??;?????????U?????d???O??'???5??6
?n4?%8???q???a??-"^
x?'?H?%?6?=?:%by??s;??A7?.???????)7?vx02?8??k?Q
????!?????v??y?c?v??x???/????2??m/Vv?o?????1???`DBI)S&???W?S{?1?)!c|??FE?Rrcb??K,??A??
??`$?/g??????`??E/<?zn??}?5hw;y qJ?????=>?0?z???[U?|Ik???)?#?
F?M<???>?d??w???
Z3?"?c_2?q}X????Q??5???W?!?????
?|??k?szy|?????{xL??sWd"??g??#
??	??|8;??-Lx?!$??W???hv?O?**?xx-$??????>?&??&??e__????l'
???
???????~???O?M?hT ?'?&??0wQ'??f??
_[????M??m??h?c:?}?G???-
<?
??d?]:?.k,mTI??????eR????,?kJl??gqT????i??h?Y??Ndp(a?%?G??$???????1-e?G???v? V
???&??g??=?'?/$?O?y???PD??P&?*??@??????Q<??;?O3>Y:+8=L4es
Y??q`?
?Y?#&u!G?,?????X?=??$???
????F?Q|V?y
<????'Yxk??)????L??6??~?"<??"fD?+X,?R??
???g??NCE}|e?4???VB?J?SK[??p>??-?4?????????b??Q????Bf>??e<???8????*?3#??|V??(]????k??P?."?q???-*VDW???%???S2??<T@y
???P?Q[^&%????B?????1M ?[?hF
|?+?@?K?q)?g~??#?<?SU
1

???06DF
?gLa`f???d??J%?1(X2?)???w??+??]-	F%?*
?5a^_T??????&?-fh)?
??_QeH???X??
d??#?
K?W??gj|>F?Kl???$(?iw??RLB???(m????kL?
.?x????+?Y?
j??`f??dbyxhvjH=?x??427?f???8<?X4_?Ht??@\???{
????@??}h?i?FX??????f??????CcQd1????~?????_G?	K/z&*??

 ?Q~?Q??vA????I???
ah%;D????(CZ?*G?l-??0*e4?g??D??????V?y??Y??S???????X|{&
.??|>,E???n??????
Tk?S??!-????V-?
,??.?=Y??,???P?f??;???\????\9?
?Tp?h,??????
)?-???'?$?b??>sI?Y?*-?v?*;?_?IX?U?^???v?MT?n??`zKq?,???-????:`??C?_+6???G???
????T??j?K?????VQ?{m??F{j????5?T??R?m??um) ???E????+??3????????R?,?M?peJ;?
??(??Q?6??
	?_???a?????7?"r?}(??h?????vS?7?k1?g7u?????ML????+Y?K?????&N??????m%
z}WI???J\i?i?<@b???A???R?
~^z??FF?%%?f?$
???Pc}???
}??$??O????-$?1??%???<`H??.j??.??^?=???B?c8e?r+?(??_??R?Fb???'??o???
Um?????6???M
???xz?E?Y?????Zh?????b????H?ab?%aS{%q1jw
?Mro%aN?6$6V"9?m??'???SY??T4?!??
????}?Q3% ???|
T??=?????G
??m????(?tbE(Rm?hB?
?d?D19l?h???'?.??D\?5I4Y2?Ht?'l?D?`6??A?pJn?????????6?????G??L?{???????oh??=??Ul?PKx?[3????cspeex_config_types.h?UMo?6??
?M??n?
vO?D?dI%?$>i??
??!????w?r??&E/?!1??y???????n_Z????,:?/??_A??????7???f????j?\.l?4e???n????2
H
?B???1?????mAq???|~ ????WW???v????;! J?9
O?[?S???o??N?p?f??
???n7????v??:-tm\???
??U?9?k?????j_???
?(}?]G([??YT?@?j??vc?N?D?????V?^i?Y???i
m?{??B?6??Jk???v?????Tnv??Vwd%W???B??Fc;?????5???????V\?+??????`?#S^???z???.?K$??.v????c????6U?[S???u?-B=p?,??????h?D?;?]
m?c?k??Vcy????C
=4?$rx?{MS?j,???]M??6?????9???<b????qv?=?(???n?????????'??c5j?%?l?nC????nx?b?1?
???????i??LH?wS%??P????{(???O>?s`w?`RB&????#"?a?8?x??()b?N@H3E0	?q
?*|??'!???h?_?O????c?R*7?
??B??HBy!?L2 
1?Q????$?0??*??0I|?qV?q?x?Bv,FY?????Ws?"E??-D?I2g??;??B1??????0q8'????3???(*?m?C#??*?I???r??
???I&?i
dQa?W
(hf`??????S??(rR?
?Es?i??c?3?B???L??????N?9?]???E?8
K???H,??l??	K#F	?r???q
?M????X????_??_r????
??C?p????Y?|?:??h?w8??
??e?/?%????j?3YN??dp?_
?Q?dpy????????????7
???1??R???s??????N??	??-]???4???e???5?<??C??}?????
??/???
?N???7??{U?
5??7f?)o?????
???PKQX?4??`h?Q ??ti_dsp_11169.patchPKx?[3????c ??5speex_config_types.hPK??

Hi Jim,
> Build 11169 in SVN works correctly.  
Good. I'll try not to forget the EXTEND32 from now on.
> I have attached a zip file (renamed 
> .txt) with a patch to bits.c to make the byteswapping for TI DSPs 
> consistent.  
Seems like unzip can't read it. Either it's in an unknown format or the
file got corrupted. Could simply send as multiple (uncompressed)
attachments? BTW, broken down patches would be nice.
> I also added a switch so that byte swapping could be enabled or 
> disabled in config.h.  
Why would you want to turn it on/off?
> There are also updates to the .pjt files to add 
> window.c, and updates in the test main files in the TI directory to account
> for the reduction in delay from 10ms to 5ms.  There is also an added file: 
> the C6x build needs speex/speex_config_types.h, so I created a speex 
> subdirectory under TI with this file.  The subversion patch generation tool
> would not handle an added file.
Actually, speex_config_types.h is not necessary for the TI builds. It's
best to put the definition directly in speex_types.h.
> I first made a patch against build 11146, applied this against 11169, made
a
> couple of other changes, and made a new patch agains 11169.  The second 
> patch is larger because some line ends got changed in a couple of the files
> when the first patch was applied.  I am sorry for the clutter, but its my 
> first time with the patch tool.
> 
> I have one lingering concern.  The C6x encoder does not produce the same 
> results as the C55x and C54x.  The waveform reproduction is less accurate
as
> measured by a sample by sample comparison.  In the test programs, the SNR
is
> 11.10 in the C55x and C54x, and 10.79 in the C6x build.  I figured that the
> encoders might not produce bit-exact results because of differences in
their
> wordlengths, but at one time they were the same (1.1.8, I think).  The 
> decoders do produce the same results.  Do you think that this is anything
to
> be worried about?
Yes, it worries me a bit. Not the fact that the SNR are slightly
different (one file is probably not enough to say which is "better"),
but the fact that they differ at all for the same fixed-point version.
Could you check when the results started being different. There might be
the same kind of error as earlier, but on a less critical bit of data.
> Finally, in the simulator I measured the peak MIPs for the C55x as 29.4, 
> where it was 41.5 in Speex 1.1.8.  I was expecting some improvement from
the
> continuing work you have been doing on the fixed point build, but this is 
> really impressive, almost too good to be true.  This is all straight 
> compiled C, after all (though the compiler has been updated also).
I think this is probably due to some filters that got changed from
32-bit accuracy to 16-bit. There is an option to use *all* filters with
16-bit accuracy (just define PRECISION16) and I suspect the difference
wouldn't be very large. Note that the quality isn't be as good with that
option on, whereas the changes I made recently don't hurt quality.

	Jean-Marc