sof: drc: fix drc_inv_fixed function and refine comments - #3810
Conversation
ShriramShastry
left a comment
ShriramShastry
left a comment
There was a problem hiding this comment.
Changes looks good!
int precision_inv; change to int32_t precision_inv;
change qc to QC
ShriramShastry
left a comment
ShriramShastry
left a comment
There was a problem hiding this comment.
Changes looks fine
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
Will get this into v1.7-rc2
Used ABS(x) instead of x for comparing to ONE_OVERT_SQRT2(~0.707) because x could be a negative value and here we want to transfrom x from range (-1, -0.5];[0.5, 1) to (-1, -0.707];[0.707, 1) And revised code line 233~237 to be easier to understand. Signed-off-by: Pin-chih Lin <johnylin@google.com>
3a11268 to
68a574e
Compare
|
Split to 3 commits, PTAL thanks. |
The proposed calculation doesn't take negative or zero input-x into consideration, and we also don't need such use cases. Just return 0 for those input-x. Signed-off-by: Pin-chih Lin <johnylin@google.com>
Refined some function comments to prevent confusion of usage. Signed-off-by: Pin-chih Lin <johnylin@google.com>
68a574e to
b79241f
Compare
yep, range is now non inclusive of 0 |
|
@johnylin76 I've tagged this for v1.7 rc2. DUe in 2 weeks, let me know if we should push this back. |
|
@johnylin76 i think we are just missing the type fixes then we are good |
Finished the type fixes. Thanks a lot. |
ShriramShastry
left a comment
ShriramShastry
left a comment
There was a problem hiding this comment.
Changes looks fine
In order to make the algorithm portable to non-xtensa arch. Signed-off-by: Pin-chih Lin <johnylin@google.com>
1a44345 to
3a52ce4
Compare
| inline int32_t drc_inv_fixed(int32_t x, int precision_x, int precision_y) | ||
| inline int32_t drc_inv_fixed(int32_t x, int32_t precision_x, int32_t precision_y) | ||
| { | ||
| #define qc 25 |
There was a problem hiding this comment.
Can qc be defined in capital [QC] instead of small?
ShriramShastry
left a comment
ShriramShastry
left a comment
There was a problem hiding this comment.
Changes looks fine to me
@ShriramShastry if changes are good please mark "approved" when doing a code review. |
|
Also, @ShriramShastry , we should look eventually at moving common math code to the math lib and de-duplicating functions used across multiple audio components |
|
CI unrelated. |
5 participants
Used ABS(x) instead of x for comparing to ONE_OVERT_SQRT2(~0.707)
because x could be a negative value and here we want to transfrom x
from range (-1, -0.5];[0.5, 1) to (-1, -0.707];[0.707, 1)
Revised code line 237~241 to be easier to understand.
Refined some comments to prevent confusion of function usage.