feat: isolate media player in internal pipeline with clocksync bridge#484
Merged
feat: isolate media player in internal pipeline with clocksync bridge#484
Conversation
Refactor the media player block to use a separate internal GStreamer pipeline behind an appsrc/appsink bridge, following the same pattern as the WHIP ingest block. This fixes three problems: 1. No playback pacing: add clocksync(sync=true) so buffers flow at the correct rate when feeding a video compositor or live sink. 2. EOS propagation: EOS is now caught in the internal pipeline and triggers next-file advancement instead of stopping the main pipeline. 3. Seek disruption: seek operates only on the internal pipeline; downstream sees continuous timestamps via a computed offset. Architecture: Internal pipeline: uridecodebin/urisourcebin -> clocksync -> appsink Main pipeline: appsrc -> queue -> identity (video_out/audio_out) Bridge: appsink callback -> push_sample with ts offset The timestamp offset (shared AtomicI64 between audio/video) is computed once from the first buffer and reset on seek, file switch, and resume to prevent accumulated drift. Also split mediaplayer.rs (1427 lines) into sub-modules following the vision_mixer pattern: mod.rs, state.rs, builder.rs, bridge.rs, definition.rs. Frontend changes: - Enable interactive seek bar in compact node view (click/drag) - Enable seek slider in property inspector panel (show_full) - Wire show_full into the properties panel for media_player blocks - Add new "sync" block property (default true) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The interactive test harness (mediaplayer-test binary) and its docs are outdated after the internal pipeline refactoring. The seek warning in the API endpoint is no longer relevant since seek now operates on the isolated internal pipeline without affecting downstream. Removed: - backend/src/bin/mediaplayer_test/ (1362 lines) - docs/MEDIAPLAYER_TEST_HARNESS.md - Cargo.toml [[bin]] entry for mediaplayer-test - Seek warning log in api/mediaplayer.rs - Outdated unit tests in mediaplayer/mod.rs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Re-add the unit tests that were removed with the test harness binary. These test pure logic (URI normalization, registry, state, definition) and remain valid after the internal pipeline refactoring. Introduced a test_state() helper to reduce boilerplate in test setup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ity fixes Add seek throttle (150ms) to avoid flooding the API during progress bar drags. Add ±15s jump buttons flanking the seek bar in compact view and in the full property inspector. Add stop button to both views. Quality fixes: - Fix registry leak: unregister media players from MEDIA_PLAYER_REGISTRY when flows stop - Implement proper stop action (pause + seek to 0) instead of just pause - Replace raw state strings with PlayerState enum throughout the stack - Fix playlist/index race condition by combining both under a single RwLock<Playlist> - Add seek bounds validation in the API endpoint - Add switching_file flag to prevent spurious EOS during file switches - Add debug logging for silent position query failures - Sanitize error messages to not expose GStreamer internals to API clients - Allow duplicate files in playlists - Don't restart playback when saving a playlist on an already playing player - Remove block_id label from property inspector Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Compact: remove stop button (too cramped), keep transport on row 1 and jump buttons flanking the progress bar on row 2. Right pane: transport buttons on row 1, jump buttons flanking the seek slider on row 2. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The bridge's appsink callback called .unwrap() on sample caps, which would crash the process if a sample arrived without caps during state transitions. Now handles the None case gracefully. set_playlist now resets current_index to 0 when the new playlist is shorter than the current position, preventing out-of-bounds access while the player is running. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
clocksyncpacing, bridged to the main pipeline via appsink→appsrc. This isolates downstream from seeks, file switches, and EOS.mediaplayer.rs) into focused sub-modules:mod.rs,state.rs,bridge.rs,builder.rs,definition.rs.set_playlistnow clampscurrent_indexwhen the new playlist is shorter.Test plan
cargo checkpassescargo clippy— no warningscargo test --test openapi_test— snapshot matchescargo test --lib mediaplayer— 10/10 pass (including newtest_set_playlist_clamps_index)🤖 Generated with Claude Code