Skip to content

feat(echo): add a robot mode that transforms the audio - #58

Merged
rickstaa merged 1 commit into
mainfrom
rs/echo-audio
Aug 12, 2026
Merged

feat(echo): add a robot mode that transforms the audio#58
rickstaa merged 1 commit into
mainfrom
rs/echo-audio

Conversation

@rickstaa

@rickstaa rickstaa commented Aug 8, 2026

Copy link
Copy Markdown
Member

echo dropped audio in three independent places, so trickle looked video-only: the client decoded video=0 (client.py:130), the runner returned None for 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.

robot ring-modulates the audio and leaves the video untouched, so it slots in next to blur as 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 echo has gray/invert/blur instead 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| <= 1 cannot clip.

Only robot declares an audio track. MediaPublish opens the container once every declared track has a first frame, or after track_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, from ffmpeg -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, and robot refuses an input with no audio rather than hanging. Every other mode behaves exactly as before.

Verified

Ran both paths against the live stack.

Run Output streams
--mode robot on a clip with a 440 Hz tone h264 + opus
--mode blur on a silent clip h264 only, no stall

The 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 ± 220 sum 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.

Copilot AI lite review requested due to automatic review settings August 8, 2026 05:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 robot mode: passes video through unchanged and ring-modulates audio in the runner.
  • Updates client publishing to optionally include audio frames/tracks only when --mode robot is selected.
  • Documents the new mode and adds the numpy dependency 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 thread echo/runner.py
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 thread echo/client.py
Comment thread echo/runner.py
Copilot AI review requested due to automatic review settings August 9, 2026 05:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 robot is the only mode that publishes an audio track, but the runner actually publishes audio whenever the client sets audio: true (independent of mode). 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 of mode. This contradicts the stated behavior/docs that only robot should publish an audio track, and it also allows accidentally stalling the stream by requesting audio in other modes (or by running robot without requesting audio). Consider validating that audio is only allowed (and required) with mode == "robot", then derive tracks from 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_audio is 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

Copilot AI review requested due to automatic review settings August 12, 2026 12:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 12, 2026 12:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 12, 2026 12:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 12, 2026 13:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 12, 2026 13:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 12, 2026 13:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 12, 2026 13:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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>
Copilot AI review requested due to automatic review settings August 12, 2026 13:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@rickstaa
rickstaa merged commit 96c36b5 into main Aug 12, 2026
5 checks passed
@rickstaa
rickstaa deleted the rs/echo-audio branch August 12, 2026 14:53
rickstaa added a commit that referenced this pull request Aug 12, 2026
echo dropped audio in three places, so trickle looked video-only: the
client decoded video=0, the runner returned None for non-video frames,
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. Ring modulation rather than a pitch shift
because it preserves the sample count, so audio stays in sync without
resampling. The output track is pinned to 48 kHz since opus rejects
44.1, which every consumer source produces.

Only robot publishes an audio track: the container opens once every
declared track has a first frame, so declaring audio for a silent input
would stall the video.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants