Remove redundant comments and fix time import

- Fix missing time import in audio.go causing build failure
- Remove 15 redundant comments that restate obvious code:
  * Hot path function docblocks (jetkvm_audio_read_encode, jetkvm_audio_decode_write)
  * Obvious state descriptions (capture_channels_swapped)
  * SIMD function docblock (simd_clear_samples_s16)
  * safe_alsa_open docblock
  * relay.go implementation comments (Connect if not connected, Read message, etc.)
  * Duplicate RFC 7587 comment in cgo_source.go

- Fix CRITICAL misleading comment: mutex protection claim
  * OLD: "The mutexes protect... ALSA I/O" (FALSE - mutex released during I/O)
  * NEW: "Mutexes protect handle lifecycle, NOT the ALSA I/O itself"
  * Added explanation of race detection via handle pointer comparison

- Reduce verbose function comments (SetAudioOutputEnabled, SetAudioInputEnabled)
  * Removed redundant first line restating function names
  * Kept valuable behavioral context (blocking, timeout)

Net result: 30 lines removed, improved code clarity, fixed build error
This commit is contained in:
Alex P 2025-11-24 20:40:28 +02:00
parent dc0ccf9af5
commit 8debd07b04
4 changed files with 8 additions and 38 deletions

View File

@ -5,6 +5,7 @@ import (
"io" "io"
"sync" "sync"
"sync/atomic" "sync/atomic"
"time"
"github.com/jetkvm/kvm/internal/audio" "github.com/jetkvm/kvm/internal/audio"
"github.com/jetkvm/kvm/internal/logging" "github.com/jetkvm/kvm/internal/logging"
@ -249,8 +250,7 @@ func setPendingInputTrack(track *webrtc.TrackRemote) {
go handleInputTrackForSession(track) go handleInputTrackForSession(track)
} }
// SetAudioOutputEnabled enables or disables audio output capture. // SetAudioOutputEnabled blocks up to 5 seconds when enabling.
// When enabling, blocks up to 5 seconds waiting for audio to start.
// Returns error if audio fails to start within timeout. // Returns error if audio fails to start within timeout.
func SetAudioOutputEnabled(enabled bool) error { func SetAudioOutputEnabled(enabled bool) error {
if audioOutputEnabled.Swap(enabled) == enabled { if audioOutputEnabled.Swap(enabled) == enabled {
@ -282,8 +282,7 @@ func SetAudioOutputEnabled(enabled bool) error {
return nil return nil
} }
// SetAudioInputEnabled enables or disables audio input playback. // SetAudioInputEnabled blocks up to 5 seconds when enabling.
// When enabling, blocks up to 5 seconds waiting for audio to start.
// Returns error if audio fails to start within timeout. // Returns error if audio fails to start within timeout.
func SetAudioInputEnabled(enabled bool) error { func SetAudioInputEnabled(enabled bool) error {
if audioInputEnabled.Swap(enabled) == enabled { if audioInputEnabled.Swap(enabled) == enabled {

View File

@ -54,7 +54,7 @@ static snd_pcm_t *pcm_playback_handle = NULL; // INPUT: Client microphone → de
static const char *alsa_capture_device = NULL; static const char *alsa_capture_device = NULL;
static const char *alsa_playback_device = NULL; static const char *alsa_playback_device = NULL;
static bool capture_channels_swapped = false; // True if hardware reports R,L instead of L,R static bool capture_channels_swapped = false;
static OpusEncoder *encoder = NULL; static OpusEncoder *encoder = NULL;
static OpusDecoder *decoder = NULL; static OpusDecoder *decoder = NULL;
@ -104,11 +104,9 @@ static uint32_t max_backoff_us_global = 500000;
static atomic_int capture_stop_requested = 0; static atomic_int capture_stop_requested = 0;
static atomic_int playback_stop_requested = 0; static atomic_int playback_stop_requested = 0;
// Mutexes to protect concurrent access to ALSA handles and codecs throughout their lifecycle // Mutexes protect handle lifecycle and codec operations, NOT the ALSA I/O itself.
// These prevent race conditions when jetkvm_audio_*_close() is called while // The mutex is temporarily released during snd_pcm_readi/writei to prevent blocking.
// jetkvm_audio_read_encode() or jetkvm_audio_decode_write() are executing. // Race conditions are detected via handle pointer comparison after reacquiring the lock.
// The mutexes protect initialization, cleanup, ALSA I/O, codec operations, and handle validation
// to ensure handles remain valid from acquisition through release.
static pthread_mutex_t capture_mutex = PTHREAD_MUTEX_INITIALIZER; static pthread_mutex_t capture_mutex = PTHREAD_MUTEX_INITIALIZER;
static pthread_mutex_t playback_mutex = PTHREAD_MUTEX_INITIALIZER; static pthread_mutex_t playback_mutex = PTHREAD_MUTEX_INITIALIZER;
@ -187,9 +185,6 @@ static void init_alsa_devices_from_env(void) {
// SIMD-OPTIMIZED BUFFER OPERATIONS (ARM NEON) // SIMD-OPTIMIZED BUFFER OPERATIONS (ARM NEON)
/**
* Clear audio buffer using NEON (16 samples/iteration with 2x unrolling)
*/
static inline void simd_clear_samples_s16(short * __restrict__ buffer, uint32_t samples) { static inline void simd_clear_samples_s16(short * __restrict__ buffer, uint32_t samples) {
const int16x8_t zero = vdupq_n_s16(0); const int16x8_t zero = vdupq_n_s16(0);
uint32_t i = 0; uint32_t i = 0;
@ -293,11 +288,6 @@ static unsigned int get_hdmi_audio_sample_rate(void) {
return detected_rate; return detected_rate;
} }
/**
* Open ALSA device with exponential backoff retry
* @return 0 on success, negative error code on failure
*/
// High-precision sleep using nanosleep
static inline void precise_sleep_us(uint32_t microseconds) { static inline void precise_sleep_us(uint32_t microseconds) {
struct timespec ts = { struct timespec ts = {
.tv_sec = microseconds / 1000000, .tv_sec = microseconds / 1000000,
@ -806,11 +796,6 @@ int jetkvm_audio_capture_init() {
return 0; return 0;
} }
/**
* Read HDMI audio, resample with SpeexDSP, encode to Opus (OUTPUT path hot function)
* @param opus_buf Output buffer for encoded Opus packet
* @return >0 = Opus packet size in bytes, -1 = error
*/
__attribute__((hot)) int jetkvm_audio_read_encode(void * __restrict__ opus_buf) { __attribute__((hot)) int jetkvm_audio_read_encode(void * __restrict__ opus_buf) {
// Two buffers: hardware buffer + resampled buffer (at 48kHz) // Two buffers: hardware buffer + resampled buffer (at 48kHz)
static short CACHE_ALIGN pcm_hw_buffer[MAX_HARDWARE_FRAME_SIZE * 2]; // Max hardware rate * stereo static short CACHE_ALIGN pcm_hw_buffer[MAX_HARDWARE_FRAME_SIZE * 2]; // Max hardware rate * stereo
@ -1007,13 +992,6 @@ int jetkvm_audio_playback_init() {
return 0; return 0;
} }
/**
* Decode Opus, write to device speakers (INPUT path hot function)
* Processing pipeline: Opus decode (with FEC) ALSA playback with error recovery
* @param opus_buf Encoded Opus packet from client
* @param opus_size Size of Opus packet in bytes
* @return >0 = PCM frames written, 0 = frame skipped, -1/-2 = error
*/
__attribute__((hot)) int jetkvm_audio_decode_write(void * __restrict__ opus_buf, int32_t opus_size) { __attribute__((hot)) int jetkvm_audio_decode_write(void * __restrict__ opus_buf, int32_t opus_size) {
static short CACHE_ALIGN pcm_buffer[960 * 2]; // Cache-aligned static short CACHE_ALIGN pcm_buffer[960 * 2]; // Cache-aligned
unsigned char * __restrict__ in = (unsigned char*)opus_buf; unsigned char * __restrict__ in = (unsigned char*)opus_buf;

View File

@ -83,8 +83,6 @@ func (c *CgoSource) Connect() error {
func (c *CgoSource) connectOutput() error { func (c *CgoSource) connectOutput() error {
os.Setenv("ALSA_CAPTURE_DEVICE", c.alsaDevice) os.Setenv("ALSA_CAPTURE_DEVICE", c.alsaDevice)
// Opus uses fixed 48kHz sample rate (RFC 7587)
// SpeexDSP handles any hardware rate conversion
const sampleRate = 48000 const sampleRate = 48000
const frameSize = uint16(sampleRate * 20 / 1000) // 20ms frames const frameSize = uint16(sampleRate * 20 / 1000) // 20ms frames

View File

@ -77,7 +77,6 @@ func (r *OutputRelay) relayLoop() {
consecutiveWriteFailures := 0 consecutiveWriteFailures := 0
for r.running.Load() { for r.running.Load() {
// Connect if not connected
if !(*r.source).IsConnected() { if !(*r.source).IsConnected() {
if err := (*r.source).Connect(); err != nil { if err := (*r.source).Connect(); err != nil {
if consecutiveFailures++; consecutiveFailures >= maxRetries { if consecutiveFailures++; consecutiveFailures >= maxRetries {
@ -93,7 +92,6 @@ func (r *OutputRelay) relayLoop() {
retryDelay = 1 * time.Second retryDelay = 1 * time.Second
} }
// Read message from source
msgType, payload, err := (*r.source).ReadMessage() msgType, payload, err := (*r.source).ReadMessage()
if err != nil { if err != nil {
if !r.running.Load() { if !r.running.Load() {
@ -110,11 +108,9 @@ func (r *OutputRelay) relayLoop() {
continue continue
} }
// Reset retry state on successful read
consecutiveFailures = 0 consecutiveFailures = 0
retryDelay = 1 * time.Second retryDelay = 1 * time.Second
// Write audio sample to WebRTC
if msgType == ipcMsgTypeOpus && len(payload) > 0 { if msgType == ipcMsgTypeOpus && len(payload) > 0 {
r.sample.Data = payload r.sample.Data = payload
if err := r.audioTrack.WriteSample(r.sample); err != nil { if err := r.audioTrack.WriteSample(r.sample); err != nil {
@ -129,7 +125,6 @@ func (r *OutputRelay) relayLoop() {
Msg("Failed to write sample to WebRTC") Msg("Failed to write sample to WebRTC")
} }
// If too many consecutive write failures, reconnect source
if consecutiveWriteFailures >= maxConsecutiveWriteFailures { if consecutiveWriteFailures >= maxConsecutiveWriteFailures {
r.logger.Error(). r.logger.Error().
Int("failures", consecutiveWriteFailures). Int("failures", consecutiveWriteFailures).
@ -140,7 +135,7 @@ func (r *OutputRelay) relayLoop() {
} }
} else { } else {
r.framesRelayed.Add(1) r.framesRelayed.Add(1)
consecutiveWriteFailures = 0 // Reset on successful write consecutiveWriteFailures = 0
} }
} }
} }