-
Notifications
You must be signed in to change notification settings - Fork 243
Add video_track_override_path to Agent to play video from local files
#249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds an optional local video-track override to Agent (runtime-settable path) and a new VideoFileTrack that reads, filters, loops, and exposes async recv() frames; CLI gains a --video-track-override option to set the path during agent startup. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
agents-core/vision_agents/core/agents/agents.py(5 hunks)agents-core/vision_agents/core/utils/video_track.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Never adjust sys.path in Python code
Never writeexcept Exception as e- use specific exception handling
Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access in Python
Docstrings should follow the Google style guide for docstrings
Files:
agents-core/vision_agents/core/utils/video_track.pyagents-core/vision_agents/core/agents/agents.py
🧬 Code graph analysis (1)
agents-core/vision_agents/core/agents/agents.py (4)
agents-core/vision_agents/core/utils/video_track.py (1)
VideoFileTrack(100-212)plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (1)
add_track_subscriber(427-430)tests/test_agent_tracks.py (1)
add_track_subscriber(96-99)agents-core/vision_agents/core/edge/edge_transport.py (1)
add_track_subscriber(58-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Mypy
🔇 Additional comments (6)
agents-core/vision_agents/core/utils/video_track.py (3)
1-16: Imports are well-organized for the new functionality.The added imports (
time,ThreadPoolExecutor,Path,Optional,cast, and av submodules) are all necessary for theVideoFileTrackimplementation.
126-156: Filter graph configuration is solid.The defensive handling of
sample_aspect_ratiobeingNoneand the clear graph construction (buffer → fps → buffersink) are well done. Keeping references to prevent GC is a good practice with PyAV.
211-212: Clean__repr__implementation.Useful for debugging and logging purposes.
agents-core/vision_agents/core/agents/agents.py (3)
9-9: Imports are appropriate for the new video override feature.Also applies to: 55-55
130-130: New optional parameter is well-placed.The
video_track_override_pathparameter has sensible typing (Optional[str | Path]) and defaults toNone, making it backward compatible.
1028-1038: Override logic is cleanly implemented.The approach of overriding all incoming video tracks with the local file maintains proper lifecycle semantics—the override track activates when a participant joins and deactivates when they leave, just as real tracks would. The comments clearly explain this design decision.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
agents-core/vision_agents/core/agents/agents.py (1)
1353-1363: Simplify the lambda and enhance the docstring.The lambda wrapper is unnecessary, and the docstring could be more complete following Google style guidelines.
Apply this diff:
async def _get_video_track_override(self) -> VideoFileTrack: """ - Create a video track override in async way if the path is set. + Create a VideoFileTrack from the override path asynchronously. + + This method offloads the potentially blocking VideoFileTrack initialization + (file opening, codec setup) to a thread pool to avoid blocking the event loop. - Returns: `VideoFileTrack` + Returns: + VideoFileTrack: A video track that plays the local video file. + + Raises: + ValueError: If video_track_override_path is not set. + RuntimeError: If VideoFileTrack initialization fails. """ if not self._video_track_override_path: raise ValueError("video_track_override_path is not set") - return await asyncio.to_thread( - lambda p: VideoFileTrack(p), self._video_track_override_path - ) + return await asyncio.to_thread(VideoFileTrack, self._video_track_override_path)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
agents-core/vision_agents/core/agents/agents.py(8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Never adjust sys.path in Python code
Never writeexcept Exception as e- use specific exception handling
Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access in Python
Docstrings should follow the Google style guide for docstrings
Files:
agents-core/vision_agents/core/agents/agents.py
🧬 Code graph analysis (1)
agents-core/vision_agents/core/agents/agents.py (2)
agents-core/vision_agents/core/utils/video_track.py (3)
VideoFileTrack(100-223)stop(91-93)stop(216-220)agents-core/vision_agents/core/edge/edge_transport.py (1)
add_track_subscriber(58-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit / Mypy
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Mypy
🔇 Additional comments (3)
agents-core/vision_agents/core/agents/agents.py (3)
9-9: LGTM: Imports are appropriate for the video override feature.The addition of
PathandVideoFileTrackimports supports the new local video playback functionality.Also applies to: 55-55
218-227: Logger usage is now consistent.The code correctly uses
self.logger.warninginstead of the module-level logger, addressing the previous review feedback.
131-131: The type annotationstr | Pathis appropriate for this project. Theagents-corepackage specifiesrequires-python = ">=3.10"in itspyproject.toml, and the union syntax was introduced in Python 3.10. No changes are required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
agents-core/vision_agents/core/agents/agents.py (3)
879-887:set_video_track_override_pathsilently no-ops whenpublish_videois false
set_video_track_override_pathreturns early whennot self.publish_video. That means:
- Programmatic callers (and the CLI) can successfully pass a path, but the override will be ignored for agents that don’t publish an outbound video track (e.g., agents that only consume video via processors or a Video LLM).
Given the documented behavior “This track will play instead of any incoming video track”, this gate on
publish_videofeels surprising: the override affects incoming track handling (_on_track_added), which is orthogonal to whether the agent publishes video.Consider either:
- Dropping the
not self.publish_videocondition, or- Replacing it with a predicate that reflects actual video-input needs (e.g.,
_needs_audio_or_video_input()or aneeds_video-style check), plus a log warning if the override is unlikely to be used.Example simplification:
- def set_video_track_override_path(self, path: str): - if not path or not self.publish_video: - return + def set_video_track_override_path(self, path: str): + if not path: + returnThis keeps the behavior predictable: if a non-empty path is provided, the override is armed.
980-987: Good: video track resources are now cleaned up on removalStopping both the
VideoForwarderand the underlying track when a video track is removed directly addresses the prior leak risk for per-participant tracks, including the newVideoFileTrackoverrides. This aligns the lifecycle with the “starts when user joins, stops when user leaves” requirement.If you want extra robustness, you might optionally mirror
_stop’s defensive pattern and wrap eachforwarder.stop()/track.stop()call in a smalltry/exceptto avoid a single failure blowing up the background task, but that’s not strictly required.
1355-1365: Minor cleanups for_get_video_track_overrideThe async helper is reasonable, but a couple of small tweaks could simplify it:
- The extra lambda is unnecessary;
asyncio.to_threadcan takeVideoFileTrackdirectly.- Given
_video_track_override_pathis annotated asstr | Path, you might want to reflect that in the setter’s type as well for consistency.For example:
- async def _get_video_track_override(self) -> VideoFileTrack: + async def _get_video_track_override(self) -> VideoFileTrack: @@ - if not self._video_track_override_path: + if not self._video_track_override_path: raise ValueError("video_track_override_path is not set") - return await asyncio.to_thread( - lambda p: VideoFileTrack(p), self._video_track_override_path - ) + return await asyncio.to_thread( + VideoFileTrack, self._video_track_override_path + )And optionally:
- def set_video_track_override_path(self, path: str): + def set_video_track_override_path(self, path: str | Path):These are purely cosmetic/maintainability improvements.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
agents-core/vision_agents/core/agents/agents.py(8 hunks)agents-core/vision_agents/core/cli/cli_runner.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Never adjust sys.path in Python code
Never writeexcept Exception as e- use specific exception handling
Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access in Python
Docstrings should follow the Google style guide for docstrings
Files:
agents-core/vision_agents/core/cli/cli_runner.pyagents-core/vision_agents/core/agents/agents.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: dangusev
Repo: GetStream/Vision-Agents PR: 249
File: agents-core/vision_agents/core/agents/agents.py:1032-1042
Timestamp: 2025-12-10T19:35:30.572Z
Learning: In `agents-core/vision_agents/core/agents/agents.py`, when using `video_track_override_path`, creating a new `VideoFileTrack` for each participant (each call to `_on_track_added`) is intentional to maintain proper track lifecycle semantics tied to each participant.
📚 Learning: 2025-12-10T19:35:30.572Z
Learnt from: dangusev
Repo: GetStream/Vision-Agents PR: 249
File: agents-core/vision_agents/core/agents/agents.py:1032-1042
Timestamp: 2025-12-10T19:35:30.572Z
Learning: In `agents-core/vision_agents/core/agents/agents.py`, when using `video_track_override_path`, creating a new `VideoFileTrack` for each participant (each call to `_on_track_added`) is intentional to maintain proper track lifecycle semantics tied to each participant.
Applied to files:
agents-core/vision_agents/core/cli/cli_runner.pyagents-core/vision_agents/core/agents/agents.py
🧬 Code graph analysis (1)
agents-core/vision_agents/core/cli/cli_runner.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
set_video_track_override_path(879-887)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit / Mypy
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Mypy
🔇 Additional comments (3)
agents-core/vision_agents/core/cli/cli_runner.py (1)
70-76: CLI wiring for--video-track-overridelooks solidThe new option type/validation and the way the resolved path is threaded into
run_agentand then intoagent.set_video_track_override_path(...)are coherent and match the intended “local file for debugging” behavior. No issues from the CLI side.Also applies to: 83-84, 106-108
agents-core/vision_agents/core/agents/agents.py (2)
210-220: Agent state for video tracks and override is clear and cohesiveThe initialization of
_active_video_tracks,_video_forwarders, and_video_track_override_pathis straightforward and keeps all video-related state localized in one place. This makes the later override logic in_on_track_added/_on_track_removedeasier to reason about.
1034-1061: Override behavior in_on_track_addedmatches the specThe new
_on_track_addedlogic:
- Short-circuits subscription when
_video_track_override_pathis set.- Creates a fresh
VideoFileTrackvia_get_video_track_override()for each added track (per-participant override, as intended).- Still preserves the original
track_id,track_type, andparticipantmetadata inTrackInfo, so priority and processor routing continue to work as before.This does exactly what the PR describes: replace incoming video with a looping local source while keeping normal track lifecycle semantics.
Adds new param
video_track_override_pathtoAgentto play video from files.It should greatly simplify testing and debugging of Agent's video processing.
How to use
or as a CLI param
How it works
When
video_track_override_pathis provided, Agent will use this track instead of any incoming video during the call.This way, we keep the track lifecycle intact (e.g., the track starts when the user joins and stops when the user leaves).
The track is played in a loop.
The video is always resampled to 30fps using ffmpeg filters.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.