Skip to content

Consolidate A/V sync debug logging and enhance real-device test workflow#79

Merged
chrisgleissner merged 4 commits into
test/av-sync-instrumentationfrom
copilot/sub-pr-77
Jan 8, 2026
Merged

Consolidate A/V sync debug logging and enhance real-device test workflow#79
chrisgleissner merged 4 commits into
test/av-sync-instrumentationfrom
copilot/sub-pr-77

Conversation

Copilot AI commented Jan 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Addresses review feedback on #77 to improve A/V sync instrumentation clarity and real-device testing usability.

Changes

A/V Sync Logging Consolidation (src/c64-video.c)

  • Before: Three log statements (1 INFO, 2 DEBUG) with duplicate information
  • After: Single DEBUG-level log per pop with all correlation data (offset, pop counts, timestamps, frame number)
  • Eliminates redundancy while preserving all technical details needed for CSV correlation
// Now emits one unified log:
C64_LOG_DEBUG("AV SYNC: offset=%.1fms video=#%u audio=#%u "
              "detected=%04d-%02d-%02d_%02d:%02d:%02d.%03u video_frame=%u "
              "video_ts=%" PRIu64 " audio_ts=%" PRIu64, ...);

Real-Device Test Script Enhancements (tests/e2e/real-device-av-sync.sh)

  • --obs-only: Skip PRG build/upload when av-sync-auto.prg already running on C64U
  • --no-mp4-analysis: Disable MP4 video analysis (CSV/log analysis only) for faster iteration
  • Both default to full workflow: build PRG → upload → record → analyze all sources

Documentation Overhaul (doc/testing/av-sync-e2e-for-real-device.md)

  • Rewritten as one-stop shop guide for Linux/macOS/Windows (WSL2, Git Bash)
  • Added prerequisites, installation steps, and troubleshooting for all platforms
  • Clarified complete automation: PRG compilation → C64U REST API start → OBS recording with debug → multi-source A/V analysis

Documentation Cleanup (doc/c64u-stream-spec.md)

  • Removed repetitive cropping paragraph, kept concise bullet list

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI mentioned this pull request Jan 8, 2026
Merged
Copilot AI and others added 2 commits January 8, 2026 09:16
@chrisgleissner
Consolidate A/V sync logs and remove duplicate documentation
76699a7 
Co-authored-by: chrisgleissner <3969147+chrisgleissner@users.noreply.github.com>
@chrisgleissner
Extend real-device-av-sync.sh with configurable OBS/MP4 analysis and …
4ddacee 
…cross-platform documentation

Co-authored-by: chrisgleissner <3969147+chrisgleissner@users.noreply.github.com>
Copilot AI changed the title [WIP] Address review comments for A/V sync improvement Consolidate A/V sync debug logging and enhance real-device test workflow Jan 8, 2026
Copilot AI requested a review from chrisgleissner January 8, 2026 09:25
@chrisgleissner chrisgleissner marked this pull request as ready for review January 8, 2026 10:55
Copilot AI review requested due to automatic review settings January 8, 2026 10:55
Copilot AI reviewed Jan 8, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR consolidates A/V sync debug logging in the C code and enhances the real-device test workflow with new command-line options for more flexible testing scenarios.

Key changes:

  • Unified A/V sync logging in src/c64-video.c from 3 log statements (1 INFO, 2 DEBUG) to a single DEBUG-level log containing all correlation data
  • Added --obs-only flag to skip PRG build/upload when the test program is already running on the C64 Ultimate device
  • Added --no-mp4-analysis flag to skip MP4 video analysis for faster iteration during CSV/log-only workflows
  • Comprehensive documentation overhaul in doc/testing/av-sync-e2e-for-real-device.md covering Linux, macOS, Windows (WSL2, Git Bash), with prerequisites, installation steps, and troubleshooting

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/c64-video.c Consolidated A/V sync logging from 3 statements to 1 unified DEBUG log with all technical details; wall clock calculation moved outside conditional for efficiency
tests/e2e/real-device-av-sync.sh Added --obs-only and --no-mp4-analysis flags; updated trap handling to only reset device when PRG was actually run; expanded help text with new examples
tests/e2e/real_device_av_sync.py Added --no-mp4-analysis argument and conditional logic to skip MP4 analysis when flag is set
doc/testing/av-sync-e2e-for-real-device.md Complete rewrite as comprehensive guide covering all platforms, installation steps, usage examples, and troubleshooting
doc/c64u-stream-spec.md Simplified cropping documentation by removing redundant paragraph

Comment thread tests/e2e/real-device-av-sync.sh Outdated
@chrisgleissner
Update tests/e2e/real-device-av-sync.sh
e0a3687 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@chrisgleissner chrisgleissner merged commit 741a479 into test/av-sync-instrumentation Jan 8, 2026
32 checks passed
@chrisgleissner chrisgleissner deleted the copilot/sub-pr-77 branch January 8, 2026 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
+1 more reviewer
@chrisgleissner chrisgleissner chrisgleissner approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@chrisgleissner chrisgleissner

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chrisgleissner