feat(echo): add a robot mode that transforms the audio - #58
Open
rickstaa wants to merge 2 commits into
Open
Conversation
echo dropped audio in three places, so trickle looked video-only: the client decoded video=0, the runner returned None for anything that was not a video frame, and only a video track was published. A stream with sound came back silent with no explanation. robot ring-modulates the audio and leaves the video alone, which makes the round trip audible: an unchanged passthrough proves nothing, since you cannot tell it from playing the local file. Ring modulation rather than a pitch shift because it preserves the sample count, so audio and video stay in sync without resampling. Only robot publishes an audio track. MediaPublish opens the container once every declared track has a first frame, or after a five second deadline, so declaring audio for a silent input would stall the video: the README's own test clip has no audio at all. Verified against a 440 Hz tone: the echoed audio comes back at 220 and 660 Hz, the sum and difference tones, with the original gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds an audio-capable “robot” mode to the echo trickle example so audio is no longer silently dropped end-to-end, while keeping existing video-only modes unchanged.
Changes:
- Introduces
robotmode: passes video through unchanged and ring-modulates audio in the runner. - Updates client publishing to optionally include audio frames/tracks only when
--mode robotis selected. - Documents the new mode and adds the
numpydependency required for the audio transform.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| echo/runner.py | Adds robot mode, audio frame transform, and conditional audio track declaration. |
| echo/client.py | Publishes audio frames/tracks only for robot and updates decode loop to include audio when enabled. |
| echo/README.md | Documents robot mode behavior and audio-track-only-for-robot rationale. |
| echo/pyproject.toml | Adds numpy dependency for ring modulation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+133
to
+149
| def _robot_audio(frame: av.AudioFrame) -> av.AudioFrame: | ||
| # sample[i] *= sin(2*pi*ROBOT_HZ*t[i]). The carrier phase comes from the frame's | ||
| # own timestamp, so it stays continuous across frames (no clicks) without keeping | ||
| # state, and |carrier| <= 1 means it cannot clip. | ||
| samples = frame.to_ndarray() | ||
| t0 = float(frame.pts * frame.time_base) if frame.pts is not None else 0.0 | ||
| t = t0 + np.arange(samples.shape[-1], dtype=np.float32) / frame.sample_rate | ||
| carrier = np.sin(2.0 * np.pi * ROBOT_HZ * t).astype(np.float32) | ||
| out = av.AudioFrame.from_ndarray( | ||
| (samples.astype(np.float32) * carrier).astype(samples.dtype), | ||
| format=frame.format.name, | ||
| layout=frame.layout.name, | ||
| ) | ||
| out.sample_rate = frame.sample_rate | ||
| out.pts = frame.pts | ||
| out.time_base = frame.time_base | ||
| return out |
Comment on lines
+151
to
+155
| frames = input_.decode() if send_audio else input_.decode(video=0) | ||
| for frame in frames: | ||
| if not isinstance(frame, av.VideoFrame): | ||
| await publisher.write_frame(frame) | ||
| continue |
Comment on lines
+213
to
+215
| tracks: list[VideoOutputConfig | AudioOutputConfig] = [VideoOutputConfig()] | ||
| if bool(payload.get("audio")): | ||
| tracks.append(AudioOutputConfig()) |
…sion echo inferred teardown from its media tasks finishing. register_runner takes on_session_release, which the orchestrator pushes over the runner's o2r channel, so the app can act on the authoritative signal instead. It matters most on the paid path: a debit the client cannot cover releases the session while media may still be arriving, and without this the app keeps serving a session it is no longer paid for. To be clear about what this does not fix: offchain, a client that dies never causes a release at all. ReleaseSession is only reached from the payment paths, an explicit stop_runner_session, and the single-shot proxy's defer, and there is no session TTL, so the session stays held and the runner stays at capacity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (3)
echo/README.md:77
- This bullet says
robotis the only mode that publishes an audio track, but the runner actually publishes audio whenever the client setsaudio: true(independent ofmode). Either tighten the runner behavior to match, or clarify here that this is specifically how the bundled client behaves.
- `robot` ring-modulates the audio and leaves the video alone, so it is the one to reach for when the question is whether trickle carries sound too. It is also the only mode that publishes an audio track, since a declared track that never gets a frame stalls the stream: the other modes are video-only, and `robot` refuses an input with no audio.
echo/runner.py:229
- The runner declares an audio output track whenever the client sends
audio: true, regardless ofmode. This contradicts the stated behavior/docs that onlyrobotshould publish an audio track, and it also allows accidentally stalling the stream by requesting audio in other modes (or by runningrobotwithout requesting audio). Consider validating thataudiois only allowed (and required) withmode == "robot", then derivetracksfrom the validated flag.
# Tracks are declared upfront and the container waits for a first frame on each,
# so only declare audio when the client says it is sending some.
tracks: list[VideoOutputConfig | AudioOutputConfig] = [VideoOutputConfig()]
if bool(payload.get("audio")):
tracks.append(AudioOutputConfig())
echo/client.py:155
- When
send_audiois true,input_.decode()decodes all streams (including additional video/audio tracks, subtitles, data), which can lead to publishing unexpected frames. If the intent is “first video + first audio”, it’s safer to explicitly select streams (e.g.video=0, audio=0) and ignore any other frame types.
# decode() yields both streams interleaved; without audio, stay on the
# video stream alone. Pacing and the blur sweep run off video frames only.
frames = input_.decode() if send_audio else input_.decode(video=0)
for frame in frames:
if not isinstance(frame, av.VideoFrame):
await publisher.write_frame(frame)
continue
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
echodropped audio in three independent places, so trickle looked video-only: the client decodedvideo=0(client.py:130), the runner returnedNonefor anything that was not a video frame (runner.py:128), and only a video track was ever published. A stream with sound came back silent, with nothing explaining why.robotring-modulates the audio and leaves the video untouched, so it slots in next tobluras one more--mode.Why an effect rather than plain passthrough. Unchanged audio proves nothing: you cannot tell a successful round trip from your player reading the local file. An audible transform is the evidence, which is the same reason
echohasgray/invert/blurinstead of only echoing video.Why ring modulation rather than a pitch shift. Multiplying each sample by a sine carrier preserves the sample count, so audio and video stay in sync with no resampling. A naive pitch shift changes duration and drifts; a proper one needs a phase vocoder or librosa, which is a heavy dependency for the repo's smallest trickle example. The carrier phase comes from each frame's own timestamp, so it stays continuous across frames without tracking state, and
|carrier| <= 1cannot clip.Only
robotdeclares an audio track.MediaPublishopens the container once every declared track has a first frame, or aftertrack_wait_timeout_s(5 s), dropping late tracks before container initialization (media_publish.py:265,605-617). Declaring audio unconditionally would therefore stall every silent input by five seconds and drop queued video frames along the way — and the README's own sample clip, fromffmpeg -f lavfi -i testsrc, has no audio stream at all. So the client sends"audio": mode == "robot"on/echo, the runner declares the track only when asked, androbotrefuses an input with no audio rather than hanging. Every other mode behaves exactly as before.Verified
Ran both paths against the live stack.
--mode roboton a clip with a 440 Hz toneh264+opus--mode bluron a silent cliph264only, no stallThe effect checks out mathematically. Input is a pure 440 Hz sine; the echoed audio's dominant frequencies are 220 Hz and 660 Hz with 440 gone, which is exactly the
440 ± 220sum and difference pair ring modulation produces.Note for whoever runs this next: this machine's ffmpeg 7.0.2 static build segfaults reading the echoed
.ts, but it does so on the video-only baseline too, so it is unrelated to this change. PyAV reads both outputs fine.🤖 Generated with Claude Code