Fix critical audio race conditions and align configuration defaults

Critical Fixes:
- Fix race condition in handleInputTrackForSession by reloading source inside mutex
- Fix ALSA handle cleanup atomicity (nullify before close to prevent use-after-free)
- Bounds check for opus buffer already present (verified)

Configuration Alignment:
- Align audio bitrate default to 192 kbps across all layers (C, Go defaults, config)
- Align audio complexity default to 8 across all layers
- Align DTX default to enabled (true/1) across all layers for bandwidth efficiency

Documentation Improvements:
- Update C header comment to reflect accurate 192 kbps default
- Clarify NEON requirement (not just "always available")
- Fix ALSA device mapping comments to reflect environment variable usage
- Document fallback behavior in playback init

Code Quality:
- Add validation logging for out-of-range audio configuration values
- Improve error visibility for configuration issues

All changes thoroughly analyzed before implementation.
This commit is contained in:
Alex P 2025-11-18 16:36:38 +02:00
parent da4c6c70d2
commit c88b98c1f0
4 changed files with 43 additions and 19 deletions

View File

@ -51,20 +51,30 @@ func getAudioConfig() audio.AudioConfig {
cfg := audio.DefaultAudioConfig() cfg := audio.DefaultAudioConfig()
if config.AudioBitrate >= 64 && config.AudioBitrate <= 256 { if config.AudioBitrate >= 64 && config.AudioBitrate <= 256 {
cfg.Bitrate = uint16(config.AudioBitrate) cfg.Bitrate = uint16(config.AudioBitrate)
} else if config.AudioBitrate != 0 {
audioLogger.Warn().Int("bitrate", config.AudioBitrate).Uint16("default", cfg.Bitrate).Msg("Invalid audio bitrate, using default")
} }
if config.AudioComplexity >= 0 && config.AudioComplexity <= 10 { if config.AudioComplexity >= 0 && config.AudioComplexity <= 10 {
cfg.Complexity = uint8(config.AudioComplexity) cfg.Complexity = uint8(config.AudioComplexity)
} else {
audioLogger.Warn().Int("complexity", config.AudioComplexity).Uint8("default", cfg.Complexity).Msg("Invalid audio complexity, using default")
} }
cfg.DTXEnabled = config.AudioDTXEnabled cfg.DTXEnabled = config.AudioDTXEnabled
cfg.FECEnabled = config.AudioFECEnabled cfg.FECEnabled = config.AudioFECEnabled
if config.AudioBufferPeriods >= 2 && config.AudioBufferPeriods <= 24 { if config.AudioBufferPeriods >= 2 && config.AudioBufferPeriods <= 24 {
cfg.BufferPeriods = uint8(config.AudioBufferPeriods) cfg.BufferPeriods = uint8(config.AudioBufferPeriods)
} else if config.AudioBufferPeriods != 0 {
audioLogger.Warn().Int("buffer_periods", config.AudioBufferPeriods).Uint8("default", cfg.BufferPeriods).Msg("Invalid buffer periods, using default")
} }
if config.AudioSampleRate == 32000 || config.AudioSampleRate == 44100 || config.AudioSampleRate == 48000 || config.AudioSampleRate == 96000 { if config.AudioSampleRate == 32000 || config.AudioSampleRate == 44100 || config.AudioSampleRate == 48000 || config.AudioSampleRate == 96000 {
cfg.SampleRate = uint32(config.AudioSampleRate) cfg.SampleRate = uint32(config.AudioSampleRate)
} else if config.AudioSampleRate != 0 {
audioLogger.Warn().Int("sample_rate", config.AudioSampleRate).Uint32("default", cfg.SampleRate).Msg("Invalid sample rate, using default")
} }
if config.AudioPacketLossPerc >= 0 && config.AudioPacketLossPerc <= 100 { if config.AudioPacketLossPerc >= 0 && config.AudioPacketLossPerc <= 100 {
cfg.PacketLossPerc = uint8(config.AudioPacketLossPerc) cfg.PacketLossPerc = uint8(config.AudioPacketLossPerc)
} else {
audioLogger.Warn().Int("packet_loss_perc", config.AudioPacketLossPerc).Uint8("default", cfg.PacketLossPerc).Msg("Invalid packet loss percentage, using default")
} }
return cfg return cfg
} }
@ -307,12 +317,19 @@ func handleInputTrackForSession(track *webrtc.TrackRemote) {
continue continue
} }
source := inputSource.Load() // Early check to avoid mutex acquisition if source is nil (optimization)
if source == nil { if inputSource.Load() == nil {
continue continue
} }
inputSourceMutex.Lock() inputSourceMutex.Lock()
// Reload source inside mutex to ensure we have the currently active source
// This prevents races with startInputAudioUnderMutex swapping the source
source := inputSource.Load()
if source == nil {
inputSourceMutex.Unlock()
continue
}
if !(*source).IsConnected() { if !(*source).IsConnected() {
if err := (*source).Connect(); err != nil { if err := (*source).Connect(); err != nil {
@ -321,11 +338,12 @@ func handleInputTrackForSession(track *webrtc.TrackRemote) {
} }
} }
if err := (*source).WriteMessage(0, opusData); err != nil { err = (*source).WriteMessage(0, opusData)
inputSourceMutex.Unlock()
if err != nil {
audioLogger.Warn().Err(err).Msg("failed to write audio message") audioLogger.Warn().Err(err).Msg("failed to write audio message")
(*source).Disconnect() (*source).Disconnect()
} }
inputSourceMutex.Unlock()
} }
} }

