Dummy keyphrase detection component by slawblauciak · Pull Request #1036 · thesofproject/sof

slawblauciak · 2019-02-22T14:18:15Z

This change introduces a component to be used for integration-testing of keyphrase detection systems.
The component will detect spikes in audio signal and will notify the host whenever they occur.
It will also simulate both memory and DSP load with the given configuration passed in the form of a binary blob.

keyonjie · 2019-02-25T04:44:05Z

@slawblauciak do we need any configuration from driver side via topology for this dummy keyphrase detection component?

keyonjie

Some comments about the default detect func.

slawblauciak · 2019-02-25T11:45:37Z

@keyonjie the component doesn't take any topology configuration. It only accepts binary blob configuration during runtime, before prepare.

ranj063 · 2019-02-25T18:08:00Z

@slawblauciak the 4th commit needs to be split up and some changes need to be squashed with earlier ones ex adding the detec_test.c for compilation should be moved to the 3rd commit.

keyonjie · 2019-02-26T03:16:54Z

@keyonjie the component doesn't take any topology configuration. It only accepts binary blob configuration during runtime, before prepare.

OK, good to know that.

slawblauciak · 2019-03-01T14:23:35Z

Changes applied and fixed the builds. Checks pass successfully now

tlauda · 2019-03-01T14:24:29Z

I think it would be better to rebase instead of merge.

lgirdwood · 2019-04-15T13:11:39Z

@slawblauciak we need a textbook CRC that can be licensed BSD/MIT.

lgirdwood · 2019-04-16T19:56:58Z

@slawblauciak please update to textbook CRC then it's good to merge.

slawblauciak · 2019-04-17T06:40:30Z

@lgirdwood very well. Please note however that the most common approach (and the one you're most likely thinking of) requires a fairly large lookup table, which consists of 256 32-bit values. This is why I've been so reluctant to use such an implementation.

EDIT: The darkest place is right under the candlestick... Plugged the math behind the computation of the lookup table directly into the main crc loop. Sorry.

It will compute crc32 of a given area in memory. Signed-off-by: Slawomir Blauciak <slawomir.blauciak@linux.intel.com>

Provides ipc_send_comp_notification function for sending custom notifications to the host. Signed-off-by: Slawomir Blauciak <slawomir.blauciak@linux.intel.com>

lgirdwood

Did you push the latest update ? Can still see old CRC32 ?

slawblauciak · 2019-04-17T10:12:43Z

Yeah. And I see the changes correctly.

lgirdwood · 2019-04-17T10:27:02Z

Btw, a quick google search shows the CRC32 algo using polynomial 0xEDB88320 is within the public domain e.g. http://home.thep.lu.se/~bjorn/crc/ and https://www.hackersdelight.org/hdcodetxt/crc.c.txt
You could just re-implement this algo with your own code to avoid the table and be licence compliant.

slawblauciak · 2019-04-17T10:40:13Z

Those were the ones I used :) hackersdelight is the one that I used first.

lbetlej

Approved as CRC implementation was fixed.

lgirdwood · 2019-04-17T16:20:57Z

@slawblauciak CI not building on CHT/BYT, can you make sure this component is disabled via Kconfig on those platforms.

q009 · 2019-04-17T16:28:07Z

@lgirdwood it's not a problem with this PR in particular, a lot of PRs are failing for the same reason, on the same platforms.

lgirdwood · 2019-04-17T18:02:32Z

@slawblauciak master is building for me today

make[1]: Leaving directory '/home/lrg/work/sof/sof/build_kbl_gcc'
-rw-r--r-- 1 lrg lrg 159744 Apr 17 18:31 build_apl_gcc/sof-apl.ri
-rw-r--r-- 1 lrg lrg  91768 Apr 17 18:31 build_bdw_gcc/sof-bdw.ri
-rw-r--r-- 1 lrg lrg  92256 Apr 17 18:30 build_byt_gcc/sof-byt.ri
-rw-r--r-- 1 lrg lrg  93088 Apr 17 18:31 build_cht_gcc/sof-cht.ri
-rw-r--r-- 1 lrg lrg 159744 Apr 17 18:31 build_cnl_gcc/sof-cnl.ri
-rw-r--r-- 1 lrg lrg  91768 Apr 17 18:31 build_hsw_gcc/sof-hsw.ri
-rw-r--r-- 1 lrg lrg 159744 Apr 17 18:31 build_icl_gcc/sof-icl.ri
-rw-r--r-- 1 lrg lrg 135168 Apr 17 18:31 build_kbl_gcc/sof-kbl.ri
-rw-r--r-- 1 lrg lrg 135168 Apr 17 18:31 build_skl_gcc/sof-skl.ri
-rw-r--r-- 1 lrg lrg 323584 Apr 17 18:31 build_sue_gcc/sof-sue.ri

Can you do the Kconfig updates for legacy platforms for thisi and I'll force the other PR owners to do likewise. Ideally we need to only enable a base set of components and not all by default.

This is a testing component acting as a dummy keyword detector comp. Add the init/free and cmd op so as to be able to accept switch cmd from userspace. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Slawomir Blauciak <slawomir.blauciak@linux.intel.com>

lgirdwood · 2019-04-18T10:19:32Z

@slawblauciak looks like we have CI code scan error, can you check.

slawblauciak · 2019-04-18T10:23:50Z

@lgirdwood KW scan itself has failed

