Fix DMR hang: call AMBE codec via C function pointers, not inline asm

The radio hard-froze (needs battery pull) the instant it processed an incoming
DMR frame — repeater reply, or noise on a DMR channel. FM and DMR TX-without-a-
listening-repeater were fine. Cause: codec_interface.c invoked the AMBE codec
through hand-written inline assembler with no operands and no clobber list,
branching (BL) to an absolute address. That code's correctness depends entirely
on a specific compiler's register allocation; modern GNU toolchains (both ARM
GCC 10 and 14) miscompile it, corrupting state on the first codec call.

Port the official R20260504 (git ebd7100) rework: replace all five inline-asm
calls (2x decode, 2x encode, 1x ECC) with C function-pointer calls. Each call
reproduces the exact register/stack layout the assembler set up (AAPCS: args
1-4 -> r0-r3, args 5+ -> stack) and sets the Thumb bit on the target address.
Also drop the trailing semicolons from the AMBE_* address macros in codec.h so
they can be used in C expressions.

Builds cleanly with the GCC 14.2 Docker toolchain (no STM32CubeIDE needed) and
DMR RX/TX now works on the radio.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Marcus Kida 2026-07-06 11:14:06 +02:00
parent 0d8a293199
commit 0867742901
2 changed files with 48 additions and 117 deletions

View file

@ -48,13 +48,13 @@
#define QU(X) QUAUX(X) #define QU(X) QUAUX(X)
#if defined(PLATFORM_MD9600) || defined(PLATFORM_MDUV380) || defined(PLATFORM_MD380) || defined(PLATFORM_RT84_DM1701) #if defined(PLATFORM_MD9600) || defined(PLATFORM_MDUV380) || defined(PLATFORM_MD380) || defined(PLATFORM_RT84_DM1701)
#define AMBE_DECODE 0x08075954; #define AMBE_DECODE 0x08075954
#define AMBE_ENCODE 0x080754ac; #define AMBE_ENCODE 0x080754ac
#define AMBE_ENCODE_ECC 0x08075864; #define AMBE_ENCODE_ECC 0x08075864
#else #else
#define AMBE_DECODE 0x0005543D; #define AMBE_DECODE 0x0005543D
#define AMBE_ENCODE 0x00054F94; #define AMBE_ENCODE 0x00054F94
#define AMBE_ENCODE_ECC 0x0005534C; #define AMBE_ENCODE_ECC 0x0005534C
#endif #endif
extern uint8_t ambebuffer_decode[CODEC_DECODE_CONFIG_DATA_LENGTH]; extern uint8_t ambebuffer_decode[CODEC_DECODE_CONFIG_DATA_LENGTH];
extern uint8_t ambebuffer_encode[CODEC_ENCODE_CONFIG_DATA_LENGTH]; extern uint8_t ambebuffer_encode[CODEC_ENCODE_CONFIG_DATA_LENGTH];

View file