View File

@ -193,8 +193,8 @@ func getDefaultConfig() Config {
AudioInputAutoEnable: false, AudioInputAutoEnable: false,
AudioOutputEnabled: true, AudioOutputEnabled: true,
AudioOutputSource: "usb", AudioOutputSource: "usb",
AudioBitrate: 128, AudioBitrate: 192,
AudioComplexity: 5, AudioComplexity: 8,
AudioDTXEnabled: true, AudioDTXEnabled: true,
AudioFECEnabled: true, AudioFECEnabled: true,
AudioBufferPeriods: 12, AudioBufferPeriods: 12,

View File

@ -3,7 +3,7 @@
* *
* Bidirectional audio processing optimized for ARM NEON SIMD: * Bidirectional audio processing optimized for ARM NEON SIMD:
* - OUTPUT PATH: TC358743 HDMI or USB Gadget audio Client speakers * - OUTPUT PATH: TC358743 HDMI or USB Gadget audio Client speakers
* Pipeline: ALSA hw:0,0 or hw:1,0 capture Opus encode (128kbps, FEC enabled) * Pipeline: ALSA hw:0,0 or hw:1,0 capture Opus encode (192kbps, FEC enabled)
* *
* - INPUT PATH: Client microphone Device speakers * - INPUT PATH: Client microphone Device speakers
* Pipeline: Opus decode (with FEC) ALSA hw:1,0 playback * Pipeline: Opus decode (with FEC) ALSA hw:1,0 playback
@ -26,7 +26,7 @@
#include <signal.h> #include <signal.h>
#include <pthread.h> #include <pthread.h>
// ARM NEON SIMD support (always available on JetKVM's ARM Cortex-A7) // ARM NEON SIMD support (required - JetKVM hardware provides ARM Cortex-A7 with NEON)
#include <arm_neon.h> #include <arm_neon.h>
// RV1106 (Cortex-A7) has 64-byte cache lines // RV1106 (Cortex-A7) has 64-byte cache lines
@ -60,7 +60,7 @@ static uint16_t max_packet_size = 1500;
#define OPUS_BANDWIDTH 1104 #define OPUS_BANDWIDTH 1104
#define OPUS_LSB_DEPTH 16 #define OPUS_LSB_DEPTH 16
static uint8_t opus_dtx_enabled = 0; static uint8_t opus_dtx_enabled = 1;
static uint8_t opus_fec_enabled = 1; static uint8_t opus_fec_enabled = 1;
static uint8_t opus_packet_loss_perc = 0; static uint8_t opus_packet_loss_perc = 0;
static uint8_t buffer_period_count = 24; static uint8_t buffer_period_count = 24;
@ -98,7 +98,7 @@ void update_audio_constants(uint32_t bitrate, uint8_t complexity,
uint32_t sr, uint8_t ch, uint16_t fs, uint16_t max_pkt, uint32_t sr, uint8_t ch, uint16_t fs, uint16_t max_pkt,
uint32_t sleep_us, uint8_t max_attempts, uint32_t max_backoff, uint32_t sleep_us, uint8_t max_attempts, uint32_t max_backoff,
uint8_t dtx_enabled, uint8_t fec_enabled, uint8_t buf_periods, uint8_t pkt_loss_perc) { uint8_t dtx_enabled, uint8_t fec_enabled, uint8_t buf_periods, uint8_t pkt_loss_perc) {
opus_bitrate = (bitrate >= 64000 && bitrate <= 256000) ? bitrate : 128000; opus_bitrate = (bitrate >= 64000 && bitrate <= 256000) ? bitrate : 192000;
opus_complexity = (complexity <= 10) ? complexity : 5; opus_complexity = (complexity <= 10) ? complexity : 5;
sample_rate = sr > 0 ? sr : 48000; sample_rate = sr > 0 ? sr : 48000;
capture_channels = (ch == 1 || ch == 2) ? ch : 2; capture_channels = (ch == 1 || ch == 2) ? ch : 2;
@ -429,8 +429,9 @@ static int configure_alsa_device(snd_pcm_t *handle, const char *device_name, uin
// AUDIO OUTPUT PATH FUNCTIONS (TC358743 HDMI Audio → Client Speakers) // AUDIO OUTPUT PATH FUNCTIONS (TC358743 HDMI Audio → Client Speakers)
/** /**
* Initialize OUTPUT path (TC358743 HDMI capture Opus encoder) * Initialize OUTPUT path (HDMI or USB Gadget audio capture Opus encoder)
* Opens hw:0,0 (TC358743) and creates Opus encoder with optimized settings * Opens ALSA capture device from ALSA_CAPTURE_DEVICE env (default: hw:1,0, set to hw:0,0 for TC358743 HDMI)
* and creates Opus encoder with optimized settings
* @return 0 on success, -EBUSY if initializing, -1/-2/-3 on errors * @return 0 on success, -EBUSY if initializing, -1/-2/-3 on errors
*/ */
int jetkvm_audio_capture_init() { int jetkvm_audio_capture_init() {
@ -484,8 +485,9 @@ int jetkvm_audio_capture_init() {
uint16_t actual_frame_size = 0; uint16_t actual_frame_size = 0;
err = configure_alsa_device(pcm_capture_handle, "capture", capture_channels, &actual_rate, &actual_frame_size); err = configure_alsa_device(pcm_capture_handle, "capture", capture_channels, &actual_rate, &actual_frame_size);
if (err < 0) { if (err < 0) {
snd_pcm_close(pcm_capture_handle); snd_pcm_t *handle = pcm_capture_handle;
pcm_capture_handle = NULL; pcm_capture_handle = NULL;
snd_pcm_close(handle);
capture_stop_requested = 0; capture_stop_requested = 0;
capture_initializing = 0; capture_initializing = 0;
return -2; return -2;
@ -499,8 +501,9 @@ int jetkvm_audio_capture_init() {
encoder = opus_encoder_create(actual_rate, capture_channels, OPUS_APPLICATION_AUDIO, &opus_err); encoder = opus_encoder_create(actual_rate, capture_channels, OPUS_APPLICATION_AUDIO, &opus_err);
if (!encoder || opus_err != OPUS_OK) { if (!encoder || opus_err != OPUS_OK) {
if (pcm_capture_handle) { if (pcm_capture_handle) {
snd_pcm_close(pcm_capture_handle); snd_pcm_t *handle = pcm_capture_handle;
pcm_capture_handle = NULL; pcm_capture_handle = NULL;
snd_pcm_close(handle);
} }
capture_stop_requested = 0; capture_stop_requested = 0;
capture_initializing = 0; capture_initializing = 0;
@ -613,7 +616,8 @@ retry_read:
/** /**
* Initialize INPUT path (Opus decoder device speakers) * Initialize INPUT path (Opus decoder device speakers)
* Opens hw:1,0 (USB gadget) or "default" and creates Opus decoder * Opens ALSA playback device from ALSA_PLAYBACK_DEVICE env (default: hw:1,0), falls back to "default" on error
* and creates Opus decoder
* @return 0 on success, -EBUSY if initializing, -1/-2 on errors * @return 0 on success, -EBUSY if initializing, -1/-2 on errors
*/ */
int jetkvm_audio_playback_init() { int jetkvm_audio_playback_init() {
@ -670,8 +674,9 @@ int jetkvm_audio_playback_init() {
uint16_t actual_frame_size = 0; uint16_t actual_frame_size = 0;
err = configure_alsa_device(pcm_playback_handle, "playback", playback_channels, &actual_rate, &actual_frame_size); err = configure_alsa_device(pcm_playback_handle, "playback", playback_channels, &actual_rate, &actual_frame_size);
if (err < 0) { if (err < 0) {
snd_pcm_close(pcm_playback_handle); snd_pcm_t *handle = pcm_playback_handle;
pcm_playback_handle = NULL; pcm_playback_handle = NULL;
snd_pcm_close(handle);
playback_stop_requested = 0; playback_stop_requested = 0;
playback_initializing = 0; playback_initializing = 0;
return -1; return -1;
@ -684,8 +689,9 @@ int jetkvm_audio_playback_init() {
int opus_err = 0; int opus_err = 0;
decoder = opus_decoder_create(actual_rate, playback_channels, &opus_err); decoder = opus_decoder_create(actual_rate, playback_channels, &opus_err);
if (!decoder || opus_err != OPUS_OK) { if (!decoder || opus_err != OPUS_OK) {
snd_pcm_close(pcm_playback_handle); snd_pcm_t *handle = pcm_playback_handle;
pcm_playback_handle = NULL; pcm_playback_handle = NULL;
snd_pcm_close(handle);
playback_stop_requested = 0; playback_stop_requested = 0;
playback_initializing = 0; playback_initializing = 0;
return -2; return -2;

View File

@ -19,7 +19,7 @@ func DefaultAudioConfig() AudioConfig {
Bitrate: 192, Bitrate: 192,
Complexity: 8, Complexity: 8,
BufferPeriods: 12, BufferPeriods: 12,
DTXEnabled: false, DTXEnabled: true,
FECEnabled: true, FECEnabled: true,
SampleRate: 48000, SampleRate: 48000,
PacketLossPerc: 0, PacketLossPerc: 0,