Skip to content

Commit 357e2e6

Browse files
pennycodersclaude
andcommitted
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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent d5c25b9 commit 357e2e6

File tree

4 files changed

+8
-38
lines changed

4 files changed

+8
-38
lines changed

audio.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"io"
66
"sync"
77
"sync/atomic"
8+
"time"
89

910
"github.com/jetkvm/kvm/internal/audio"
1011
"github.com/jetkvm/kvm/internal/logging"
@@ -249,8 +250,7 @@ func setPendingInputTrack(track *webrtc.TrackRemote) {
249250
go handleInputTrackForSession(track)
250251
}
251252

252-
// SetAudioOutputEnabled enables or disables audio output capture.
253-
// When enabling, blocks up to 5 seconds waiting for audio to start.
253+
// SetAudioOutputEnabled blocks up to 5 seconds when enabling.
254254
// Returns error if audio fails to start within timeout.
255255
func SetAudioOutputEnabled(enabled bool) error {
256256
if audioOutputEnabled.Swap(enabled) == enabled {
@@ -282,8 +282,7 @@ func SetAudioOutputEnabled(enabled bool) error {
282282
return nil
283283
}
284284

285-
// SetAudioInputEnabled enables or disables audio input playback.
286-
// When enabling, blocks up to 5 seconds waiting for audio to start.
285+
// SetAudioInputEnabled blocks up to 5 seconds when enabling.
287286
// Returns error if audio fails to start within timeout.
288287
func SetAudioInputEnabled(enabled bool) error {
289288
if audioInputEnabled.Swap(enabled) == enabled {

internal/audio/c/audio.c

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ static snd_pcm_t *pcm_playback_handle = NULL; // INPUT: Client microphone → de
5454

5555
static const char *alsa_capture_device = NULL;
5656
static const char *alsa_playback_device = NULL;
57-
static bool capture_channels_swapped = false; // True if hardware reports R,L instead of L,R
57+
static bool capture_channels_swapped = false;
5858

5959
static OpusEncoder *encoder = NULL;
6060
static OpusDecoder *decoder = NULL;
@@ -104,11 +104,9 @@ static uint32_t max_backoff_us_global = 500000;
104104
static atomic_int capture_stop_requested = 0;
105105
static atomic_int playback_stop_requested = 0;
106106

107-
// Mutexes to protect concurrent access to ALSA handles and codecs throughout their lifecycle
108-
// These prevent race conditions when jetkvm_audio_*_close() is called while
109-
// jetkvm_audio_read_encode() or jetkvm_audio_decode_write() are executing.
110-
// The mutexes protect initialization, cleanup, ALSA I/O, codec operations, and handle validation
111-
// to ensure handles remain valid from acquisition through release.
107+
// Mutexes protect handle lifecycle and codec operations, NOT the ALSA I/O itself.
108+
// The mutex is temporarily released during snd_pcm_readi/writei to prevent blocking.
109+
// Race conditions are detected via handle pointer comparison after reacquiring the lock.
112110
static pthread_mutex_t capture_mutex = PTHREAD_MUTEX_INITIALIZER;
113111
static pthread_mutex_t playback_mutex = PTHREAD_MUTEX_INITIALIZER;
114112

@@ -187,9 +185,6 @@ static void init_alsa_devices_from_env(void) {
187185

188186
// SIMD-OPTIMIZED BUFFER OPERATIONS (ARM NEON)
189187

190-
/**
191-
* Clear audio buffer using NEON (16 samples/iteration with 2x unrolling)
192-
*/
193188
static inline void simd_clear_samples_s16(short * __restrict__ buffer, uint32_t samples) {
194189
const int16x8_t zero = vdupq_n_s16(0);
195190
uint32_t i = 0;
@@ -293,11 +288,6 @@ static unsigned int get_hdmi_audio_sample_rate(void) {
293288
return detected_rate;
294289
}
295290

296-
/**
297-
* Open ALSA device with exponential backoff retry
298-
* @return 0 on success, negative error code on failure
299-
*/
300-
// High-precision sleep using nanosleep
301291
static inline void precise_sleep_us(uint32_t microseconds) {
302292
struct timespec ts = {
303293
.tv_sec = microseconds / 1000000,
@@ -806,11 +796,6 @@ int jetkvm_audio_capture_init() {
806796
return 0;
807797
}
808798

809-
/**
810-
* Read HDMI audio, resample with SpeexDSP, encode to Opus (OUTPUT path hot function)
811-
* @param opus_buf Output buffer for encoded Opus packet
812-
* @return >0 = Opus packet size in bytes, -1 = error
813-
*/
814799
__attribute__((hot)) int jetkvm_audio_read_encode(void * __restrict__ opus_buf) {
815800
// Two buffers: hardware buffer + resampled buffer (at 48kHz)
816801
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() {
1007992
return 0;
1008993
}
1009994

1010-
/**
1011-
* Decode Opus, write to device speakers (INPUT path hot function)
1012-
* Processing pipeline: Opus decode (with FEC) → ALSA playback with error recovery
1013-
* @param opus_buf Encoded Opus packet from client
1014-
* @param opus_size Size of Opus packet in bytes
1015-
* @return >0 = PCM frames written, 0 = frame skipped, -1/-2 = error
1016-
*/
1017995
__attribute__((hot)) int jetkvm_audio_decode_write(void * __restrict__ opus_buf, int32_t opus_size) {
1018996
static short CACHE_ALIGN pcm_buffer[960 * 2]; // Cache-aligned
1019997
unsigned char * __restrict__ in = (unsigned char*)opus_buf;

internal/audio/cgo_source.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,6 @@ func (c *CgoSource) Connect() error {
8383
func (c *CgoSource) connectOutput() error {
8484
os.Setenv("ALSA_CAPTURE_DEVICE", c.alsaDevice)
8585

86-
// Opus uses fixed 48kHz sample rate (RFC 7587)
87-
// SpeexDSP handles any hardware rate conversion
8886
const sampleRate = 48000
8987
const frameSize = uint16(sampleRate * 20 / 1000) // 20ms frames
9088

internal/audio/relay.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ func (r *OutputRelay) relayLoop() {
7777
consecutiveWriteFailures := 0
7878

7979
for r.running.Load() {
80-
// Connect if not connected
8180
if !(*r.source).IsConnected() {
8281
if err := (*r.source).Connect(); err != nil {
8382
if consecutiveFailures++; consecutiveFailures >= maxRetries {
@@ -93,7 +92,6 @@ func (r *OutputRelay) relayLoop() {
9392
retryDelay = 1 * time.Second
9493
}
9594

96-
// Read message from source
9795
msgType, payload, err := (*r.source).ReadMessage()
9896
if err != nil {
9997
if !r.running.Load() {
@@ -110,11 +108,9 @@ func (r *OutputRelay) relayLoop() {
110108
continue
111109
}
112110

113-
// Reset retry state on successful read
114111
consecutiveFailures = 0
115112
retryDelay = 1 * time.Second
116113

117-
// Write audio sample to WebRTC
118114
if msgType == ipcMsgTypeOpus && len(payload) > 0 {
119115
r.sample.Data = payload
120116
if err := r.audioTrack.WriteSample(r.sample); err != nil {
@@ -129,7 +125,6 @@ func (r *OutputRelay) relayLoop() {
129125
Msg("Failed to write sample to WebRTC")
130126
}
131127

132-
// If too many consecutive write failures, reconnect source
133128
if consecutiveWriteFailures >= maxConsecutiveWriteFailures {
134129
r.logger.Error().
135130
Int("failures", consecutiveWriteFailures).
@@ -140,7 +135,7 @@ func (r *OutputRelay) relayLoop() {
140135
}
141136
} else {
142137
r.framesRelayed.Add(1)
143-
consecutiveWriteFailures = 0 // Reset on successful write
138+
consecutiveWriteFailures = 0
144139
}
145140
}
146141
}

0 commit comments

Comments
 (0)