C port of core encoder and decoder #33

drowe67 · 2024-11-06T23:17:08Z

General strategy is to follow the fine work from DRED/RDOVAE. Scope for this PR is x86 float (int8/other arches later) Vanilla float might actually be fast enough, given the slow frame rate.

Summary:

C port of RADE core encoder and decoder
ability to compile in model weights or load from binary blob, tool for writing blobs
surprising result that (after C port) core enc/dec CPU load is small, even with non SIMD "vanilla" float
Tx stack CPU load also small
CPU is dominated by Rx Python DSP 🤔
Some small optimisations to Python dsp.py

TODO

CPU Load Characterisation of Rx DSP + Core Dec

radae_rxe.py, which includes Rx DSP plus Core Decoder. Using all.wav (49 seconds), modern Desktop Intel(R) Core(TM) i5-7600 CPU @ 3.50GHz

The lines below:

generate test data
Python Rx DSP + C core decoder
Python DSP + Python core decoder
Just Python DSP

ctest -V -R radae_rx_profile
\time -o log.txt -f '%e' cat rx.f32 | PYTHONPATH='.' build/src/radae_rx > features_c.f32
\time -o log.txt -f '%e' cat rx.f32 | python3 -m cProfile -s time radae_rxe.py --no_stdout | head -n20
\time -o log.txt -f '%e' cat rx.f32 | python3 -m cProfile -s time radae_rxe.py --bypass_dec  --no_stdout | head -n20

To measure just the C core dec:

cat rx.f32 | python3 radae_rxe.py --bypass_dec > z_hat.f32
time cat z_hat.f32 | ./build/src/test_rade_dec 1 > /dev/null

	DSP+ core dec	DSP	core dec
C	15.35		0.11
Python	30.3	15.03

These results suggest the C core decoder CPU is small (almost 0) compared to the DSP (currently coded in Python). Quite a surprising result. Note the core enc/dec is just using vanilla float (no SIMD).

This suggests (i) the CPU load for the RADE enc/dec is quite low (ii) the next target for optimisation should be the Rx DSP (iii) the overall CPU load will be dominated by the FARGAN decoder (iv) further optimisation of the RADE core enc/dec (e.g. SIMD) may not be warranted/is low priority.

Re (ii) - we should not leap into DSP optimisation for RADE V1 (especially a C port) without careful thought. The DSP will change for RADE V2 so this may be wasted effort. Most of the effort is spent in DSP coding and having the DSP in Python makes this much easier and greatly speeds our development. There may be some simple algorithmic changes we can make in the Python code to speed up performance (i.e. without a full C port).

CPU Load of complete Rx Stack (including FARGAN)

As measured by:

ctest -V -R radae_rx_stack_py
ctest -V -R radae_rx_stack_c

	CPU %
Python	58
C	33

These results also suggest most of the CPU is in the Rx DSP, i.e. Rx DSP >> FARGAN (at least currently). Another test to demonstrate the breakdown between the RADE Rx and FARGAN synthesis (using rx.f32 from all.wav):

\time -o log.txt -f '%e' cat rx.f32 | PYTHONPATH='.' build/src/radae_rx > features_c.f32
\time -o log.txt -f '%e' cat features_c.f32 | ./build/src/lpcnet_demo -fargan-synthesis - /dev/null

	Time(s)
radae_rx	16.15
lpcnet_demo	0.33

CPU Load of complete Tx Stack (including feat extract)

As measured by:

ctest -V -R radae_tx_stack_c

	CPU %
C	4.5

drowe67 · 2024-11-08T19:21:09Z

@tmiw - is there a reason we are using -fvisibility=hidden and this sort of stuff:

#if IS_BUILDING_RADE_API
#if _WIN32
#define RADE_EXPORT __declspec(dllexport) __stdcall
#else
#define RADE_EXPORT __attribute__((visibility("default")))
#endif // _WIN32
#else
#if _WIN32
#define RADE_EXPORT __declspec(dllimport) __stdcall
#else
#define RADE_EXPORT 
#endif // _WIN32
#endif // IS_BUILDING_RADE_API

Maybe keeping the namespace as clean as possible?

drowe67 · 2024-11-08T19:25:40Z

Oh Ok it's a libopus convention. Hmm, I need to use some "hidden" function calls inside libopus 🤔

tmiw · 2024-11-08T22:00:09Z

I actually included the above code because I was running into linking issues when trying to integrate it with freedv-gui, but it does make sense that forcing -fvisibility=hidden would cause that to be needed. What hidden functions are you trying to use?

…mpt at fixing Issue #7, patch to export libopus selected nnet.h funcs

…by export_weights

… ctest busted

…ng sensible

… model05 & model19_check3, both ctests work, updated opus nnet.h patch

…or bbfm_sc_internal

…ed_c passing OK

