eembc / audiomark Goto Github PK

It appears there are four places the system integrator needs to be aware of when managing memory.

1. The intermediate buffers declared at the start of ee_audiomark.c:35-42
2. The Hanning and rotation arrays in ee_abf_f32.c:32-33
3. The neural net scratch buffer and layer coefficients in th_api.c:210-239
4. The requests made to th_malloc by the component's NODE_MEMREQ, indicated by an enumeration constant. See th_api.c:th_malloc.

I believe the intent is to use NODE_MEMREQ for fast working memory, and then allow the integrator to partition up the remaining as needed. Not quite clear on this myself, but it should be documented in the user guide.

The three input audio files contain a left, right, and noise. These files each contain 27008 int16_t samples.

Questions:

1. Why 27008 samples and not 16000, one second?
2. Do the audio samples contain one of the known keywords* in English? Laurent suggested in an email that they were french? :)
3. Is there anything special that needs to be added? Laurent mentioned adding since wave noise?

* Keywords:
yes
no
up
down
left
right
on
off
stop
go

1. There are still float types instead of ee_f32_t.
2. Move all int to unit32_t unless a signed type is explicitly required.
3. Suffix all unsigned constants with U.

Frankly speaking, the size of the CMSIS-NN and CMSIS-DSP folders is big, can we use the submodule rather than a solid copy of those libraries here?

Thank you.

All modules need some scratch memory, global (shared) memory. What is the "correct" way to create this memory (esp. if it shared). The memory may be moving through DMA or FIFO in an accelerator and not a slice of shared SRAM. Should there be a th_<NODE>_memreq function that the developer may edit in order to properly set their XDAIS pointers? How much flexibility is too much (is there even a thing as "too much").

The speexdsp component supports both floating-point and fixed-point implementations. It is set to fixed-point but the top level API is for floating-point (f32) implementation with fixed-point audio inputs. In the speexdsp component, change to floating-point by modify config.h file as shown below. It compiles but it does not run properly.
/* Compile as fixed-point */
//#define FIXED_POINT 1

/* Compile as floating-point */
#define FLOATING_POINT /**//

Almost every block uses an FFT. The adaptive beamformer and KWS.mfcc use Arm CMSIS/DSP directly, and all of the libspeexdsp code uses the Xiph API. This means we are asking a developer to address two different port layers for the same code.

It would help the user and make the porting easier if there was just one API/port layer.

Currently the Arm implementation code is mixed in with the benchmark source code.

Historically the porting layer is isolated from the main source and put in an explicit porting-layer directory, along with a "bare bones" file that contains the unimplemented function calls.

Move this code (and libs), create the barebones directory & files, and correct the cmake files to expect a PORT_DIR name, just like CoreMark.

There is cmake file so I tried to build and run the project. To build on MS Visual Studio, I have to comment out line 13 and 14 in CMakeLists.txt file. When I run the test, it crashed. The error message is "File: minkernel\crts\ucrt\src\appcrt\stdio\fread.cpp Line:48 Expression: stream != nullptr"". I tried to insert screenshot of the crash below but not sure if it works.

I noticed a few cases with reused buffers (and found an out of bounds check) with valgrind. This is a reminder to perform static checking before releasing (esp. with strict typecasting, e.g. gcc -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Wconversion). Also: is coverity free? (https://scan.coverity.com/)

idx_microphone_L, idx_microphone_R, idx_downlink, idx_for_asr in hard_coded_demo.c should be initialized to 0. This issue only applicable when _MSC_VER is not defined.

Synopsys asked to review if we are effectively using restricted, as their compiler makes good use of it. This has largely been ignored, other than to fix CMSIS compilation warnings. However, should we consider it four our custom nodes? And should it be a macro, like in CMSIS?

For fun I ran the code through a MISRA linter. How much do we care about being MISRA compliant?

th_memmove is used with overlapping source and destination, as you'd expect, but it has restrict qualifiers on its pointers, leading to undefined behaviour.

This prompts Arm Compiler 6 to use __aeabi_memcpy for the memmove inside the default implementation, rather than __aeabi_memmove.

CMSIS/DSP and CMSIS/NN have their own repositories. CMSIS/NN is from a PR from Felix, and CMSIS/DSP is 5.9.

These should be updated.

(Don't forget about the PrivateIncludes folder in CMSIS/DSP)

When compiling on MSVC, the Arm code generates 20,000+ warnings. And speex has a lot of warnings.

How much do we clean up?

Out-of-the-box it forces memory pointers to spxGlobalHeapPtr, which is an arbitrary value in segfault range (45000).

Restoring os_support.h to the original Xiph library enables the speex_alloc to use library functions. However, the node _NODE_MEMREQ functions still force the pointers to inaccessible memory.

There is no facility to provide a local heap, as spxGlobalHeapPtr is constantly reassigned (at appears to be a "no-free" type allocation). So even if it is assigned by main.c via a calloc(), it is still overwritten.

What is the intention here?

We have a mix of code from Laurent (arm), Kaiping (infineon), Arm repositories, CMSIS reference, etc. Most is Apache 2.0.

I think we can replace all the Apache 2.0 licenses with the EEMBC license as long as the copyright is preserved from the previous writers. E.g., the license changes, but the copyright must remain in the new header. Not sure how to do this correctly.

Almost every component has a set of #define macros that indicate the size of datatypes and arrays, many of which are fixed by the benchmark, and redundant.

These should be reduced and renamed.