Skip to content

Feature/screen capture input mode#38

Open
roshan-ku wants to merge 11 commits into
OpenVisualCloud:mainfrom
roshan-ku:feature/screen-capture-input-mode
Open

Feature/screen capture input mode#38
roshan-ku wants to merge 11 commits into
OpenVisualCloud:mainfrom
roshan-ku:feature/screen-capture-input-mode

Conversation

@roshan-ku

Copy link
Copy Markdown
Contributor

Description

Checklist

Code Quality

  • Code follows project style guidelines
  • No unnecessary debug logs or commented-out code
  • No hardcoded values / secrets

Testing

  • Unit test added/modified accordingly
  • Perform manual basic sanity testing at system level

Review Readiness

  • PR title and description are clear and meaningful
  • Story/Task IDs are linked

Documentation

  • README or relevant docs updated (if applicable)

Security

  • No sensitive data exposed (keys, passwords, tokens)
  • Input validation added where needed

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Documentation content changes
  • Testing
  • Other... Please describe:

…pport

- src/main.c: call avdevice_register_all() unconditionally instead of
  only when ENABLE_MTL_TX is not defined. x11grab screen-capture input
  needs avdevice registered regardless of which TX path (FFmpeg muxer
  vs native MTL) is compiled in. Previously, builds with
  -Denable_mtl_tx=true (the flag used for the MTL TX pipeline) never
  registered avdevice, causing 'x11grab input format not found'.
- meson.build: always link libavdevice regardless of enable_mtl_tx,
  since the x11grab decode path requires it independent of the TX path.
- config/tx_fullhd_virtual_display.json: new example config
  demonstrating screen capture from a virtual (Xvfb) display instead
  of a physical one.
- README.md: document how to set up a headless virtual display with
  a full GNOME desktop session (Xvfb + gnome-session) for screen
  capture testing/transmission on machines with no physical monitor.

Verified end-to-end: transmitted both a physical display (:0) and a
virtual Xvfb display (:99, running a real GNOME desktop) via x11grab
and MTL ST20P TX.
- Add note under FFmpeg build prerequisites that libx11-dev,
  libxcb1-dev, libxcb-shm0-dev, and libxcb-xfixes0-dev must be
  installed before building FFmpeg for x11grab (screen_capture mode)
  to be compiled in, plus a verification command
  (ffmpeg -devices | grep x11grab).
- Add xvfb/ubuntu-desktop as prerequisites for headless machines and
  link to the existing virtual-display setup section.
- Add the new headless screen-capture subsection to the table of
  contents.
- test_ffmpeg_decoder_mock.c: cover the previously-untested x11grab
  screen-capture branch of open_ffmpeg_decoder() via open_shared_ffmpeg()
  and load_video_source(). Verifies (a) x11grab is NOT resolvable before
  avdevice_register_all() runs, (b) it IS resolvable afterwards, and
  (c) both the shared-decoder and per-session paths fail gracefully
  (return -1, no crash) when pointed at a nonexistent display.
- test_main.c: add test_main_registers_avdevice_for_x11grab as a direct
  regression guard for the bug fixed in f74a727 — avdevice_register_all()
  must be called unconditionally in tx_app_real_main(), not gated behind
  an ENABLE_MTL_TX ifdef.
Comment thread config/tx_fullhd_screen_capture.json Outdated
Comment thread config/tx_fullhd_single_session.json
Comment thread README.md
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated

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 a new “screen capture” video input mode (via FFmpeg x11grab) so dvledtx can transmit a live X11 desktop instead of reading from a file, including config parsing/validation and unit tests.