jajanusz · 2019-04-18T10:25:43Z

+	/* TODO: remove in the future */
+	dev->params.channels = 1;
+
+	if (dev->params.channels != 1) {


CodeCheck complains that you have unrechable code here

@tlauda @mrajwa may I remove line 261 now?

lgirdwood · 2019-04-18T11:47:39Z

@slawblauciak @jajanusz so I'm seeing Codecheck passing now but no push update ?

slawblauciak · 2019-04-18T11:49:07Z

@lgirdwood as I mentioned, the codecheck crashed, that's why it didn't pass before. The issue located is probably non-critical, which is why it's passing.

lgirdwood · 2019-04-18T11:51:45Z

@slawblauciak ok, the unreachable code (if it still exists) can be fixed in a new PR.

slawblauciak requested review from bardliao, dbaluta, jajanusz, lbetlej, lgirdwood, mmaka1, plbossart, singalsu, wwittbrx, xiulipan and zrombel as code owners February 22, 2019 14:18

jajanusz suggested changes Feb 22, 2019

View reviewed changes

Comment thread src/include/sof/audio/component.h Resolved

keyonjie reviewed Feb 25, 2019

View reviewed changes

Comment thread src/audio/detect_test.c Resolved

Comment thread src/audio/detect_test.c Resolved

singalsu reviewed Feb 25, 2019

View reviewed changes

Comment thread src/include/uapi/abi.h Resolved

slawblauciak force-pushed the kd_dummy_comp branch from 8fa6450 to ab6c7d9 Compare February 25, 2019 11:42

mrajwa mentioned this pull request Feb 25, 2019

Key phrase detected: async notifications #1042

Closed

slawblauciak force-pushed the kd_dummy_comp branch 6 times, most recently from 7f7133c to 5a4606d Compare March 1, 2019 13:15

slawblauciak force-pushed the kd_dummy_comp branch 2 times, most recently from ea708d7 to 26710eb Compare March 1, 2019 14:50

slawblauciak force-pushed the kd_dummy_comp branch from 60b9f2a to e943b5d Compare April 11, 2019 08:41

lgirdwood requested changes Apr 15, 2019

View reviewed changes

Comment thread src/math/numbers.c Outdated

lgirdwood Apr 15, 2019

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slawblauciak we need a textbook CRC that can be licensed BSD/MIT.

slawblauciak force-pushed the kd_dummy_comp branch from e943b5d to 5857d2e Compare April 16, 2019 13:45

slawblauciak added 2 commits April 17, 2019 09:20

math: added crc32

e32e9d4

It will compute crc32 of a given area in memory. Signed-off-by: Slawomir Blauciak <slawomir.blauciak@linux.intel.com>

ipc: initial host notification support

fcc5e8d

Provides ipc_send_comp_notification function for sending custom notifications to the host. Signed-off-by: Slawomir Blauciak <slawomir.blauciak@linux.intel.com>

slawblauciak force-pushed the kd_dummy_comp branch from 5857d2e to 813a046 Compare April 17, 2019 07:21

lgirdwood reviewed Apr 17, 2019

View reviewed changes

lbetlej approved these changes Apr 17, 2019

View reviewed changes

lgirdwood approved these changes Apr 17, 2019

View reviewed changes

slawblauciak force-pushed the kd_dummy_comp branch from 813a046 to 0646d2f Compare April 18, 2019 07:09

jajanusz reviewed Apr 18, 2019

View reviewed changes

lgirdwood merged commit 738c83d into thesofproject:master Apr 18, 2019

This was referenced Apr 22, 2019

Add "Test" ipc class thesofproject/linux#852

Merged

ASoC: SOF: ABI unify for KeyWordDetect thesofproject/linux#857

Closed

Gaps of ABI and headers for KeywordDectect thesofproject/linux#858

Closed

jajanusz mentioned this pull request May 10, 2019

[BUG] Detect test shall not be in uapi files #1375

Closed

Conversation

slawblauciak commented Feb 22, 2019

Uh oh!

Uh oh!

keyonjie commented Feb 25, 2019

Uh oh!

keyonjie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slawblauciak commented Feb 25, 2019

Uh oh!

ranj063 commented Feb 25, 2019

Uh oh!

keyonjie commented Feb 26, 2019

Uh oh!

slawblauciak commented Mar 1, 2019

Uh oh!

tlauda commented Mar 1, 2019

Uh oh!

lgirdwood Apr 15, 2019

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Apr 16, 2019

Uh oh!

slawblauciak commented Apr 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

slawblauciak commented Apr 17, 2019

Uh oh!

lgirdwood commented Apr 17, 2019

Uh oh!

slawblauciak commented Apr 17, 2019

Uh oh!

lbetlej left a comment

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Apr 17, 2019

Uh oh!

q009 commented Apr 17, 2019

Uh oh!

lgirdwood commented Apr 17, 2019

Uh oh!

lgirdwood commented Apr 18, 2019

Uh oh!

slawblauciak commented Apr 18, 2019

Uh oh!

jajanusz Apr 18, 2019

Choose a reason for hiding this comment

Uh oh!

slawblauciak Apr 18, 2019

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Apr 18, 2019

Uh oh!

slawblauciak commented Apr 18, 2019

Uh oh!

lgirdwood commented Apr 18, 2019

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

slawblauciak commented Apr 17, 2019 •

edited

Loading