Jean-Marc Valin wrote:>Hi Jamey, > >Really cool to see Speex being ported to the C55xx and I'd be glad to >integrate the changes required in Speex (and the style's fine with me). > >I have the encoder and decoder running now and have verified that the encoder is bit-exact wrt to the fixed-point code running on x86 for the same 30-second audio sample. Encode and decode together run in real-time for 8KHz data, complexity=3, on 120MHz C5509 when code and data are all in on-chip SRAM. I have not tested the wideband codec yet.>Here are a couple comments on the patch you sent (I looked at it, but >haven't compiled). > >1) The changes you made to the pack un unpack functions would only work >if the 16-bit chars are "big endian" (relative to the two bytes in the >16-bit chars) > >I fixed the problem where extracting bytes from SpeexBits was wrong endian.>2) I would prefer spx_int16_t to int16_t. Eventually, part of that >should go in the configure script and the config.h > >I changed int32_t to spx_int32_t.>3) Not relative to this patch, but I think an important issue will be >what to do about the codebooks. All codebooks are stored as "signed >char", you'd be wasting lots of space on the C55xx unless you find a way >of packing everything. >I have not addressed this problem yet. In my testing all the Speex code and data in the on-chip sram of TI C5509 DSP, and it all fit. Is compute_weighted_codebook the only routine that looks at the raw codebooks? If so, I'll take a look at packing the codebooks to save a few KB. Here is a description of the changes. 1) update arch.h to provide definitions of spx_int32_t, spx_sig_t, etc, for 16-bit processor. 2) update fixed_generic.h with typecasts to compensate for difference between GCC and TI's C compiler. I believe that the result is portable so I have not wrapped this in ifdefs. TI's compiler, unlike GCC on 32bit machines, produces 16bit results for shift, add, and multiply if the operands are 16-bit, even if result will be expanded to 32-bit. There were also some occurrances of this problem in a few of the files, and tracking those down was quite tedious. 3) update SpeexBits implementation to work with 8 or 16bit characters. Patch is attached. Jamey -------------- next part -------------- A non-text attachment was scrubbed... Name: speex-1.1.6-jeh1.patch Type: text/x-patch Size: 37358 bytes Desc: not available Url : http://lists.xiph.org/pipermail/speex-dev/attachments/20041029/facb8c73/speex-1.1.6-jeh1-0001.bin
> I have the encoder and decoder running now and have verified that the > encoder is bit-exact wrt to the fixed-point code running on x86 for the > same 30-second audio sample. Encode and decode together run in > real-time for 8KHz data, complexity=3, on 120MHz C5509 when code and > data are all in on-chip SRAM. I have not tested the wideband codec yet.Cool! Just curious, how much of the DSP does it take to do that?> I fixed the problem where extracting bytes from SpeexBits was wrong endian.Good!> I have not addressed this problem yet. In my testing all the Speex code > and data in the on-chip sram of TI C5509 DSP, and it all fit. Is > compute_weighted_codebook the only routine that looks at the raw > codebooks? If so, I'll take a look at packing the codebooks to save a > few KB.compute_weighted_codebook (and the corresponding decoding function) is the only function using the codebooks that have "exc" in the name. The are two other places that use other codebooks.> Patch is attached.I looked at the patch. Seems mostly OK, though there are some minor issues. First, I don't think it's right to say spx_int_32_t=long except for alpha. This will likely cause problems on Itanium, Power64, SPARC64 and probably x86-64. I suggest either an explicit check for C55xx (and later others) or a test in configure.ac. Also, I'm not sure I get your use of "+#ifdef UNUSED" in the SpeexBits code. Finally, there's a few changes which I think should be removed, like for speex_alloc, speex_warning, ... same as the "#if 0" for wideband and other debug stuff. Could you clean the patch and send it back to me so I can apply it? Jean-Marc
Jean-Marc Valin wrote:>>I have the encoder and decoder running now and have verified that the >>encoder is bit-exact wrt to the fixed-point code running on x86 for the >>same 30-second audio sample. Encode and decode together run in >>real-time for 8KHz data, complexity=3, on 120MHz C5509 when code and >>data are all in on-chip SRAM. I have not tested the wideband codec yet. >> >> > >Cool! Just curious, how much of the DSP does it take to do that? > >It took all of the DSP at that speed. The C5509 will go up to 200MHz, so I guess it's 2/3 of the DSP. We're going to profile the code to see if we can speed it up.> >I looked at the patch. Seems mostly OK, though there are some minor >issues. First, I don't think it's right to say spx_int_32_t=long except >for alpha. This will likely cause problems on Itanium, Power64, SPARC64 >and probably x86-64. I suggest either an explicit check for C55xx (and >later others) or a test in configure.ac. > >Also, I'm not sure I get your use of "+#ifdef UNUSED" in the SpeexBits >code. Finally, there's a few changes which I think should be removed, >like for speex_alloc, speex_warning, ... same as the "#if 0" for >wideband and other debug stuff. Could you clean the patch and send it >back to me so I can apply it? > >I'll clean up the patch as suggested. I meant to remove all the extraneous changes but I guess I missed some. A test in configure.ac for sizeof(long) makes more sense than the ifdef I put in there. Jamey