search for: silk_sum_sqr_shift

...us repository, I can send you the suggested patch. Cheers, Marcello On 18/06/2014 08:09, "Jean-Marc Valin" <jmvalin at jmvalin.ca> wrote: >Hi Marcello, > >It turns out that the problem has a much simpler explanation. As far as >I can tell, it's actually a bug in silk_sum_sqr_shift() and this trivial >patch appears to fix the problem: >http://jmvalin.ca/misc_stuff/sum_sqr_shift_fix.patch > >It would still require some testing to check that the fix doesn't have >any bad side effect. Let me know how well the fix works for you. Again, >thanks for investigat...

...s 16 rather than 2 (which is what would be expected with a sine wave). - The problem seems to happen when rshifts >= 3 - when pre-scaling the signal to be < 16384 the problem goes away (patch scale_burg_in.diff) - When calculating C0 and rshifts based on a 64 bits correlation instead of using silk_sum_sqr_shift the problem also goes away (patch new_C0_calc.diff) I suspect that for very high prediction gain the fixed point implementation becomes very sensitive to numerical error, but I am not too sure why the new versions work better. I favour the version with the new C0 calculation, as it avoids rescalin...

...defined. The cast back to int is implementation-defined but we are already assuming two's complement behaviour every time we are shifting. As for using the 64-bit version anyway, I'm not sure of the impact since I'm not the original author of the code. Koen, what are the advantages of silk_sum_sqr_shift() over the 64-bit version? See any issue with using one over the other? Cheers, Jean-Marc On 20/06/14 06:10 AM, Marcello Caramma (mcaramma) wrote: > Hi Jean-Marc, > > well spotted! The patch provided fixes the issue for me. > > Nevertheless, in my code (and I would suggest doin...

...t; int is implementation-defined but we are already assuming two's > complement behaviour every time we are shifting. > > As for using the 64-bit version anyway, I'm not sure of the impact since > I'm not the original author of the code. Koen, what are the advantages > of silk_sum_sqr_shift() over the 64-bit version? See any issue with > using one over the other? > > Cheers, > > Jean-Marc > > On 20/06/14 06:10 AM, Marcello Caramma (mcaramma) wrote: > > Hi Jean-Marc, > > > > well spotted! The patch provided fixes the issue for me. > &gt...

Hi Marcello, It turns out that the problem has a much simpler explanation. As far as I can tell, it's actually a bug in silk_sum_sqr_shift() and this trivial patch appears to fix the problem: http://jmvalin.ca/misc_stuff/sum_sqr_shift_fix.patch It would still require some testing to check that the fix doesn't have any bad side effect. Let me know how well the fix works for you. Again, thanks for investigating this. Cheers, Jea...

...gt; what would be expected with a sine wave). > - The problem seems to happen when rshifts >= 3 > - when pre-scaling the signal to be < 16384 the problem goes away (patch > scale_burg_in.diff) > - When calculating C0 and rshifts based on a 64 bits correlation instead > of using silk_sum_sqr_shift the problem also goes away (patch > new_C0_calc.diff) > > I suspect that for very high prediction gain the fixed point > implementation becomes very sensitive to numerical error, but I am not too > sure why the new versions work better. > I favour the version with the new C0 calc...

Hi, Apologies if this is a known issues, but I have found what I believe is a bug in the fixed point implementation of the Silk codec and could not find any mention on this in the archives. The bug can be easily reproduced with the fixed point demo program (./configure ?enable-fixed-point ?disable-float-api && make) using the following command: ./opus_demo voip 16000 1 23000