AV sync test against real C64U#82
Merged
Merged
Conversation
- Updated resource.csv and resource.json with new performance metrics. - Modified validation_results.json to reflect changes in packet reception and processing. - Added recording configuration for AV sync in properties.ini. - Enhanced scenario.yaml to include AV sync CSV validation. - Implemented new functions in av_pop_analyzer.py for analyzing AV sync CSV data. - Integrated AV sync CSV analysis into the overall analysis workflow. - Improved error handling and reporting for AV sync validation.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds automated A/V sync testing infrastructure for real C64 Ultimate devices. It introduces a new record_av_sync property that generates av-sync.csv files containing synchronized audio/video pop event data, and integrates with the Ultimate 64 REST API to automatically start/stop test programs on the hardware. The PR also improves network connectivity by supporting hostname resolution (not just IP addresses) and optimizes the configuration update logic to avoid unnecessary streaming restarts.
Changes:
- Added
av-sync.csvrecording capability with automatic C64 test program triggering via REST API - Implemented REST client library (
c64-rest-client.c/h) with CURL for Ultimate 64 control - Enhanced TCP connectivity to resolve hostnames using
getaddrinfoinstead of requiring IP addresses - Optimized
c64_update()to only restart streaming when network-related settings change - Integrated 64tass assembler into CMake build to compile and bundle C64 PRG files
- Added comprehensive E2E validation for
av-sync.csvconsistency with OBS logs
Reviewed changes
Copilot reviewed 53 out of 58 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/e2e/util/av_pop_analyzer.py |
Added analyze_av_sync_csv() and cross-validation logic for CSV vs log consistency |
tests/e2e/assertions/av_sync_csv_validation.py |
New E2E assertion validating av-sync.csv matches OBS log AV SYNC entries |
tests/e2e/assertions/av_sync_offset.py |
Extended to recognize av-sync.csv as a valid CSV source, added MP4 audio pop filtering |
tests/e2e/scenarios/ntsc_default_debug/* |
Enabled av-sync.csv recording and validation assertion |
src/c64-rest-client.c/h |
New REST API client implementation with CURL for Ultimate 64 control |
src/c64-types.h |
Added c64_password, record_av_sync, and av_sync_file fields |
src/c64-source.c |
Optimized update logic to avoid unnecessary streaming restarts, added password handling |
src/c64-network.c |
Refactored TCP connectivity to use getaddrinfo for hostname resolution |
src/c64-record.c |
Added av-sync.csv recording, REST-based PRG trigger/reset logic |
src/c64-av-sync.c |
Extended to write matched A/V events to av-sync.csv file |
src/c64-properties.c |
Added c64_password and record_av_sync UI properties with INI persistence |
CMakeLists.txt |
Integrated 64tass to build av-sync-auto.prg and install it with plugin data |
local-build.sh |
Updated install paths to copy generated PRGs |
data/locale/*.ini |
Added translations for C64UPassword and RecordAVSync across all supported languages |
doc/c64u/*.md |
Added comprehensive REST API and streaming protocol documentation |
.github/docker/Dockerfile.* |
Added 64tass package to all build containers |
6f72741 to
27dd708
Compare
chrisgleissner
commented
Jan 10, 2026
chrisgleissner
left a comment
Owner
Author
There was a problem hiding this comment.
@copilot Resolve all PR comments
Contributor
|
@chrisgleissner I've opened a new pull request, #83, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Initial plan * Add explanatory comments to except clauses in av_sync_csv_validation.py Co-authored-by: chrisgleissner <3969147+chrisgleissner@users.noreply.github.com> * Remove PLANS.md as requested Co-authored-by: chrisgleissner <3969147+chrisgleissner@users.noreply.github.com> * Document "Record A/V Sync" checkbox in README Co-authored-by: chrisgleissner <3969147+chrisgleissner@users.noreply.github.com> * security: exclude password from config export/import Co-authored-by: chrisgleissner <3969147+chrisgleissner@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: chrisgleissner <3969147+chrisgleissner@users.noreply.github.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
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.
Features