@ -28,139 +28,70 @@
#include "dmr_codec/codec.h" #include "dmr_codec/codec.h"
#include "functions/voicePrompts.h" #include "functions/voicePrompts.h"
#include <stdint.h>
#include <string.h> #include <string.h>
static uint16_t bitbuffer_encode[72]; static uint16_t bitbuffer_encode[72];
/*
* The AMBE codec entry points live inside the proprietary codec blob that the
* firmware loader patches into flash at 0x0807537C. They are Thumb functions,
* so a call through a pointer requires bit 0 (the Thumb bit) to be set.
*
* These used to be called with hand-written inline assembler that did
* "BL <absolute address>". Newer GNU toolchains no longer accept a direct
* branch to an absolute address (it fails to link), and even where it links the
* bare asm (no operands / no clobber list) miscompiles and hard-faults on the
* DMR path. Following the official R20260504 rework, the codec is now invoked
* through C function pointers instead.
*
* The parameter lists below reproduce, exactly, the register/stack state the
* original assembler set up (AAPCS: args 1-4 -> r0-r3, args 5+ -> stack).
*/
typedef void (*ambeDecodeFn_t)(void *waveOut, int n, void *bitBufIn, int a4, int a5, int halfFrame, void *ambeState);
typedef void (*ambeEncodeFn_t)(void *bitBufOut, int a2, void *waveIn, int n, int a5, int halfFrame, int a7, void *ambeState);
typedef void (*ambeEncodeEccFn_t)(void *bitBuf0, void *bitBuf1, int a3, void *eccState);
#define AMBE_DECODE_FN ((ambeDecodeFn_t)(uintptr_t)(AMBE_DECODE | 1u))
#define AMBE_ENCODE_FN ((ambeEncodeFn_t)(uintptr_t)(AMBE_ENCODE | 1u))
#define AMBE_ENCODE_ECC_FN ((ambeEncodeEccFn_t)(uintptr_t)(AMBE_ENCODE_ECC | 1u))
void codecDecode(uint8_t *indata_ptr, int numbBlocks) void codecDecode(uint8_t *indata_ptr, int numbBlocks)
{ {
uint16_t bitbuffer_decode[49]; uint16_t bitbuffer_decode[49];
register int r0 asm ("r0") __attribute__((unused));
register int r1 asm ("r1") __attribute__((unused));
register int r2 asm ("r2") __attribute__((unused));
for (int idx = 0; idx < numbBlocks; idx++) for (int idx = 0; idx < numbBlocks; idx++)
{ {
initFrame(indata_ptr, bitbuffer_decode); initFrame(indata_ptr, bitbuffer_decode);
indata_ptr += 9; indata_ptr += 9;
soundSetupBuffer();// this just sets currentWaveBuffer but the compiler seems to optimise out the code if I try to do it in this file // First half-frame
r2 = (int)bitbuffer_decode; soundSetupBuffer();// sets currentWaveBuffer (the decoded-audio output buffer)
r0 = (int)currentWaveBuffer; AMBE_DECODE_FN((void *)currentWaveBuffer, 80, (void *)bitbuffer_decode, 0, 0, 0, (void *)ambebuffer_decode);
r1 = (int)ambebuffer_decode;
asm volatile (
"PUSH {R4-R11}\n"
"SUB SP, SP, #0x10\n"
"STR R1, [SP, #0x08]\n"
"LDR R1, =0\n"
"STR R1, [SP, #0x04]\n"
"LDR R1, =0\n"
"STR R1, [SP, #0x00]\n"
"LDR R3, =0\n"
"LDR R1, =80\n"
"BL " QU(AMBE_DECODE)
"ADD SP, SP, #0x10\n"
"POP {R4-R11}"
);
soundStoreBuffer(); soundStoreBuffer();
soundSetupBuffer();// this just sets currentWaveBuffer but the compiler seems to optimise out the code if I try to do it in this file // Second half-frame
r2 = (int)bitbuffer_decode; soundSetupBuffer();
r0 = (int)currentWaveBuffer; AMBE_DECODE_FN((void *)currentWaveBuffer, 80, (void *)bitbuffer_decode, 0, 0, 1, (void *)ambebuffer_decode);
r1 = (int)ambebuffer_decode;
asm volatile (
"PUSH {R4-R11}\n"
"SUB SP, SP, #0x10\n"
"STR R1, [SP, #0x08]\n"
"LDR R1, =1\n"
"STR R1, [SP, #0x04]\n"
"LDR R1, =0\n"
"STR R1, [SP, #0x00]\n"
"LDR R3, =0\n"
"LDR R1, =80\n"
"BL " QU(AMBE_DECODE)
"ADD SP, SP, #0x10\n"
"POP {R4-R11}"
);
soundStoreBuffer(); soundStoreBuffer();
} }
} }
void codecEncodeBlock(uint8_t *outdata_ptr) void codecEncodeBlock(uint8_t *outdata_ptr)
{ {
register int r0 asm ("r0") __attribute__((unused));
register int r1 asm ("r1") __attribute__((unused));
register int r2 asm ("r2") __attribute__((unused));
memset((uint8_t *)outdata_ptr, 0, 9);// fills with zeros memset((uint8_t *)outdata_ptr, 0, 9);// fills with zeros
memset(bitbuffer_encode, 0, sizeof(bitbuffer_encode));// faster to call memset as it will be compiled as optimised code memset(bitbuffer_encode, 0, sizeof(bitbuffer_encode));
// First half-frame
soundRetrieveBuffer();// sets currentWaveBuffer (the audio input buffer)
AMBE_ENCODE_FN((void *)bitbuffer_encode, 0, (void *)currentWaveBuffer, 80, 0x1840, 0, 0x2000, (void *)ambebuffer_encode);
soundRetrieveBuffer();// gets currentWaveBuffer pointer used as input r2 to the encoder // Second half-frame
soundRetrieveBuffer();
AMBE_ENCODE_FN((void *)bitbuffer_encode, 0, (void *)currentWaveBuffer, 80, 0x0800, 1, 0x2000, (void *)ambebuffer_encode);
r0 = (int)bitbuffer_encode; // ECC pass
r2 = (int)currentWaveBuffer;//tmp_wavbuffer; AMBE_ENCODE_ECC_FN((void *)bitbuffer_encode, (void *)bitbuffer_encode, 0, (void *)ambebuffer_encode_ecc);
r1 = (int)ambebuffer_encode;// seems to be a hard coded (defined) memory address of 0x1FFF6B60. I'm not sure why it has to be hard coded, since its passed as a paramater (register)
asm volatile (
"PUSH {R4-R11}\n"
"SUB SP, SP, #0x14\n"
"STR R1, [SP, #0x0C]\n"
"LDR R1, =0x00002000\n"
"STR R1, [SP, #0x08]\n"
"LDR R1, =0\n"
"STR R1, [SP, #0x04]\n"
"LDR R1, =0x00001840\n"
"STR R1, [SP, #0x00]\n"
"LDR R3, =80\n"
"LDR R1, =0\n"
"BL " QU(AMBE_ENCODE)
"ADD SP, SP, #0x14\n"
"POP {R4-R11}"
);
soundRetrieveBuffer();// gets currentWaveBuffer pointer used as input r2 to the encoder
r0 = (int)bitbuffer_encode;
r2 = (int)currentWaveBuffer;//tmp_wavbuffer;
r1 = (int)ambebuffer_encode;
asm volatile (
"PUSH {R4-R11}\n"
"SUB SP, SP, #0x14\n"
"STR R1, [SP, #0x0C]\n"
"LDR R1, =0x00002000\n"
"STR R1, [SP, #0x08]\n"
"LDR R1, =1\n"
"STR R1, [SP, #0x04]\n"
"LDR R1, =0x00000800\n"
"STR R1, [SP, #0x00]\n"
"LDR R3, =80\n"
"LDR R1, =0\n"
"BL " QU(AMBE_ENCODE)
"ADD SP, SP, #0x14\n"
"POP {R4-R11}"
);
r0 = (int)bitbuffer_encode;
r1 = (int)ambebuffer_encode_ecc;
asm volatile (
"PUSH {R4-R11}\n"
"SUB SP, SP, #0x14\n"
"MOV R3, R1\n"
"LDR R2, =0\n"
"MOV R1, R0\n"
"BL " QU(AMBE_ENCODE_ECC)
"ADD SP, SP, #0x14\n"
"POP {R4-R11}"
);
for (int i = 0; i < 72; i++) for (int i = 0; i < 72; i++)
{ {