search for: output_rows

Aha good point! Im travelling this weekend but will submit another patch Monday morning. Cheers, Drew On Fri, Nov 24, 2017 at 9:15 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > Hi Drew, > > I noticed you reverted the > output[output_rows * i] = (tmp + 16384) >> 15; > from the previous patch. That's still good. What should have been > changed is the float version: > output[output_rows * i] = (1/32768.f) * ((tmp + 16384) >> 15); > which should just be: > output[output_rows * i] = (1/(32768.f*32768.f)) *...

I only had a quick look, but your patch looks good except for the: output[output_rows * i] = (1/(32768.f*128.f))*tmp; For floating point, you shouldn't do the >>7 either. Just remove the >>8 from the floating-point calculation of tmp so that all the scaling is done in float. Cheers, Jean-Marc On 11/27/2017 04:01 PM, Drew Allen wrote: > Hi Jean-Marc, > &gt...

I've solely addressed this concern here. Cheers, Drew On Fri, Nov 24, 2017 at 9:15 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > Hi Drew, > > I noticed you reverted the > output[output_rows * i] = (tmp + 16384) >> 15; > from the previous patch. That's still good. What should have been > changed is the float version: > output[output_rows * i] = (1/32768.f) * ((tmp + 16384) >> 15); > which should just be: > output[output_rows * i] = (1/(32768.f*32768.f)) *...

Hi Jean-Marc, Attached is an updated patch. I had to include some of Mark's suggestions in order to get the tests to work correctly. I will still submit a separate patch for him for a few other concerns he had after this one clears. Cheers, Drew On Thu, Nov 23, 2017 at 10:42 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > Actually, there's also something wrong with the

Done! On Tue, Nov 28, 2017 at 12:12 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > I only had a quick look, but your patch looks good except for the: > output[output_rows * i] = (1/(32768.f*128.f))*tmp; > > For floating point, you shouldn't do the >>7 either. Just remove the >>8 > from the floating-point calculation of tmp so that all the scaling is > done in float. > > Cheers, > > Jean-Marc > > On 11/27/2017 04:...

...n-Marc On 11/28/2017 12:24 PM, Drew Allen wrote: > Done! > > On Tue, Nov 28, 2017 at 12:12 AM Jean-Marc Valin <jmvalin at jmvalin.ca > <mailto:jmvalin at jmvalin.ca>> wrote: > > I only had a quick look, but your patch looks good except for the: >   output[output_rows * i] = (1/(32768.f*128.f))*tmp; > > For floating point, you shouldn't do the >>7 either. Just remove the >>8 > from the floating-point calculation of tmp so that all the scaling is > done in float. > > Cheers, > >         Jean-Marc >...

Hi Drew, I noticed you reverted the output[output_rows * i] = (tmp + 16384) >> 15; from the previous patch. That's still good. What should have been changed is the float version: output[output_rows * i] = (1/32768.f) * ((tmp + 16384) >> 15); which should just be: output[output_rows * i] = (1/(32768.f*32768.f)) * tmp; since there's n...

...ha good point! Im travelling this weekend but will submit another patch > Monday morning. > > Cheers, > Drew > On Fri, Nov 24, 2017 at 9:15 AM Jean-Marc Valin <jmvalin at jmvalin.ca> > wrote: > >> Hi Drew, >> >> I noticed you reverted the >> output[output_rows * i] = (tmp + 16384) >> 15; >> from the previous patch. That's still good. What should have been >> changed is the float version: >> output[output_rows * i] = (1/32768.f) * ((tmp + 16384) >> 15); >> which should just be: >> output[output_rows * i] = (1/...

...M, Drew Allen wrote: >> Done! >> >> On Tue, Nov 28, 2017 at 12:12 AM Jean-Marc Valin <jmvalin at jmvalin.ca >> <mailto:jmvalin at jmvalin.ca>> wrote: >> >> I only had a quick look, but your patch looks good except for the: >> output[output_rows * i] = (1/(32768.f*128.f))*tmp; >> >> For floating point, you shouldn't do the >>7 either. Just remove the >>8 >> from the floating-point calculation of tmp so that all the scaling is >> done in float. >> >> Cheers, >> &gt...

...> I've solely addressed this concern here. > > Cheers, > Drew > > On Fri, Nov 24, 2017 at 9:15 AM Jean-Marc Valin <jmvalin at jmvalin.ca > <mailto:jmvalin at jmvalin.ca>> wrote: > > Hi Drew, > > I noticed you reverted the > output[output_rows * i] = (tmp + 16384) >> 15; > from the previous patch. That's still good. What should have been > changed is the float version: > output[output_rows * i] = (1/32768.f) * ((tmp + 16384) >> 15); > which should just be: > output[output_rows * i] = (1/...

...;mailto:jmvalin at jmvalin.ca> > >>> <mailto:jmvalin at jmvalin.ca <mailto:jmvalin at jmvalin.ca>>> wrote: > >>> > >>>     I only had a quick look, but your patch looks good except > for the: > >>>       output[output_rows * i] = (1/(32768.f*128.f))*tmp; > >>> > >>>     For floating point, you shouldn't do the >>7 either. Just > remove the >>8 > >>>     from the floating-point calculation of tmp so that all the > scaling is > >&gt...

...lines (untested) to improve accuracy: opus_int32 tmp = 0; for (col = 0; col < input_rows; col++) { tmp += (matrix_data[MATRIX_INDEX(matrix->rows, row, col)] * input[MATRIX_INDEX(input_rows, col, i)]) >> 8; } output[MATRIX_INDEX(output_rows, row, i)] = (tmp+64)>>7; 3) Looking at your get_*() function to compute offsets, I think you're missing some align() calls in the terms. I think you could end up with misaligned pointers, which is usually find on x86, but not on other architectures (e.g. ARM). Cheers, Jean-Marc On 1...

Hi Drew, On 10/10/17 02:29 PM, Drew Allen wrote: > 2) Someone on this list might know better than I do on that one, but for > the new _ctl_va_list() calls, I believe the convention is for va_start() > and va_end() to appear in the caller rather than in in the va_list() > function itself. > > *My understanding is that it's impossible to pass ellipsis to another >