Skip to content

feat(presenter): subtitle/audio delay, mute, and volume query across the full stack - #53

Closed
Shinokawa wants to merge 2 commits into
mainfrom
feat/delay-mute-volume-controls
Closed

feat(presenter): subtitle/audio delay, mute, and volume query across the full stack#53
Shinokawa wants to merge 2 commits into
mainfrom
feat/delay-mute-volume-controls

Conversation

@Shinokawa

@Shinokawa Shinokawa commented Jul 26, 2026

Copy link
Copy Markdown
Member

Summary

Adds four playback-control APIs that the engine previously lacked, wired end to end: Rust presenter → C ABI → platform plugins (macOS/iOS/Windows/Android) → Dart.

C ABI Semantics Range
erika_presenter_set_subtitle_delay(handle, double seconds) mpv sub-delay: positive delays subtitle display clamped to ±60 s
erika_presenter_set_audio_delay(handle, double seconds) mpv audio-delay: positive delays audio relative to video clamped to ±10 s
erika_presenter_set_muted(handle, bool) / erika_presenter_muted(handle, bool*) independent mute flag; does not clobber the stored volume
erika_presenter_get_volume(handle, double*) reads the user volume; reports the stored level while muted

Without these, hosts could not correct A/V or subtitle sync (external audio tracks, Bluetooth latency, badly muxed sources) and had to emulate mute with set_volume(0), losing the user's level and the muted/zero-volume distinction that OS media controls expect.

Implementation

Subtitle delay. The pts handed to subtitle overlay composition is shifted at all four append_to_overlay call sites (render tick, clock update, overlay refresh, frame capture), via a pure helper shifted_subtitle_pts. The danmaku timeline is deliberately not shifted — it has its own per-track and global offset mechanism. Changing the delay refreshes the current overlay immediately, following the set_subtitle_scale pattern. Negative delay is bounded by decode lookahead; documented on the setter.

Audio delay. Frame pts is shifted in push_audio before the ring-buffer hand-off. Because the audio-master clock disciplines itself to these pts values, video scheduling, subtitles, and danmaku follow automatically; no backend changes are required. A delay change takes effect from the next pushed frame and does not reset the output device.

Mute. muted and saved_volume are independent presenter state: muting drives the backend volume to zero without touching saved_volume; set_volume while muted updates the stored level without unmuting; volume() always reports the stored level. This contract is fixed in C ABI tests.

Platform wiring follows the existing set_volume / setSubtitleScale template at every layer: header declarations with cfg-gated stubs for unsupported targets, JNI dispatch entries, macOS and iOS Swift bindings (function additions only — no mirrored C struct is modified), the Windows C++ plugin, the Kotlin method registry (setSubtitleDelay also added to RENDER_REQUEST_METHODS since it must trigger a re-render), and Dart methods taking Duration/bool.

Testing

  • Rust: pts-shift helper and mute state-machine unit tests — cargo test -p erika --lib, 317 passed
  • C ABI: volume/mute round-trip, delay setter null-handle and clamp behavior; new exported functions carry # Safety docs — cargo test -p erika_capi --lib, 24 passed
  • Dart: four method-channel forwarding tests — flutter test, 37 passed
  • Both Swift files pass swiftc -parse; cargo clippy reports no new warnings

Notes for reviewers

The Android JNI dispatch entries mirror the existing setVolume / getUpscalerStatus patterns but were not compiled locally (macOS host without the Android toolchain); the android.yml workflow covers that target.

🤖 Generated with Claude Code

…oss the full stack

Four playback-control APIs that danmaku players need, wired from the
Rust presenter through the C ABI and every platform plugin to Dart:

- erika_presenter_set_subtitle_delay (±60s, mpv sub-delay semantics):
  shifts the pts handed to subtitle overlay composition; the danmaku
  timeline keeps its own independent offsets
- erika_presenter_set_audio_delay (±10s, mpv audio-delay semantics):
  shifts audio frame pts before the ring buffer push; the audio-master
  clock, subtitles, and danmaku follow automatically
- erika_presenter_set_muted / erika_presenter_muted: independent flag
  that does not clobber the stored volume, so getters keep reporting
  the user's level while muted
- erika_presenter_get_volume

Platform wiring follows the existing set_volume/setSubtitleScale
template: C header declarations, cfg-gated stubs for unsupported
targets, JNI dispatch, macOS/iOS Swift (function additions only — no
mirrored C struct changes), Windows C++, Kotlin method registry
(setSubtitleDelay also triggers a render request), and Dart methods
taking Duration/bool.

Tests: pts-shift and mute state-machine unit tests, C ABI round-trip
and null-pointer tests, and 4 Dart method-channel forwarding tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a1727254be

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +2022 to +2025
frame.frame.pts = frame
.frame
.pts
.map(|pts| shifted_audio_pts(pts, self.audio_delay));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Delay audio samples instead of retagging their clock

For any nonzero audio delay, changing only frame.pts does not delay or advance the samples: AudioRingBuffer queues and reads sample data immediately in FIFO order and uses PTS only to produce clock_snapshot(). That shifted snapshot is then sent to Player::update_audio_clock, so a positive delay drives the video/master clock forward rather than holding audio back, while a negative shift may be rejected as a stale clock. The implementation needs to schedule/insert/drop audio relative to an unshifted playback clock rather than altering the clock reported for the samples.

Useful? React with 👍 / 👎.

Comment on lines +1346 to +1349
self.subtitles.append_to_overlay(
shifted_subtitle_pts(pts, self.subtitle_delay),
&mut overlay,
subtitle_style,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Apply subtitle delay before subtitle delivery and retention

Shifting only the overlay lookup time cannot implement the documented delay semantics. PlaybackEngine::tick_subtitle does not deliver a cue until its unshifted start time, so negative delays can never display it early; for positive delays, SubtitleFrameState::push prunes preceding finite cues using the next cue's original start, meaning adjacent text/bitmap cues are commonly discarded before their delayed display window. Subtitle delivery and retention must account for the configured delay, not just the final lookup.

Useful? React with 👍 / 👎.

Cues arrive on the unshifted timeline but render on the shifted one, and
retention ran in the arrival domain: an incoming cue retired everything
that ended at or before its own raw start time. For adjacent cues that is
the cue immediately before it, which a positive delay still owes `delay`
seconds of screen time, so the text vanished early and left a gap.

Mirror the delay onto SubtitleFrameState so retention and display agree.
It defaults to zero, so nothing moves until a delay is actually set.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copy link
Copy Markdown
Member Author

This PR has been split into three focused drafts:

Each replacement branch is based on the latest main, contains one feature area, and has passed Rust/C ABI and Flutter tests. Closing #53 to avoid duplicate review and merge paths.

@Shinokawa Shinokawa closed this Jul 29, 2026
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.

1 participant