drowe67 · 2024-11-19T21:58:09Z

@tmiw - when convenient - could you pls review this PR? In particular the rade_api.h changes and src/CMakeList.txt with a view to using on Windows.

tmiw · 2024-11-20T07:47:32Z

src/CMakeLists.txt

 target_include_directories(radae_rx PRIVATE
    "$<TARGET_PROPERTY:Python3::NumPy,INTERFACE_INCLUDE_DIRECTORIES>")

+add_library(radecore rade_enc.c rade_dec.c rade_enc_data.c rade_dec_data.c)


Does it make sense to have a separate radecore library just for the C port? There'd have to be modifications on the freedv-gui side to also make sure libradecore.dll got properly packaged along with librade.dll.

Sure now the port is done I can reconcile into one library.

tmiw · 2024-11-20T07:48:55Z

src/opus-nnet.h.diff

This should really be handled by the opus project (i.e. through some sort of ./configure flag) as major code changes on their end could require changes to the patch.

Maybe, but changes to Opus is outside of the scope of this review, ie we are unlikely to get them to accept a patch in the near term. This code (nnet.h) has been pretty stable for several years and its a small patch so this approach should be workable for us in the near term as RADE evolves.

tmiw · 2024-11-20T07:50:52Z

src/rade_api.h

@@ -71,6 +71,12 @@ extern "C" {
 #define RADE_MODEM_SAMPLE_RATE 8000           // modem waveform sample rate
 #define RADE_SPEECH_SAMPLE_RATE 16000         // speech sample rate

+// init rade_open() flags
+#define RADE_USE_C_ENCODER 0x1


Question: is there a scenario where one would actually want to use Python for RX or TX (and C for the opposite)? If not, should these two be consolidated into one flag?

For testing, e.g. C encoder driving Python decoder. If you swapped both to C and a test failed you wouldn't know which one had bugs.

tmiw · 2024-11-21T20:30:37Z

Question--where's config.h coming from? I tried using this branch in freedv-gui and ended up with the following error:

/Users/mooneer/devel/freedv-gui/build_osx/rade_src/src/rade_enc.c:31:10: fatal error: 'config.h' file not found
   31 | #include "config.h"
      |          ^~~~~~~~~~
/Users/mooneer/devel/freedv-gui/build_osx/rade_src/src/rade_dec.c:29:10: fatal error: 'config.h' file not found
   29 | #include "config.h"
      |          ^~~~~~~~~~
1 error generated.
make[5]: *** [src/CMakeFiles/rade.dir/rade_enc.c.o] Error 1
make[5]: *** Waiting for unfinished jobs....
1 error generated.
make[5]: *** [src/CMakeFiles/rade.dir/rade_dec.c.o] Error 1
/Users/mooneer/devel/freedv-gui/build_osx/rade_src/src/rade_api.c:48:10: fatal error: 'config.h' file not found
   48 | #include "config.h"
      |          ^~~~~~~~~~
/Users/mooneer/devel/freedv-gui/build_osx/rade_src/src/rade_enc_data.c:5:10: fatal error: 'config.h' file not found
    5 | #include "config.h"
      |          ^~~~~~~~~~
1 error generated.
make[5]: *** [src/CMakeFiles/rade.dir/rade_api.c.o] Error 1

drowe67 · 2024-11-21T22:02:48Z

Question--where's config.h coming from? I tried using this branch in freedv-gui and ended up with the following error:

It's the opus config.h. Seems to build Ok on my two Ubuntu machines and GitHub.

tmiw · 2024-11-22T08:31:21Z

Question--where's config.h coming from? I tried using this branch in freedv-gui and ended up with the following error:

It's the opus config.h. Seems to build Ok on my two Ubuntu machines and GitHub.

I think it was something specific to macOS. I was able to get it to build on its own, but the warning below came back:

In file included from /Users/mooneer/devel/freedv-gui/build_osx/rade_src/src/test_rade_dec.c:14:
In file included from /Users/mooneer/devel/freedv-gui/build_osx/rade_src/src/rade_core.h:35:
/Users/mooneer/devel/freedv-gui/build_osx/rade_build/build_opus_arm-prefix/src/build_opus_arm/dnn/nnet.h:159:2: warning: "Only SSE and SSE2 are available. On newer machines, enable SSSE3/AVX/AVX2 using -march= to get better performance" [-W#warnings]
  159 | #warning "Only SSE and SSE2 are available. On newer machines, enable SSSE3/AVX/AVX2 using -march= to get better performance"
      |  ^

Trying to build into freedv-gui now.

tmiw · 2024-11-22T09:00:14Z

OK, I'm now able to build freedv-gui on both macOS and Windows with this branch. Will need to test actual execution when I have a chance.

tmiw · 2024-11-22T09:10:15Z

OTOH, now freedv-gui won't build on Linux:

[ 91%] Linking CXX executable freedv
cd /home/runner/work/freedv-gui/freedv-gui/build_linux/src && /usr/local/bin/cmake -E cmake_link_script CMakeFiles/freedv.dir/link.txt --verbose=1
/usr/bin/ld: freedv: hidden symbol `linear_init' in ../build_opus-prefix/src/build_opus/.libs/libopus.a(parse_lpcnet_weights.o) is referenced by DSO
/usr/bin/ld: final link failed: bad value
collect2: error: ld returned 1 exit status

I may need to think about this some more.

tmiw · 2024-11-22T22:35:39Z

I think the issue is that we're trying to do something different than what Opus does (i.e. make symbols public in our case when they're private in the original library). Maybe we can copy in the implementations of the various functions being used in the C port to avoid needing to link Opus in at all (or only for the tools and not the library).

Alternatively, maybe we can build Opus as a dynamic library here instead of a static one. Then the build would produce both a libopus.dll/so/dylib as well as a librade.dll/so/dylib that freedv-gui would need to bring in. Since we're focused mainly on Windows (and macOS to a lesser extent) right now, this wouldn't necessarily be bad (but could be once we get to a place where we can think about packaging).

drowe67 · 2024-11-23T03:51:38Z

I think the issue is that we're trying to do something different than what Opus does (i.e. make symbols public in our case when they're private in the original library).

So why is it a problem to expose a few library functions? It seems to work just fine for me on Linux.

tmiw · 2024-11-23T05:46:52Z

I think the issue is that we're trying to do something different than what Opus does (i.e. make symbols public in our case when they're private in the original library).

So why is it a problem to expose a few library functions? It seems to work just fine for me on Linux.

It's a problem mainly with integrating the library with others. I worked around my issue for now by having the freedv-gui build use the version of opus built by RADE (i.e. it no longer builds its own copy). I am getting a lot of warnings like this one, though:

In file included from /home/mooneer/freedv-gui/src/freedv_interface.cpp:23:
In file included from /home/mooneer/freedv-gui/src/main.h:88:
In file included from /home/mooneer/freedv-gui/src/freedv_interface.h:49:
In file included from /home/mooneer/freedv-gui/build_win_unsigned/rade_build/build_opus-prefix/src/build_opus/dnn/fargan.h:31:
In file included from /home/mooneer/freedv-gui/build_win_unsigned/rade_build/build_opus-prefix/src/build_opus/dnn/fargan_data.h:5:
/home/mooneer/freedv-gui/build_win_unsigned/rade_build/build_opus-prefix/src/build_opus/dnn/nnet.h:31:9: warning: 'RADE_EXPORT' macro redefined [-Wmacro-redefined]
#define RADE_EXPORT __attribute__((visibility("default")))
        ^
/home/mooneer/freedv-gui/build_win_unsigned/rade_src/src/rade_api.h:50:9: note: previous definition is here
#define RADE_EXPORT __declspec(dllimport) __stdcall
        ^
1 warning generated.

But AFAICT they don't seem to affect execution (only tested with Windows so far).

tmiw · 2024-11-23T07:23:21Z

Warnings look like they're gone now with the latest commit. (BTW I'm working in drowe67/freedv-gui#774 right now for testing.)

drowe67 · 2024-11-23T20:05:20Z

It's a problem mainly with integrating the library with others. I worked around my issue for now by having the freedv-gui build use the version of opus built by RADE (i.e. it no longer builds its own copy).

Yep 👍 That's the correct way to build the overall project. It's not a work around - we are patching libopus so of course we need to use only the patched version.

drowe67 · 2024-11-25T09:03:05Z

@tmiw - I'll merge this now, just open another PR if there are any more tweaks you need.

drowe67 added 9 commits November 6, 2024 18:35

wip export_rade_weights.py

d6d508d

added extra cols to make divisible by 8, might bite us later

000b456

wip - export script running, needs clean up and a bunch of name changes

a8b22de

clean up weight exporting

64d4618

typo

3cd792b

Merge branch 'dr-bbfm' into dr-cport

8bafe95

Merge branch 'main' into dr-cport

da7a5ab

wip C encoder

df81adb

wip - first build of libradecore

5482f85

drowe67 mentioned this pull request Nov 9, 2024

Building Opus code with vector acceleration #7

Closed

drowe67 added 17 commits November 9, 2024 11:41

wip C port of core encoder, test_rade_enc.c builds (not tested), atte…

3b060b7

…mpt at fixing Issue #7, patch to export libopus selected nnet.h funcs

test_rade_enc.c reading 26 el feature vectors, model init bombing

df8e23b

mod radae_txe.py to be a core encoder test tool

0776873

backed out of adding cols in export_weights for dec_output layer

2fd18ff

stop annoying warnings with cmake 3.22.1 on Ubuntu 22

69c19e7

move WeightArray radeenc_arrays extern so it doesn't get overwritten …

a94b3ff

…by export_weights

C encoder doing sensible thinsg with model05, added ctest

62f98ec

typos

b2e80bb

added bottleneck support to core encoder

12e5d58

C core encoder doing sensible things with model19_check3, but model05…

01471f6

… ctest busted

first pass writing and reading weight binary blob files doing somethi…

5d7247f

…ng sensible

manual mods post export_weights to accomodate different input_dim for…

55dd56c

… model05 & model19_check3, both ctests work, updated opus nnet.h patch

use RNG with the same seed for every run to give repeatable results f…

878af3e

…or bbfm_sc_internal

first pass a C core decoder and ctests

c748fbd

README.md notes on C port of core enc/dec

ff5591e

Merge branch 'dr-cport' of github.com:drowe67/radae into dr-cport

37c15a9

first pass of C core encoder being used with RADE C API, radae_tx_emb…

da1c12e

…ed_c passing OK

drowe67 added 5 commits November 20, 2024 11:33

doc EOO frame format

09ef372

wip optimise refine()

3eba4e8

wip optimise refine()

f67a82f

wip optimise bpf()

94ae11d

radae_rx_awgn ctest tweak

d56d572

tmiw reviewed Nov 20, 2024

View reviewed changes

reconciled librade and libradecore into one library

8aec5a7

tmiw added 2 commits November 21, 2024 23:54

Add CMake logic to make sure Opus actually builds before librade.

0c777c2

Universal builds should also include the root opus dir.

e6e99ed

tmiw added 4 commits November 22, 2024 00:36

Partially back out previous change as it breaks compile on Linux.

c9fe9ed

Not sure why we need to link opus to librade only on macOS.

fde5dbc

Need to link opus on Windows too.

18de6de

Disable compilation of code that can't compile on Windows.

8797ede

Only define RADE_EXPORT if it's not already defined.

98cf722

drowe67 mentioned this pull request Nov 25, 2024

Enable capture of RX features from RADE decoder drowe67/freedv-gui#776

Merged

drowe67 merged commit d05ed99 into main Nov 25, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

C port of core encoder and decoder #33

C port of core encoder and decoder #33

drowe67 commented Nov 6, 2024 •

edited

Loading

drowe67 commented Nov 8, 2024 •

edited

Loading

drowe67 commented Nov 8, 2024

tmiw commented Nov 8, 2024

drowe67 commented Nov 19, 2024

tmiw Nov 20, 2024

drowe67 Nov 20, 2024

drowe67 Nov 20, 2024

tmiw Nov 20, 2024

drowe67 Nov 20, 2024

tmiw Nov 20, 2024

drowe67 Nov 20, 2024

tmiw commented Nov 21, 2024

drowe67 commented Nov 21, 2024

tmiw commented Nov 22, 2024

tmiw commented Nov 22, 2024

tmiw commented Nov 22, 2024

tmiw commented Nov 22, 2024

drowe67 commented Nov 23, 2024

tmiw commented Nov 23, 2024

tmiw commented Nov 23, 2024

drowe67 commented Nov 23, 2024

drowe67 commented Nov 25, 2024

C port of core encoder and decoder #33

C port of core encoder and decoder #33

Conversation

drowe67 commented Nov 6, 2024 • edited Loading

CPU Load Characterisation of Rx DSP + Core Dec

CPU Load of complete Rx Stack (including FARGAN)

CPU Load of complete Tx Stack (including feat extract)

drowe67 commented Nov 8, 2024 • edited Loading

drowe67 commented Nov 8, 2024

tmiw commented Nov 8, 2024

drowe67 commented Nov 19, 2024

tmiw Nov 20, 2024

Choose a reason for hiding this comment

drowe67 Nov 20, 2024

Choose a reason for hiding this comment

drowe67 Nov 20, 2024

Choose a reason for hiding this comment

tmiw Nov 20, 2024

Choose a reason for hiding this comment

drowe67 Nov 20, 2024

Choose a reason for hiding this comment

tmiw Nov 20, 2024

Choose a reason for hiding this comment

drowe67 Nov 20, 2024

Choose a reason for hiding this comment

tmiw commented Nov 21, 2024

drowe67 commented Nov 21, 2024

tmiw commented Nov 22, 2024

tmiw commented Nov 22, 2024

tmiw commented Nov 22, 2024

tmiw commented Nov 22, 2024

drowe67 commented Nov 23, 2024

tmiw commented Nov 23, 2024

tmiw commented Nov 23, 2024

drowe67 commented Nov 23, 2024

drowe67 commented Nov 25, 2024

drowe67 commented Nov 6, 2024 •

edited

Loading

drowe67 commented Nov 8, 2024 •

edited

Loading