Skip to content

Add PTP hardware timing support to MTL and FFmpeg TX paths - #49

Merged
dmkarthi merged 6 commits into
OpenVisualCloud:mainfrom
sunilnom:ptp
Aug 4, 2026
Merged

Add PTP hardware timing support to MTL and FFmpeg TX paths#49
dmkarthi merged 6 commits into
OpenVisualCloud:mainfrom
sunilnom:ptp

Conversation

@sunilnom

Copy link
Copy Markdown
Contributor
  • Wire MTL_FLAG_PTP_ENABLE/PI/UNICAST_ADDR into both the direct MTL TX path (src/mtl/mtl_tx.c) and the FFmpeg mtl_st20p muxer path (src/ffmpeg/ffmpeg_tx.c), with a sync-notify callback logging master_utc_offset/delta.
  • app_context.h: add ptp_enable/ptp_pi/ptp_unicast fields.
  • config_reader.c/.h: hardcode PTP defaults (enable=true, pi=true, unicast=false) instead of reading them from JSON.

Verified on hardware (Intel I225-V/igc) with both enable_mtl_tx=true and enable_mtl_tx=false builds: PTP enables cleanly (tx pacing ptp, ptp_init pi controller) with no grandmaster present on the test network (sync cnt 0, as expected).

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:

- Wire MTL_FLAG_PTP_ENABLE/PI/UNICAST_ADDR into both the direct MTL TX
  path (src/mtl/mtl_tx.c) and the FFmpeg mtl_st20p muxer path
  (src/ffmpeg/ffmpeg_tx.c), with a sync-notify callback logging
  master_utc_offset/delta.
- app_context.h: add ptp_enable/ptp_pi/ptp_unicast fields.
- config_reader.c/.h: hardcode PTP defaults (enable=true, pi=true,
  unicast=false) instead of reading them from JSON.

Verified on hardware (Intel I225-V/igc) with both enable_mtl_tx=true
and enable_mtl_tx=false builds: PTP enables cleanly (tx pacing ptp,
ptp_init pi controller) with no grandmaster present on the test
network (sync cnt 0, as expected).

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 application-level support for MTL’s built-in PTP client so TX pacing can be driven by hardware timing in both the direct MTL TX pipeline and the FFmpeg mtl_st20p muxer-based TX path.

Changes:

  • Adds PTP fields to the app context and wires them into MTL initialization (flags, PTP pacing, and a sync-notify log callback).
  • Propagates PTP settings through the FFmpeg TX path by setting muxer AVOptions (ptp_enable, ptp_pi, ptp_unicast).
  • Hardcodes PTP defaults in the config reader and logs when PTP is enabled.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/util/config_reader.c Hardcodes PTP defaults and copies them into the runtime app context (with logging).
include/util/config_reader.h Adds PTP fields to the parsed config struct.
include/app_context.h Adds PTP fields to the application context shared by TX paths.
src/mtl/mtl_tx.c Enables MTL PTP flags/pacing and registers a sync-notify callback during mtl_init().
src/ffmpeg/ffmpeg_tx.c Sets PTP-related AVOptions on the mtl_st20p muxer when enabled.

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

Comment thread include/app_context.h Outdated
Comment thread include/util/config_reader.h Outdated
Comment thread src/mtl/mtl_tx.c Outdated
- ffmpeg_tx.c: treat AVERROR_OPTION_NOT_FOUND from PTP AVOptions as a
  benign case so the app works whether or not the MTL plugin carries the
  PTP patch (single INFO log instead of repeated WARNs)
- config_reader.h: use bool for ptp_enable/ptp_pi/ptp_unicast toggles
- app_context.h / mtl_tx.c: clarify PTP comments (used by both the direct
  MTL TX pipeline and the FFmpeg avdevice path)
- config: add tx_gbrp12le_pf_8session.json (8-session pf test config)
PTP-paced TX requires a PTP grandmaster on the network. MTL's built-in
PTP client is slave-only and cannot elect itself grandmaster, so with no
grandmaster present the TX pacing clock never locks and transmission
stalls. Previously ptp_enable/ptp_pi were hardcoded to true in the config
reader, forcing this failure mode on every deployment.

PTP now defaults to disabled (MTL falls back to TSC-based pacing) and is
opted into via an optional top-level "ptp" block:

    "ptp": { "enable": true, "pi": true, "unicast": false }

- Add extract_json_bool() helper to config_reader, mirroring
  extract_json_int() (returns 1/0 for true/false, -1 when absent).
- Parse the optional "ptp" object for enable/pi/unicast; each key falls
  back to false when omitted.
- Log "PTP disabled (default TSC-based TX pacing)" on the default path.
- Document the block in the config_reader schema comment and README,
  including the grandmaster requirement and expected log output.
- Point tx_1session_12bit and tx_fullhd_single_session at the local NIC
  BDF 0000:01:00.1 and correct the latter's source dimensions/format.

Verified on an 8-session 1080p yuv422p12le config: PTP disabled selects
"pacing way: tsc" and runs clean (0 errors, ~29.8 fps per session,
~1361 Mb/s aggregate); setting "enable": true selects "pacing way: ptp".
Comment thread include/util/config_reader.h Outdated
The PTP settings are no longer hardcoded — they are parsed from the
optional top-level "ptp" JSON object and default to false when absent.
@dmkarthi

dmkarthi commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Update unit test for these code changes

Add seven cases for the PTP config option added in 1f17b84:

- block absent           -> all flags default to false
- all keys true          -> all flags set
- all keys false         -> all flags clear
- only "enable" present  -> pi/unicast stay false (keys are independent)
- mixed values, keys out of declaration order
- empty "ptp": {} object -> defaults preserved
- block present          -> surrounding video/tx_video/tx_sessions fields intact

The shared write_ptp_config() helper keeps the surrounding config constant
so each case varies only the "ptp" fragment.
Adds six cases covering the PTP lines gcovr reported as unexecuted
(config_reader.c 128, 130 and 898):

- non-boolean values (numeric and string) reject and fall back to the
  default instead of being coerced -> extract_json_bool early return
- a key name appearing as a value, with no ':' separator, is not
  mistaken for a real key -> continue/loop-exit path
- load_and_apply_config propagates ptp_enable/pi/unicast into the app
  context, covering the PTP-enabled log branch, plus the enable-only and
  no-block-at-all variants

config_reader.c line coverage 93% -> 94%; suite now 131 tests, all green.

@dmkarthi dmkarthi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@dmkarthi
dmkarthi merged commit 92bb5f9 into OpenVisualCloud:main Aug 4, 2026
1 check passed
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