feat: audiocapture support for real iOS devices with device kit #157
Conversation
WalkthroughAdds audio capture capability to iOS devices via a new CLI subcommand and HTTP/WebSocket server endpoints. Introduces Opus audio streaming over device-kit port forwarding with OGG encoding support. Includes request/response types, device interface extensions, and WebSocket server infrastructure. Android and iOS simulator devices reject audio capture operations. Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@utils/ogg_opus.go`:
- Around line 42-45: The WritePacket method currently increments o.granule
before calling writePage which can leave o.granule advanced if writePage fails;
change the logic in OggOpusWriter.WritePacket to compute newGranule := o.granule
+ opusFrameSize, call o.writePage(0x00, newGranule, packet) first, and only on a
nil error assign o.granule = newGranule so state only advances after a
successful write.
- Around line 70-104: The writePage function currently increments o.seq before
performing the actual write, causing the page sequence to advance even on write
failures; change the logic in writePage so that o.seq is incremented only after
o.w.Write(page) succeeds (i.e., check err from o.w.Write and only then do
o.seq++), ensuring the sequence number reflects only successfully written pages.
- Around line 117-137: The OpusFrameParser.Write currently reads packetLen with
binary.BigEndian and allocates without bounds; add a sanity bounds check in
OpusFrameParser.Write to ensure packetLen is non-negative and below a reasonable
max (e.g., a configurable cap like 10MB) before allocating to prevent OOM and
return an error if out of range, and handle zero/negative as now; also reconcile
endianness by switching the length read to binary.LittleEndian (or change
OggOpusWriter to BigEndian) so OpusFrameParser.Write and OggOpusWriter use the
same endianness — verify device-kit format and apply the matching endianness in
OpusFrameParser.Write and/or OggOpusWriter methods.
🧹 Nitpick comments (6)
utils/ogg_opus.go (2)
11-17: Confirm Opus framing constants align with the upstream encoder.Fixed
opusFrameSize=960andopusPreskip=312assume 20 ms frames and a specific preskip; if device‑kit outputs variable durations or a different preskip, granule timing can drift. Please confirm against the device‑kit/encoder behavior; if it differs, consider making these values configurable or deriving granule increments from packet TOC.
47-56: Verify OpusHead channel count/preskip matches source audio.
head[9]=1hard‑codes mono and preskip is fixed. Please confirm the actual channel count and preskip from device‑kit; if they can vary, consider threading these as options.cli/audiocapture.go (1)
17-35: Simplify error wrapping.The pattern
fmt.Errorf("%s", response.Error)is redundant sinceresponse.Erroris already a string. Consider usingerrors.New(response.Error)or just returningfmt.Errorf(response.Error)directly. However, if this pattern is used elsewhere in the codebase for consistency, it can remain.server/server.go (1)
993-1004: Uselog.Printfinstead offmt.Printlnfor error logging.For consistency with the rest of the file (e.g., lines 195, 205, 230-231), use
log.Printffor error output rather thanfmt.Println.♻️ Suggested fix
if parser != nil { if err := parser.Write(data); err != nil { - fmt.Println("Error writing Ogg Opus data:", err) + log.Printf("Error writing Ogg Opus data: %v", err) return false } } else { _, writeErr := w.Write(data) if writeErr != nil { - fmt.Println("Error writing data:", writeErr) + log.Printf("Error writing data: %v", writeErr) return false } }server/websocket.go (1)
260-264: Consider setting write deadline beforeWriteJSON.The ping loop sets a write deadline before writing, but
sendJSONdoesn't. For consistency and to prevent hanging on slow clients, consider setting the deadline here as well.♻️ Suggested fix
func (wsc *wsConnection) sendJSON(v interface{}) error { wsc.writeMu.Lock() defer wsc.writeMu.Unlock() + if err := wsc.conn.SetWriteDeadline(time.Now().Add(wsWriteWait)); err != nil { + return err + } return wsc.conn.WriteJSON(v) }devices/ios.go (1)
779-786: Remove redundant device type check.
IOSDevice.Platform()always returns"ios"andIOSDevice.DeviceType()always returns"real"(see lines 66-72). This check will never fail for anIOSDeviceinstance. The format validation is appropriate, but the device type check is redundant here.♻️ Suggested fix
func (d IOSDevice) StartAudioCapture(config AudioCaptureConfig) error { if config.Format != "opus+rtp" && config.Format != "opus+ogg" { return fmt.Errorf("format must be 'opus+rtp' or 'opus+ogg' for audio capture") } - if d.Platform() != "ios" || d.DeviceType() != "real" { - return fmt.Errorf("audio capture is only supported on real iOS devices") - } - if config.OnProgress != nil { config.OnProgress("Starting port forwarding for Opus audio stream") }
| func (o *OggOpusWriter) WritePacket(packet []byte) error { | ||
| o.granule += opusFrameSize | ||
| return o.writePage(0x00, o.granule, packet) | ||
| } |
There was a problem hiding this comment.
Avoid advancing granule on failed writes.
granule is incremented before writePage; if writePage fails, state drifts. Consider updating only after a successful write.
🐛 Proposed fix
func (o *OggOpusWriter) WritePacket(packet []byte) error {
- o.granule += opusFrameSize
- return o.writePage(0x00, o.granule, packet)
+ nextGranule := o.granule + opusFrameSize
+ if err := o.writePage(0x00, nextGranule, packet); err != nil {
+ return err
+ }
+ o.granule = nextGranule
+ return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (o *OggOpusWriter) WritePacket(packet []byte) error { | |
| o.granule += opusFrameSize | |
| return o.writePage(0x00, o.granule, packet) | |
| } | |
| func (o *OggOpusWriter) WritePacket(packet []byte) error { | |
| nextGranule := o.granule + opusFrameSize | |
| if err := o.writePage(0x00, nextGranule, packet); err != nil { | |
| return err | |
| } | |
| o.granule = nextGranule | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In `@utils/ogg_opus.go` around lines 42 - 45, The WritePacket method currently
increments o.granule before calling writePage which can leave o.granule advanced
if writePage fails; change the logic in OggOpusWriter.WritePacket to compute
newGranule := o.granule + opusFrameSize, call o.writePage(0x00, newGranule,
packet) first, and only on a nil error assign o.granule = newGranule so state
only advances after a successful write.
| func (o *OggOpusWriter) writePage(headerType uint8, granule uint64, packet []byte) error { | ||
| segments := make([]byte, 0, 255) | ||
| remaining := len(packet) | ||
| for remaining > 0 { | ||
| seg := remaining | ||
| if seg > 255 { | ||
| seg = 255 | ||
| } | ||
| segments = append(segments, byte(seg)) | ||
| remaining -= seg | ||
| } | ||
| if len(segments) > 255 { | ||
| return fmt.Errorf("opus packet too large for single ogg page") | ||
| } | ||
|
|
||
| headerLen := 27 + len(segments) | ||
| page := make([]byte, headerLen+len(packet)) | ||
| copy(page[0:4], []byte("OggS")) | ||
| page[4] = 0 | ||
| page[5] = headerType | ||
| binary.LittleEndian.PutUint64(page[6:], granule) | ||
| binary.LittleEndian.PutUint32(page[14:], o.serial) | ||
| binary.LittleEndian.PutUint32(page[18:], o.seq) | ||
| binary.LittleEndian.PutUint32(page[22:], 0) | ||
| page[26] = byte(len(segments)) | ||
| copy(page[27:], segments) | ||
| copy(page[headerLen:], packet) | ||
|
|
||
| crc := oggCRC(page) | ||
| binary.LittleEndian.PutUint32(page[22:], crc) | ||
|
|
||
| o.seq++ | ||
| _, err := o.w.Write(page) | ||
| return err | ||
| } |
There was a problem hiding this comment.
Don’t increment the sequence number on failed writes.
o.seq++ runs before Write, so failures still advance the sequence. Consider incrementing only on success.
🐛 Proposed fix
- o.seq++
- _, err := o.w.Write(page)
- return err
+ _, err := o.w.Write(page)
+ if err != nil {
+ return err
+ }
+ o.seq++
+ return nil📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (o *OggOpusWriter) writePage(headerType uint8, granule uint64, packet []byte) error { | |
| segments := make([]byte, 0, 255) | |
| remaining := len(packet) | |
| for remaining > 0 { | |
| seg := remaining | |
| if seg > 255 { | |
| seg = 255 | |
| } | |
| segments = append(segments, byte(seg)) | |
| remaining -= seg | |
| } | |
| if len(segments) > 255 { | |
| return fmt.Errorf("opus packet too large for single ogg page") | |
| } | |
| headerLen := 27 + len(segments) | |
| page := make([]byte, headerLen+len(packet)) | |
| copy(page[0:4], []byte("OggS")) | |
| page[4] = 0 | |
| page[5] = headerType | |
| binary.LittleEndian.PutUint64(page[6:], granule) | |
| binary.LittleEndian.PutUint32(page[14:], o.serial) | |
| binary.LittleEndian.PutUint32(page[18:], o.seq) | |
| binary.LittleEndian.PutUint32(page[22:], 0) | |
| page[26] = byte(len(segments)) | |
| copy(page[27:], segments) | |
| copy(page[headerLen:], packet) | |
| crc := oggCRC(page) | |
| binary.LittleEndian.PutUint32(page[22:], crc) | |
| o.seq++ | |
| _, err := o.w.Write(page) | |
| return err | |
| } | |
| func (o *OggOpusWriter) writePage(headerType uint8, granule uint64, packet []byte) error { | |
| segments := make([]byte, 0, 255) | |
| remaining := len(packet) | |
| for remaining > 0 { | |
| seg := remaining | |
| if seg > 255 { | |
| seg = 255 | |
| } | |
| segments = append(segments, byte(seg)) | |
| remaining -= seg | |
| } | |
| if len(segments) > 255 { | |
| return fmt.Errorf("opus packet too large for single ogg page") | |
| } | |
| headerLen := 27 + len(segments) | |
| page := make([]byte, headerLen+len(packet)) | |
| copy(page[0:4], []byte("OggS")) | |
| page[4] = 0 | |
| page[5] = headerType | |
| binary.LittleEndian.PutUint64(page[6:], granule) | |
| binary.LittleEndian.PutUint32(page[14:], o.serial) | |
| binary.LittleEndian.PutUint32(page[18:], o.seq) | |
| binary.LittleEndian.PutUint32(page[22:], 0) | |
| page[26] = byte(len(segments)) | |
| copy(page[27:], segments) | |
| copy(page[headerLen:], packet) | |
| crc := oggCRC(page) | |
| binary.LittleEndian.PutUint32(page[22:], crc) | |
| _, err := o.w.Write(page) | |
| if err != nil { | |
| return err | |
| } | |
| o.seq++ | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In `@utils/ogg_opus.go` around lines 70 - 104, The writePage function currently
increments o.seq before performing the actual write, causing the page sequence
to advance even on write failures; change the logic in writePage so that o.seq
is incremented only after o.w.Write(page) succeeds (i.e., check err from
o.w.Write and only then do o.seq++), ensuring the sequence number reflects only
successfully written pages.
| func (p *OpusFrameParser) Write(data []byte) error { | ||
| p.buf = append(p.buf, data...) | ||
| for { | ||
| if len(p.buf) < 4 { | ||
| return nil | ||
| } | ||
| packetLen := int(binary.BigEndian.Uint32(p.buf[:4])) | ||
| if packetLen <= 0 { | ||
| p.buf = p.buf[4:] | ||
| continue | ||
| } | ||
| if len(p.buf) < 4+packetLen { | ||
| return nil | ||
| } | ||
| packet := make([]byte, packetLen) | ||
| copy(packet, p.buf[4:4+packetLen]) | ||
| p.buf = p.buf[4+packetLen:] | ||
| if err := p.onPacket(packet); err != nil { | ||
| return err | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the full OpusFrameParser structure and surrounding code
fd -t f "ogg_opus.go" | head -20Repository: mobile-next/mobilecli
Length of output: 82
🏁 Script executed:
# Read the entire file to understand the context
wc -l utils/ogg_opus.goRepository: mobile-next/mobilecli
Length of output: 86
🏁 Script executed:
# Get the full context around the Write function
cat -n utils/ogg_opus.goRepository: mobile-next/mobilecli
Length of output: 5359
🏁 Script executed:
# Search for any existing bounds checks or packet size limits in the codebase
rg -i "packet.*size|maxpacket|maxpayload" --type goRepository: mobile-next/mobilecli
Length of output: 47
🏁 Script executed:
# Search for device-kit references to understand endianness usage
rg -i "device.?kit|endian" --type go -B 2 -A 2Repository: mobile-next/mobilecli
Length of output: 12262
🏁 Script executed:
# Check if there are any tests for OpusFrameParser to understand expected behavior
rg -l "OpusFrameParser" --type goRepository: mobile-next/mobilecli
Length of output: 119
🏁 Script executed:
# Check usage of OpusFrameParser in other files
rg -B 3 -A 3 "OpusFrameParser|NewOpusFrameParser" --type goRepository: mobile-next/mobilecli
Length of output: 2044
🏁 Script executed:
# Check if there are any tests for opus parsing
fd -t f "*test*.go" | xargs rg -l "Opus|opus" 2>/dev/nullRepository: mobile-next/mobilecli
Length of output: 440
🏁 Script executed:
# Look at how WritePacket and audio data flows
rg -B 5 -A 5 "WritePacket" --type goRepository: mobile-next/mobilecli
Length of output: 2064
🏁 Script executed:
# Check where the parser data comes from - the device-kit audio stream
rg -B 10 -A 10 "StartAudioCapture|audioPort.*12006" --type goRepository: mobile-next/mobilecli
Length of output: 6214
🏁 Script executed:
# Look for any constants or specs about the packet format from device-kit
rg -i "packet.*format|little.*endian|big.*endian" --type go -B 2 -A 2Repository: mobile-next/mobilecli
Length of output: 1844
🏁 Script executed:
# Check if there's any documentation or comments about device-kit packet format
rg -i "device.?kit.*packet|opus.*format|packet.*format" --type go -B 2 -A 2Repository: mobile-next/mobilecli
Length of output: 1426
🏁 Script executed:
# Look for any GitHub or documentation links related to device-kit
rg -i "device.?kit|github.*device" --type md --type txt -B 2 -A 2Repository: mobile-next/mobilecli
Length of output: 639
🏁 Script executed:
# Check if there are any comments explaining the packet framing format
rg -B 5 -A 5 "OpusFrameParser|Write.*data" utils/ogg_opus.goRepository: mobile-next/mobilecli
Length of output: 452
🏁 Script executed:
# Look for device-kit-ios repository references or documentation
rg -i "devicekit-ios|github.*mobile-next" README.md 2>/dev/null || echo "README.md not found"Repository: mobile-next/mobilecli
Length of output: 932
🏁 Script executed:
# Check if there are any comments or specs about the packet framing from device
rg -B 10 "OpusFrameParser" server/server.go cli/audiocapture.goRepository: mobile-next/mobilecli
Length of output: 2263
🏁 Script executed:
# Search for any documentation about device-kit audio packet format
find . -name "*.md" -o -name "*.txt" | xargs grep -l "device.?kit\|audio.*packet" 2>/dev/null | head -5Repository: mobile-next/mobilecli
Length of output: 47
🏁 Script executed:
# Look at the broader context of how audio data flows from device to parser
rg -B 15 "OnData.*data.*bool" devices/ios.goRepository: mobile-next/mobilecli
Length of output: 47
Add bounds check on packet length and verify endianness with device-kit.
The packet length is read from untrusted network data with no bounds check before allocation at line 131, allowing unbounded memory allocation (potential OOM). Additionally, OpusFrameParser reads the length prefix using BigEndian (line 123) while OggOpusWriter writes all binary data using LittleEndian (lines 52–54, 64, 90–93). This endianness mismatch is incompatible and will cause parsing failures if device-kit output doesn't match the BigEndian expectation. Confirm the actual packet format from device-kit and correct if needed.
Suggested fix
func (p *OpusFrameParser) Write(data []byte) error {
p.buf = append(p.buf, data...)
for {
if len(p.buf) < 4 {
return nil
}
packetLen := int(binary.BigEndian.Uint32(p.buf[:4]))
if packetLen <= 0 {
p.buf = p.buf[4:]
continue
}
+ const maxPacketSize = 64 * 1024
+ if packetLen > maxPacketSize {
+ return fmt.Errorf("opus packet length %d exceeds max %d", packetLen, maxPacketSize)
+ }
if len(p.buf) < 4+packetLen {
return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (p *OpusFrameParser) Write(data []byte) error { | |
| p.buf = append(p.buf, data...) | |
| for { | |
| if len(p.buf) < 4 { | |
| return nil | |
| } | |
| packetLen := int(binary.BigEndian.Uint32(p.buf[:4])) | |
| if packetLen <= 0 { | |
| p.buf = p.buf[4:] | |
| continue | |
| } | |
| if len(p.buf) < 4+packetLen { | |
| return nil | |
| } | |
| packet := make([]byte, packetLen) | |
| copy(packet, p.buf[4:4+packetLen]) | |
| p.buf = p.buf[4+packetLen:] | |
| if err := p.onPacket(packet); err != nil { | |
| return err | |
| } | |
| } | |
| func (p *OpusFrameParser) Write(data []byte) error { | |
| p.buf = append(p.buf, data...) | |
| for { | |
| if len(p.buf) < 4 { | |
| return nil | |
| } | |
| packetLen := int(binary.BigEndian.Uint32(p.buf[:4])) | |
| if packetLen <= 0 { | |
| p.buf = p.buf[4:] | |
| continue | |
| } | |
| const maxPacketSize = 64 * 1024 | |
| if packetLen > maxPacketSize { | |
| return fmt.Errorf("opus packet length %d exceeds max %d", packetLen, maxPacketSize) | |
| } | |
| if len(p.buf) < 4+packetLen { | |
| return nil | |
| } | |
| packet := make([]byte, packetLen) | |
| copy(packet, p.buf[4:4+packetLen]) | |
| p.buf = p.buf[4+packetLen:] | |
| if err := p.onPacket(packet); err != nil { | |
| return err | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@utils/ogg_opus.go` around lines 117 - 137, The OpusFrameParser.Write
currently reads packetLen with binary.BigEndian and allocates without bounds;
add a sanity bounds check in OpusFrameParser.Write to ensure packetLen is
non-negative and below a reasonable max (e.g., a configurable cap like 10MB)
before allocating to prevent OOM and return an error if out of range, and handle
zero/negative as now; also reconcile endianness by switching the length read to
binary.LittleEndian (or change OggOpusWriter to BigEndian) so
OpusFrameParser.Write and OggOpusWriter use the same endianness — verify
device-kit format and apply the matching endianness in OpusFrameParser.Write
and/or OggOpusWriter methods.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.