Changes:

  • Add input_mode / screen_input to JSON config parsing + validation, and map into runtime app context.
  • Extend FFmpeg decoder to open x11grab sources when screen-capture mode is enabled; update session manager logic to share the decoder across multiple sessions.
  • Add tests and documentation + example configs for screen capture (including headless/virtual display guidance).

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/test_session_manager.c Adds unit tests covering shared decoder vs per-session source behavior for screen capture.
tests/test_config_reader.c Adds tests for parsing defaults and validation of input_mode / screen_input, plus config→app mapping.
src/util/config_reader.c Implements config defaults, parsing, validation, and mapping of screen-capture fields.
src/main.c Makes libavdevice registration unconditional to support device-based inputs (x11grab) across TX paths.
src/ffmpeg/ffmpeg_decoder.c Adds x11grab open path + wiring for screen capture through shared/per-session decode.
src/core/session_manager.c Updates shared-decoder selection and source-loading conditions to support screen capture.
README.md Documents input_mode: screen_capture, x11grab build requirements, and headless setup with Xvfb.
meson.build Links libavdevice unconditionally since it’s now needed for screen capture (and muxer path).
meson_options.txt Changes the default of enable_mtl_tx (see review comments).
include/util/config_reader.h Extends config struct with input_mode and screen_input.
include/app_context.h Extends app context with screen_input and use_screen_capture.
config/tx_fullhd_virtual_display.json Adds example config targeting a virtual X display (:99).
config/tx_fullhd_single_session.json Removes log_file from the example config.
config/tx_fullhd_screen_capture.json Adds example config for local display screen capture (:0.0+0,0).
config/tx_1session_12bit.json Adds a 12-bit example config (currently schema-mismatched; see review comments).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread meson_options.txt Outdated
Comment thread config/tx_1session_12bit.json
Comment thread src/util/config_reader.c Outdated
Comment thread src/ffmpeg/ffmpeg_decoder.c
- config/tx_fullhd_single_session.json: restore log_file field that was
  accidentally dropped
- config/tx_1session_12bit.json: move fps/fmt into a tx_video block to
  match what the parser actually reads
- config/tx_fullhd_screen_capture.json: use display :99 to match the
  virtual display documented in the README headless walkthrough; drop
  the now-redundant tx_fullhd_virtual_display.json
- meson_options.txt: revert enable_mtl_tx default back to false so it
  matches the documented default behavior
- src/util/config_reader.c: remove duplicate screen_input validation in
  validate_tx_config() (already validated earlier in the function)
- src/ffmpeg/ffmpeg_decoder.c: open_shared_ffmpeg() now passes the
  effective source string (screen_input when screen capture is enabled)
  to open_ffmpeg_decoder() so log messages stay meaningful
- README.md: move the headless/Xvfb/GNOME walkthrough before the
  screen-capture JSON example, turn the example blocks into proper
  headings, link back to Software Requirements instead of repeating
  install commands, and drop the xfce4 aside
@dmkarthi

dmkarthi commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Fix Unit tests

The CI FFmpeg build didn't install libxcb dev packages or pass
--enable-x11grab, so av_find_input_format("x11grab") always returned
NULL on runners, failing test_main_registers_avdevice_for_x11grab and
test_screen_capture_after_avdevice_register_finds_x11grab.

- install libx11-dev libxcb1-dev libxcb-shm0-dev libxcb-xfixes0-dev
- pass --enable-x11grab explicitly to FFmpeg's ./configure
- verify x11grab is registered after the build
- bump the environment cache key so runners rebuild FFmpeg with the
  new flag instead of restoring a stale cached build
Comment thread .github/actions/environment-check/action.yml Outdated
Comment thread .github/actions/environment-check/action.yml Outdated
Comment thread .github/actions/environment-check/action.yml Outdated
roshan-ku added 2 commits July 8, 2026 10:21
x11grab is auto-detected by FFmpeg's ./configure once the libxcb dev
packages are installed, so the explicit --enable-x11grab flag isn't
needed on this system. Revert the flag and the accompanying cache-key
suffix that was added to force a rebuild for it.
The cache key only encoded the dpdk/mtl/ffmpeg version inputs, so
adding the libxcb dev packages to the APT install list didn't
invalidate the previously cached FFmpeg build (built before those
packages existed), and the FFmpeg build step's pkg-config short-circuit
kept restoring/reusing that stale build without x11grab.

Add a generic env-cache-version input (default 2) to the cache key so
runners rebuild the environment now, and document that it should be
bumped in the future whenever a change alters what gets installed into
the cache.
@roshan-ku roshan-ku requested a review from dmkarthi July 8, 2026 06:24
roshan-ku added 2 commits July 8, 2026 14:56
Resolve conflicts:
- meson.build: MTL is always required (pkg-config provides include paths),
  avdevice is always required (needed for x11grab screen capture)
- README.md: keep feature branch content
- Add config/tx_fullhd_screen_capture_multi_session.json (3 sessions, 640x1080 strips)
- Update README: replace Xvfb with Xorg + dummy driver for physical input support
- Document audio routing via PULSE_SERVER for headless virtual display
- Document application launch with PULSE_SERVER (e.g. Chrome)
- Add multi-session screen capture JSON example to README
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.

3 participants