From b86129d22a8e2ee4367bedd795ee2011b605a1ef Mon Sep 17 00:00:00 2001 From: Barry Walker Date: Mon, 7 Sep 2026 09:11:11 -0400 Subject: [PATCH 01/13] feat(stream): send runtime VIC palette Emit the active 16-color RGB palette through the software UDP stack on VIC stream start and runtime updates. Reserve line 0x7fff so the FPGA stream remains unchanged. Refs #850 --- software/io/network/data_streamer.cc | 35 +++++++++-- software/io/network/data_streamer.h | 5 +- software/u64/u64_config.cc | 7 +++ .../io/command_interface/uci_targets_test.py | 63 ++++++++++++++++--- 4 files changed, 96 insertions(+), 14 deletions(-) diff --git a/software/io/network/data_streamer.cc b/software/io/network/data_streamer.cc index 19fce6c6c..6904f314f 100644 --- a/software/io/network/data_streamer.cc +++ b/software/io/network/data_streamer.cc @@ -8,6 +8,7 @@ #include "network_interface.h" #include "socket.h" #include "netdb.h" +#include "u64_config.h" #include "userinterface.h" #include "profiler.h" #include "init_function.h" @@ -38,6 +39,7 @@ struct t_cfg_definition stream_cfg[] = { DataStreamer :: DataStreamer() { my_ip = 0; + palette_sequence = 0; memset(streams, 0, 4*sizeof(stream_config_t)); cfg = ConfigManager :: getConfigManager()->register_store(0x44617461, "Data Streams", stream_cfg, NULL); @@ -201,7 +203,8 @@ SubsysResultCode_e DataStreamer :: startStream(SubsysCommand *cmd) } else { bool ok = false; for(int i=0;i<10;i++) { - send_udp_packet(query_ip, stream->dest_port); + const uint8_t probe[2] = { 0, 0 }; + send_udp_packet(query_ip, stream->dest_port, probe, sizeof(probe)); vTaskDelay(20); if (intf->peekArpTable(query_ip, stream->dest_mac)) { ok = true; @@ -220,6 +223,11 @@ SubsysResultCode_e DataStreamer :: startStream(SubsysCommand *cmd) // start stream! calculate_udp_headers(streamID); + if (streamID == 0) { + uint8_t rgb[16][3]; + U64Config::get_palette_rgb(rgb); + sendVicPalette(rgb); + } if (cmd->bufferSize) { int stopAfter = cmd->bufferSize; @@ -279,7 +287,7 @@ void DataStreamer :: update_task_items(bool writablePath) myActions.stopDbg->setHidden(streams[2].enable == 0); } -void DataStreamer :: send_udp_packet(uint32_t ip, uint16_t port) +void DataStreamer :: send_udp_packet(uint32_t ip, uint16_t port, const uint8_t *data, int length) { int sockfd; static struct sockaddr_in server; @@ -296,15 +304,34 @@ void DataStreamer :: send_udp_packet(uint32_t ip, uint16_t port) server.sin_addr.s_addr = ip; server.sin_port = htons(port); - uint8_t buffer[2] = { 0, 0 }; printf("Send UDP data...\n"); - if (sendto(sockfd, buffer, 2, 0, (const struct sockaddr*)&server, sizeof(server)) < 0) { + if (sendto(sockfd, data, length, 0, (const struct sockaddr*)&server, sizeof(server)) < 0) { printf("Error in sendto()\n"); } // close the socket again lwip_close(sockfd); } +void DataStreamer :: sendVicPalette(const uint8_t rgb[16][3]) +{ + stream_config_t *stream = &streams[0]; + if (!stream->enable) { + return; + } + + uint8_t packet[60] = { 0 }; + uint16_t sequence = palette_sequence++; + packet[0] = (uint8_t)sequence; + packet[1] = (uint8_t)(sequence >> 8); + packet[4] = 0xFF; + packet[5] = 0x7F; // Reserved line number: this packet carries the VIC palette. + packet[6] = 16; + packet[8] = 1; + packet[9] = 24; + memcpy(packet + 12, rgb, 48); + send_udp_packet(stream->dest_ip, stream->dest_port, packet, sizeof(packet)); +} + void DataStreamer :: calculate_udp_headers(int id) { diff --git a/software/io/network/data_streamer.h b/software/io/network/data_streamer.h index a09093c34..5e25c4cfd 100644 --- a/software/io/network/data_streamer.h +++ b/software/io/network/data_streamer.h @@ -43,6 +43,7 @@ class DataStreamer : public ObjectWithMenu uint8_t my_mac[6]; uint32_t my_ip; + uint16_t palette_sequence; stream_config_t streams[4]; TimerHandle_t timers[4]; @@ -52,7 +53,7 @@ class DataStreamer : public ObjectWithMenu SubsysResultCode_e stopStream(SubsysCommand *cmd); void calculate_udp_headers(int id); - void send_udp_packet(uint32_t ip, uint16_t port); + void send_udp_packet(uint32_t ip, uint16_t port, const uint8_t *data, int length); public: DataStreamer(); virtual ~DataStreamer(); @@ -60,6 +61,8 @@ class DataStreamer : public ObjectWithMenu static SubsysResultCode_e S_startStream(SubsysCommand *cmd); static SubsysResultCode_e S_stopStream(SubsysCommand *cmd); + void sendVicPalette(const uint8_t rgb[16][3]); + // from ObjectWithMenu void create_task_items(void); void update_task_items(bool writablePath); diff --git a/software/u64/u64_config.cc b/software/u64/u64_config.cc index d4edec4a7..b16bea97d 100755 --- a/software/u64/u64_config.cc +++ b/software/u64/u64_config.cc @@ -44,6 +44,7 @@ extern "C" { #include "usb_hid.h" #include "usb_hid_config.h" #include "monitor_init.h" +#include "data_streamer.h" // TODO: This doesn't belong here. #ifndef CMD_IF_SLOT_BASE @@ -2768,6 +2769,9 @@ void U64Config :: set_palette_rgb(const uint8_t rgb[16][3]) memcpy(active_palette, rgb, sizeof(active_palette)); active_palette_valid = true; program_palette_rgb(rgb); + if (dataStreamer) { + dataStreamer->sendVicPalette(active_palette); + } } void U64Config :: get_palette_rgb(uint8_t rgb[16][3]) @@ -2783,6 +2787,9 @@ void U64Config :: set_palette_color(uint8_t index, const uint8_t rgb[3]) } memcpy(active_palette[index], rgb, 3); program_palette_color(index, rgb); + if (dataStreamer) { + dataStreamer->sendVicPalette(active_palette); + } } void U64Config :: reset_palette() diff --git a/tests/e2e/io/command_interface/uci_targets_test.py b/tests/e2e/io/command_interface/uci_targets_test.py index bad0c75a7..7b09ab8ec 100755 --- a/tests/e2e/io/command_interface/uci_targets_test.py +++ b/tests/e2e/io/command_interface/uci_targets_test.py @@ -58,6 +58,7 @@ import cli # noqa: E402 import ftp as ftp_lib import rest as rest_lib +import streams as stream_lib import targets from report import ( FAIL, Failure, OK, SKIP, check, check_skip, check_start, detail, @@ -119,6 +120,8 @@ CTRL_CMD_SET_PALETTE = 0x52 CTRL_CMD_SET_PALETTE_COLOR = 0x53 CTRL_CMD_RESET_PALETTE = 0x54 +VIC_PALETTE_PACKET_SIZE = 60 +VIC_PALETTE_LINE = 0x7FFF SOFTIEC_CMD_IDENTIFY = 0x01 SOFTIEC_CMD_LOAD_SU = 0x10 SOFTIEC_CMD_GET_FATNAME = 0x22 @@ -626,7 +629,21 @@ def run_control_target(uci: Uci) -> bool: return True -def run_palette(uci: Uci) -> bool: +def expect_palette_packet(sock, addresses: set[str], expected: bytes) -> None: + for _, packet, mine in stream_lib.receive([sock], addresses, 2.0): + if not mine or len(packet) != VIC_PALETTE_PACKET_SIZE: + continue + if int.from_bytes(packet[4:6], "little") != VIC_PALETTE_LINE: + continue + if packet[6:12] != bytes([16, 0, 1, 24, 0, 0]): + raise Failure(f"palette stream packet has invalid format bytes {packet[6:12]!r}") + if packet[12:] != expected: + raise Failure(f"palette stream packet was {packet[12:]!r}, expected {expected!r}") + return + raise Failure("no palette packet arrived on the VIC stream within 2 seconds") + + +def run_palette(session: RestSession, uci: Uci) -> bool: """Exercise the U64 runtime palette protocol without changing saved config.""" scenario = "palette" product, status = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_HWINFO, 0x00])) @@ -651,7 +668,22 @@ def run_palette(uci: Uci) -> bool: if len(original) != 48: raise Failure(f"{scenario}: expected 48 palette bytes, got {len(original)}") + group = session.target.video_group + port = session.target.video_port + addresses = stream_lib.source_addresses(session.target) + if not addresses: + raise Failure(f"{scenario}: could not resolve the VIC stream source address") + sock = stream_lib.stream_socket(group, port) + stream_started = False try: + with check(f"{scenario}: VIC stream start sends the active RGB palette"): + status, body = session.request( + "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}"}) + if status != 200: + raise Failure(f"video stream start returned HTTP {status}: {body[:200]!r}") + stream_started = True + expect_palette_packet(sock, addresses, original) + with check(f"{scenario}: SET_PALETTE_COLOR changes only the requested color"): changed = bytearray(original) changed[-3:] = bytes(component ^ 0x5A for component in changed[-3:]) @@ -663,6 +695,9 @@ def run_palette(uci: Uci) -> bool: if text != STATUS_OK or actual != bytes(changed): raise Failure(f"{scenario}: single-color readback was {actual!r}, expected {bytes(changed)!r}") + with check(f"{scenario}: runtime color change sends the updated RGB palette"): + expect_palette_packet(sock, addresses, bytes(changed)) + expect(uci, f"{scenario}: color index 16 is rejected", bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE_COLOR, 16, 0, 0, 0]), STATUS_INVALID_PARAMS, reply=b"") @@ -699,13 +734,23 @@ def run_palette(uci: Uci) -> bool: bytes([TARGET_CONTROL, CTRL_CMD_RESET_PALETTE, 0]), STATUS_INVALID_PARAMS, reply=b"") finally: - with check(f"{scenario}: restore the original runtime palette"): - reply, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE]) + original) - if text != STATUS_OK or reply: - raise Failure(f"{scenario}: restore returned data {reply!r}, status {text!r}") - actual, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) - if text != STATUS_OK or actual != original: - raise Failure(f"{scenario}: restored palette readback was {actual!r}") + try: + with check(f"{scenario}: restore the original runtime palette"): + reply, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE]) + original) + if text != STATUS_OK or reply: + raise Failure(f"{scenario}: restore returned data {reply!r}, status {text!r}") + actual, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) + if text != STATUS_OK or actual != original: + raise Failure(f"{scenario}: restored palette readback was {actual!r}") + finally: + try: + if stream_started: + with check(f"{scenario}: stop the VIC stream"): + status, body = session.request("PUT", "/v1/streams/video:stop") + if status != 200: + raise Failure(f"video stream stop returned HTTP {status}: {body[:200]!r}") + finally: + sock.close() return True @@ -1136,7 +1181,7 @@ def run(name: str, fn, *fn_args) -> None: run("transport", run_transport, uci) run("control-target", run_control_target, uci) - run("palette", run_palette, uci) + run("palette", run_palette, session, uci) run("issue-740-matrix", run_issue_740_matrix, session, ftp, uci) run("save-reu-offset-past-end", run_save_reu_offset_past_end, session, uci) run("load-reu-disabled", run_reu_disabled, session, uci, CTRL_CMD_LOAD_REU, "load-reu-disabled") From 6578b890e8f27ed9ad988785996b6e44946bfe0c Mon Sep 17 00:00:00 2001 From: Barry Walker Date: Mon, 7 Sep 2026 09:49:29 -0400 Subject: [PATCH 02/13] fix(stream): match palette packet proposal Refs #850 --- .../io/command_interface/control_target.cc | 4 +++ software/io/network/data_streamer.cc | 25 ++++++++-------- software/io/network/data_streamer.h | 4 +-- software/u64/u64_config.cc | 7 ----- .../io/command_interface/uci_targets_test.py | 29 ++++++++++++++++--- 5 files changed, 43 insertions(+), 26 deletions(-) diff --git a/software/io/command_interface/control_target.cc b/software/io/command_interface/control_target.cc index 7cfee80c5..f81456c26 100644 --- a/software/io/command_interface/control_target.cc +++ b/software/io/command_interface/control_target.cc @@ -13,6 +13,7 @@ #if U64 #include "u64_config.h" #include "u64.h" +#include "data_streamer.h" #else #include "audio_select.h" #endif @@ -273,6 +274,7 @@ void ControlTarget :: parse_command(Message *command, Message **reply, Message * *status = &c_status_invalid_params; } else { U64Config::set_palette_rgb(rgb); + if (dataStreamer) dataStreamer->sendVicPalette(); *status = &c_status_ok; } break; @@ -285,6 +287,7 @@ void ControlTarget :: parse_command(Message *command, Message **reply, Message * *status = &c_status_invalid_params; } else { U64Config::set_palette_color(index, rgb); + if (dataStreamer) dataStreamer->sendVicPalette(); *status = &c_status_ok; } break; @@ -295,6 +298,7 @@ void ControlTarget :: parse_command(Message *command, Message **reply, Message * *status = &c_status_invalid_params; } else { U64Config::reset_palette(); + if (dataStreamer) dataStreamer->sendVicPalette(); *status = &c_status_ok; } break; diff --git a/software/io/network/data_streamer.cc b/software/io/network/data_streamer.cc index 6904f314f..53912113c 100644 --- a/software/io/network/data_streamer.cc +++ b/software/io/network/data_streamer.cc @@ -39,7 +39,7 @@ struct t_cfg_definition stream_cfg[] = { DataStreamer :: DataStreamer() { my_ip = 0; - palette_sequence = 0; + palette_stream_enabled = false; memset(streams, 0, 4*sizeof(stream_config_t)); cfg = ConfigManager :: getConfigManager()->register_store(0x44617461, "Data Streams", stream_cfg, NULL); @@ -223,10 +223,8 @@ SubsysResultCode_e DataStreamer :: startStream(SubsysCommand *cmd) // start stream! calculate_udp_headers(streamID); - if (streamID == 0) { - uint8_t rgb[16][3]; - U64Config::get_palette_rgb(rgb); - sendVicPalette(rgb); + if ((streamID == 0) && palette_stream_enabled) { + sendVicPalette(); } if (cmd->bufferSize) { @@ -312,22 +310,23 @@ void DataStreamer :: send_udp_packet(uint32_t ip, uint16_t port, const uint8_t * lwip_close(sockfd); } -void DataStreamer :: sendVicPalette(const uint8_t rgb[16][3]) +void DataStreamer :: sendVicPalette() { + palette_stream_enabled = true; stream_config_t *stream = &streams[0]; if (!stream->enable) { return; } uint8_t packet[60] = { 0 }; - uint16_t sequence = palette_sequence++; - packet[0] = (uint8_t)sequence; - packet[1] = (uint8_t)(sequence >> 8); - packet[4] = 0xFF; - packet[5] = 0x7F; // Reserved line number: this packet carries the VIC palette. - packet[6] = 16; + packet[4] = 239; + packet[6] = 0x80; + packet[7] = 0x01; packet[8] = 1; - packet[9] = 24; + packet[9] = 4; + packet[10] = 1; + uint8_t rgb[16][3]; + U64Config::get_palette_rgb(rgb); memcpy(packet + 12, rgb, 48); send_udp_packet(stream->dest_ip, stream->dest_port, packet, sizeof(packet)); } diff --git a/software/io/network/data_streamer.h b/software/io/network/data_streamer.h index 5e25c4cfd..9d8a29160 100644 --- a/software/io/network/data_streamer.h +++ b/software/io/network/data_streamer.h @@ -43,7 +43,7 @@ class DataStreamer : public ObjectWithMenu uint8_t my_mac[6]; uint32_t my_ip; - uint16_t palette_sequence; + bool palette_stream_enabled; stream_config_t streams[4]; TimerHandle_t timers[4]; @@ -61,7 +61,7 @@ class DataStreamer : public ObjectWithMenu static SubsysResultCode_e S_startStream(SubsysCommand *cmd); static SubsysResultCode_e S_stopStream(SubsysCommand *cmd); - void sendVicPalette(const uint8_t rgb[16][3]); + void sendVicPalette(); // from ObjectWithMenu void create_task_items(void); diff --git a/software/u64/u64_config.cc b/software/u64/u64_config.cc index b16bea97d..d4edec4a7 100755 --- a/software/u64/u64_config.cc +++ b/software/u64/u64_config.cc @@ -44,7 +44,6 @@ extern "C" { #include "usb_hid.h" #include "usb_hid_config.h" #include "monitor_init.h" -#include "data_streamer.h" // TODO: This doesn't belong here. #ifndef CMD_IF_SLOT_BASE @@ -2769,9 +2768,6 @@ void U64Config :: set_palette_rgb(const uint8_t rgb[16][3]) memcpy(active_palette, rgb, sizeof(active_palette)); active_palette_valid = true; program_palette_rgb(rgb); - if (dataStreamer) { - dataStreamer->sendVicPalette(active_palette); - } } void U64Config :: get_palette_rgb(uint8_t rgb[16][3]) @@ -2787,9 +2783,6 @@ void U64Config :: set_palette_color(uint8_t index, const uint8_t rgb[3]) } memcpy(active_palette[index], rgb, 3); program_palette_color(index, rgb); - if (dataStreamer) { - dataStreamer->sendVicPalette(active_palette); - } } void U64Config :: reset_palette() diff --git a/tests/e2e/io/command_interface/uci_targets_test.py b/tests/e2e/io/command_interface/uci_targets_test.py index 7b09ab8ec..0d029ffa0 100755 --- a/tests/e2e/io/command_interface/uci_targets_test.py +++ b/tests/e2e/io/command_interface/uci_targets_test.py @@ -121,7 +121,7 @@ CTRL_CMD_SET_PALETTE_COLOR = 0x53 CTRL_CMD_RESET_PALETTE = 0x54 VIC_PALETTE_PACKET_SIZE = 60 -VIC_PALETTE_LINE = 0x7FFF +VIC_PALETTE_LINE = 239 SOFTIEC_CMD_IDENTIFY = 0x01 SOFTIEC_CMD_LOAD_SU = 0x10 SOFTIEC_CMD_GET_FATNAME = 0x22 @@ -635,7 +635,9 @@ def expect_palette_packet(sock, addresses: set[str], expected: bytes) -> None: continue if int.from_bytes(packet[4:6], "little") != VIC_PALETTE_LINE: continue - if packet[6:12] != bytes([16, 0, 1, 24, 0, 0]): + if packet[:4] != bytes(4): + raise Failure(f"palette stream packet has nonzero sequence/frame {packet[:4]!r}") + if packet[6:12] != bytes([0x80, 0x01, 1, 4, 1, 0]): raise Failure(f"palette stream packet has invalid format bytes {packet[6:12]!r}") if packet[12:] != expected: raise Failure(f"palette stream packet was {packet[12:]!r}, expected {expected!r}") @@ -643,6 +645,13 @@ def expect_palette_packet(sock, addresses: set[str], expected: bytes) -> None: raise Failure("no palette packet arrived on the VIC stream within 2 seconds") +def expect_no_palette_packet(sock, addresses: set[str]) -> None: + for _, packet, mine in stream_lib.receive([sock], addresses, 0.25): + if (mine and len(packet) == VIC_PALETTE_PACKET_SIZE and + packet[10:12] == bytes([1, 0])): + raise Failure("VIC stream sent palette data before a runtime palette command") + + def run_palette(session: RestSession, uci: Uci) -> bool: """Exercise the U64 runtime palette protocol without changing saved config.""" scenario = "palette" @@ -676,13 +685,13 @@ def run_palette(session: RestSession, uci: Uci) -> bool: sock = stream_lib.stream_socket(group, port) stream_started = False try: - with check(f"{scenario}: VIC stream start sends the active RGB palette"): + with check(f"{scenario}: ordinary VIC stream sends no palette packets"): status, body = session.request( "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}"}) if status != 200: raise Failure(f"video stream start returned HTTP {status}: {body[:200]!r}") stream_started = True - expect_palette_packet(sock, addresses, original) + expect_no_palette_packet(sock, addresses) with check(f"{scenario}: SET_PALETTE_COLOR changes only the requested color"): changed = bytearray(original) @@ -698,6 +707,18 @@ def run_palette(session: RestSession, uci: Uci) -> bool: with check(f"{scenario}: runtime color change sends the updated RGB palette"): expect_palette_packet(sock, addresses, bytes(changed)) + with check(f"{scenario}: restarted VIC stream repeats the runtime palette"): + status, body = session.request("PUT", "/v1/streams/video:stop") + if status != 200: + raise Failure(f"video stream stop returned HTTP {status}: {body[:200]!r}") + stream_started = False + status, body = session.request( + "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}"}) + if status != 200: + raise Failure(f"video stream restart returned HTTP {status}: {body[:200]!r}") + stream_started = True + expect_palette_packet(sock, addresses, bytes(changed)) + expect(uci, f"{scenario}: color index 16 is rejected", bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE_COLOR, 16, 0, 0, 0]), STATUS_INVALID_PARAMS, reply=b"") From 28cf4c2d90c6991579c044099c11513d98de5f0a Mon Sep 17 00:00:00 2001 From: Barry Walker Date: Mon, 7 Sep 2026 11:11:37 -0400 Subject: [PATCH 03/13] fix(streams): require palette packet opt-in Protect legacy video receivers from short palette datagrams while allowing opted-in clients to follow runtime VIC colors. Refs #850 --- doc/api/rest_api_openapi_u64.yaml | 18 +++ software/api/route_streams.cc | 15 +- .../io/command_interface/control_target.cc | 4 - software/io/network/data_streamer.cc | 133 ++++++++++++++++-- software/io/network/data_streamer.h | 11 +- software/u64/u64_config.cc | 13 ++ .../io/command_interface/uci_targets_test.py | 44 ++++-- 7 files changed, 207 insertions(+), 31 deletions(-) diff --git a/doc/api/rest_api_openapi_u64.yaml b/doc/api/rest_api_openapi_u64.yaml index d1484f8da..3434922be 100644 --- a/doc/api/rest_api_openapi_u64.yaml +++ b/doc/api/rest_api_openapi_u64.yaml @@ -3375,6 +3375,13 @@ paths: schema: type: string example: '192.168.1.10:11000' + - name: palette + in: query + required: false + description: For video, request runtime VIC palette packets (0 or 1). + schema: + type: integer + example: 0 responses: '200': description: The stream is running. @@ -3382,6 +3389,17 @@ paths: application/json: schema: $ref: '#/components/schemas/ErrorResponse' + '400': + description: Bad request + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + examples: + Palette must be 0 or 1: + value: + errors: + - Palette must be 0 or 1 '403': $ref: '#/components/responses/Forbidden' '404': diff --git a/software/api/route_streams.cc b/software/api/route_streams.cc index 0dfa580dd..3c5684410 100644 --- a/software/api/route_streams.cc +++ b/software/api/route_streams.cc @@ -31,11 +31,13 @@ API_DOC(PUT, streams, start, PATH_PARAM("stream", "string", "Which stream to act on.", "video") PATH_PARAM_ENUM("stream", "video,audio,debug") PARAM("ip", "string", "Where to send the stream. An address, optionally followed by a port.", "", "192.168.1.10:11000") + PARAM("palette", "integer", "For video, request runtime VIC palette packets (0 or 1).", "", "0") RESPONSE("200", "application/json", "ErrorResponse", "The stream is running.", "") + RESPONSE_ERROR("400", "Palette must be 0 or 1", "") RESPONSE_ERROR("404", "Unrecognized stream name 'screen'", "") RESPONSE_ERROR("500", "No Operational Network Interface", "") ) -API_CALL(PUT, streams, start, NULL, ARRAY ( { { "ip", P_REQUIRED } })) +API_CALL(PUT, streams, start, NULL, ARRAY ( { { "ip", P_REQUIRED }, { "palette", P_OPTIONAL } })) { const char *streamName = args.get_path(0); SubsysCommand *sys_command; @@ -58,7 +60,16 @@ API_CALL(PUT, streams, start, NULL, ARRAY ( { { "ip", P_REQUIRED } })) sys_command->execute(); } - sys_command = new SubsysCommand(NULL, -1, (int)dataStreamer, streamIndex, args["ip"], ""); + const char *paletteArg = args.get_or("palette", NULL); + const bool paletteRequested = paletteArg && strcmp(paletteArg, "1") == 0; + if ((paletteArg && strcmp(paletteArg, "0") != 0 && !paletteRequested) || + (streamIndex != 0 && paletteRequested)) { + resp->error("Palette must be 0 or 1 and is only valid for the video stream"); + resp->json_response(HTTP_BAD_REQUEST); + return; + } + const char *palette = paletteRequested ? "1" : ""; + sys_command = new SubsysCommand(NULL, -1, (int)dataStreamer, streamIndex, args["ip"], palette); sys_command->direct_call = DataStreamer :: S_startStream; SubsysResultCode_t retval = sys_command->execute(); resp->error(SubsysCommand::error_string(retval.status)); diff --git a/software/io/command_interface/control_target.cc b/software/io/command_interface/control_target.cc index f81456c26..7cfee80c5 100644 --- a/software/io/command_interface/control_target.cc +++ b/software/io/command_interface/control_target.cc @@ -13,7 +13,6 @@ #if U64 #include "u64_config.h" #include "u64.h" -#include "data_streamer.h" #else #include "audio_select.h" #endif @@ -274,7 +273,6 @@ void ControlTarget :: parse_command(Message *command, Message **reply, Message * *status = &c_status_invalid_params; } else { U64Config::set_palette_rgb(rgb); - if (dataStreamer) dataStreamer->sendVicPalette(); *status = &c_status_ok; } break; @@ -287,7 +285,6 @@ void ControlTarget :: parse_command(Message *command, Message **reply, Message * *status = &c_status_invalid_params; } else { U64Config::set_palette_color(index, rgb); - if (dataStreamer) dataStreamer->sendVicPalette(); *status = &c_status_ok; } break; @@ -298,7 +295,6 @@ void ControlTarget :: parse_command(Message *command, Message **reply, Message * *status = &c_status_invalid_params; } else { U64Config::reset_palette(); - if (dataStreamer) dataStreamer->sendVicPalette(); *status = &c_status_ok; } break; diff --git a/software/io/network/data_streamer.cc b/software/io/network/data_streamer.cc index 53912113c..1f607c859 100644 --- a/software/io/network/data_streamer.cc +++ b/software/io/network/data_streamer.cc @@ -39,7 +39,11 @@ struct t_cfg_definition stream_cfg[] = { DataStreamer :: DataStreamer() { my_ip = 0; - palette_stream_enabled = false; + palette_stream_requested = false; + palette_generation = 0; + palette_task_handle = NULL; + palette_socket = -1; + palette_socket_ip = 0; memset(streams, 0, 4*sizeof(stream_config_t)); cfg = ConfigManager :: getConfigManager()->register_store(0x44617461, "Data Streams", stream_cfg, NULL); @@ -49,12 +53,19 @@ DataStreamer :: DataStreamer() for (int i=0; i < 4; i++) { timers[i] = xTimerCreate("StreamTimer", 100, pdFALSE, (void *)i, DataStreamer :: S_timer); } + if (xTaskCreate(DataStreamer :: S_palette_task, "VIC Palette", configMINIMAL_STACK_SIZE, + this, PRIO_NETSERVICE, &palette_task_handle) != pdPASS) { + palette_task_handle = NULL; + puts("Could not create VIC palette stream task"); + } } // This should never be called DataStreamer :: ~DataStreamer() { - + if (palette_socket >= 0) { + lwip_close(palette_socket); + } } DataStreamer *dataStreamer; @@ -88,6 +99,11 @@ void DataStreamer :: S_timer(TimerHandle_t a) } } +void DataStreamer :: S_palette_task(void *context) +{ + ((DataStreamer *)context)->paletteTask(); +} + SubsysResultCode_e DataStreamer :: startStream(SubsysCommand *cmd) { if(NetworkInterface :: getNumberOfInterfaces() < 1) { @@ -221,11 +237,19 @@ SubsysResultCode_e DataStreamer :: startStream(SubsysCommand *cmd) } stream->enable = 1; + if (streamID == 0) { + const bool requested = strcmp(cmd->filename.c_str(), "1") == 0; + taskENTER_CRITICAL(); + palette_stream_requested = requested; + taskEXIT_CRITICAL(); + + if (requested && palette_task_handle) { + xTaskNotifyGive(palette_task_handle); + } + } + // start stream! calculate_udp_headers(streamID); - if ((streamID == 0) && palette_stream_enabled) { - sendVicPalette(); - } if (cmd->bufferSize) { int stopAfter = cmd->bufferSize; @@ -251,6 +275,11 @@ SubsysResultCode_e DataStreamer :: stopStream(SubsysCommand *cmd) } stream_config_t *stream = &streams[streamID]; stream->enable = 0; + if (streamID == 0) { + taskENTER_CRITICAL(); + palette_stream_requested = false; + taskEXIT_CRITICAL(); + } calculate_udp_headers(streamID); return SSRET_OK; } @@ -288,7 +317,7 @@ void DataStreamer :: update_task_items(bool writablePath) void DataStreamer :: send_udp_packet(uint32_t ip, uint16_t port, const uint8_t *data, int length) { int sockfd; - static struct sockaddr_in server; + struct sockaddr_in server; sockfd = socket(AF_INET, SOCK_DGRAM, 0); if (sockfd < 0) @@ -302,7 +331,6 @@ void DataStreamer :: send_udp_packet(uint32_t ip, uint16_t port, const uint8_t * server.sin_addr.s_addr = ip; server.sin_port = htons(port); - printf("Send UDP data...\n"); if (sendto(sockfd, data, length, 0, (const struct sockaddr*)&server, sizeof(server)) < 0) { printf("Error in sendto()\n"); } @@ -310,15 +338,50 @@ void DataStreamer :: send_udp_packet(uint32_t ip, uint16_t port, const uint8_t * lwip_close(sockfd); } -void DataStreamer :: sendVicPalette() +bool DataStreamer :: sendVicPalette() { - palette_stream_enabled = true; - stream_config_t *stream = &streams[0]; - if (!stream->enable) { - return; + uint32_t source_ip; + uint32_t dest_ip; + uint16_t dest_port; + uint16_t generation; + taskENTER_CRITICAL(); + const bool requested = palette_stream_requested && streams[0].enable; + source_ip = my_ip; + dest_ip = streams[0].dest_ip; + dest_port = streams[0].dest_port; + generation = palette_generation; + taskEXIT_CRITICAL(); + + if (!requested || !source_ip || !dest_ip || !dest_port) { + return false; + } + + if ((palette_socket < 0) || (palette_socket_ip != source_ip)) { + if (palette_socket >= 0) { + lwip_close(palette_socket); + } + palette_socket = socket(AF_INET, SOCK_DGRAM, 0); + palette_socket_ip = 0; + if (palette_socket < 0) { + return false; + } + + struct sockaddr_in local; + memset(&local, 0, sizeof(local)); + local.sin_family = AF_INET; + local.sin_addr.s_addr = source_ip; + local.sin_port = htons(53248); + if (bind(palette_socket, (const struct sockaddr *)&local, sizeof(local)) < 0) { + lwip_close(palette_socket); + palette_socket = -1; + return false; + } + palette_socket_ip = source_ip; } uint8_t packet[60] = { 0 }; + packet[0] = (uint8_t)generation; + packet[1] = (uint8_t)(generation >> 8); packet[4] = 239; packet[6] = 0x80; packet[7] = 0x01; @@ -328,7 +391,51 @@ void DataStreamer :: sendVicPalette() uint8_t rgb[16][3]; U64Config::get_palette_rgb(rgb); memcpy(packet + 12, rgb, 48); - send_udp_packet(stream->dest_ip, stream->dest_port, packet, sizeof(packet)); + + struct sockaddr_in destination; + memset(&destination, 0, sizeof(destination)); + destination.sin_family = AF_INET; + destination.sin_addr.s_addr = dest_ip; + destination.sin_port = htons(dest_port); + if (sendto(palette_socket, packet, sizeof(packet), 0, + (const struct sockaddr *)&destination, sizeof(destination)) < 0) { + lwip_close(palette_socket); + palette_socket = -1; + palette_socket_ip = 0; + return false; + } + return true; +} + +void DataStreamer :: vicPaletteChanged() +{ + taskENTER_CRITICAL(); + palette_generation++; + taskEXIT_CRITICAL(); + if (palette_task_handle) { + xTaskNotifyGive(palette_task_handle); + } +} + +void DataStreamer :: paletteTask() +{ + const TickType_t repeat_ticks = 1000 / portTICK_PERIOD_MS; + const TickType_t minimum_ticks = 20 / portTICK_PERIOD_MS; + TickType_t last_send = 0; + + while (true) { + const uint32_t notified = ulTaskNotifyTake(pdTRUE, repeat_ticks); + if (notified && last_send) { + const TickType_t now = xTaskGetTickCount(); + const TickType_t elapsed = now - last_send; + if (elapsed < minimum_ticks) { + vTaskDelay(minimum_ticks - elapsed); + } + } + if (sendVicPalette()) { + last_send = xTaskGetTickCount(); + } + } } diff --git a/software/io/network/data_streamer.h b/software/io/network/data_streamer.h index 9d8a29160..2b126a006 100644 --- a/software/io/network/data_streamer.h +++ b/software/io/network/data_streamer.h @@ -43,17 +43,24 @@ class DataStreamer : public ObjectWithMenu uint8_t my_mac[6]; uint32_t my_ip; - bool palette_stream_enabled; + volatile bool palette_stream_requested; + volatile uint16_t palette_generation; + TaskHandle_t palette_task_handle; + int palette_socket; + uint32_t palette_socket_ip; stream_config_t streams[4]; TimerHandle_t timers[4]; static void S_timer(TimerHandle_t a); + static void S_palette_task(void *context); SubsysResultCode_e startStream(SubsysCommand *cmd); SubsysResultCode_e stopStream(SubsysCommand *cmd); void calculate_udp_headers(int id); void send_udp_packet(uint32_t ip, uint16_t port, const uint8_t *data, int length); + bool sendVicPalette(); + void paletteTask(); public: DataStreamer(); virtual ~DataStreamer(); @@ -61,7 +68,7 @@ class DataStreamer : public ObjectWithMenu static SubsysResultCode_e S_startStream(SubsysCommand *cmd); static SubsysResultCode_e S_stopStream(SubsysCommand *cmd); - void sendVicPalette(); + void vicPaletteChanged(); // from ObjectWithMenu void create_task_items(void); diff --git a/software/u64/u64_config.cc b/software/u64/u64_config.cc index d4edec4a7..c6bc45cb6 100755 --- a/software/u64/u64_config.cc +++ b/software/u64/u64_config.cc @@ -21,6 +21,7 @@ extern "C" { #include "product.h" #include "userinterface.h" #include "u64_config.h" +#include "data_streamer.h" #include "audio_select.h" #include "fpll.h" #include "i2c_drv.h" @@ -2765,24 +2766,36 @@ static void program_palette_color(uint8_t index, const uint8_t rgb[3]) void U64Config :: set_palette_rgb(const uint8_t rgb[16][3]) { + taskENTER_CRITICAL(); memcpy(active_palette, rgb, sizeof(active_palette)); active_palette_valid = true; + taskEXIT_CRITICAL(); program_palette_rgb(rgb); + if (dataStreamer) { + dataStreamer->vicPaletteChanged(); + } } void U64Config :: get_palette_rgb(uint8_t rgb[16][3]) { + taskENTER_CRITICAL(); memcpy(rgb, active_palette_valid ? active_palette : default_colors, sizeof(active_palette)); + taskEXIT_CRITICAL(); } void U64Config :: set_palette_color(uint8_t index, const uint8_t rgb[3]) { + taskENTER_CRITICAL(); if (!active_palette_valid) { memcpy(active_palette, default_colors, sizeof(active_palette)); active_palette_valid = true; } memcpy(active_palette[index], rgb, 3); + taskEXIT_CRITICAL(); program_palette_color(index, rgb); + if (dataStreamer) { + dataStreamer->vicPaletteChanged(); + } } void U64Config :: reset_palette() diff --git a/tests/e2e/io/command_interface/uci_targets_test.py b/tests/e2e/io/command_interface/uci_targets_test.py index 0d029ffa0..7a2bb9601 100755 --- a/tests/e2e/io/command_interface/uci_targets_test.py +++ b/tests/e2e/io/command_interface/uci_targets_test.py @@ -629,19 +629,17 @@ def run_control_target(uci: Uci) -> bool: return True -def expect_palette_packet(sock, addresses: set[str], expected: bytes) -> None: +def expect_palette_packet(sock, addresses: set[str], expected: bytes) -> int: for _, packet, mine in stream_lib.receive([sock], addresses, 2.0): if not mine or len(packet) != VIC_PALETTE_PACKET_SIZE: continue if int.from_bytes(packet[4:6], "little") != VIC_PALETTE_LINE: continue - if packet[:4] != bytes(4): - raise Failure(f"palette stream packet has nonzero sequence/frame {packet[:4]!r}") if packet[6:12] != bytes([0x80, 0x01, 1, 4, 1, 0]): raise Failure(f"palette stream packet has invalid format bytes {packet[6:12]!r}") if packet[12:] != expected: raise Failure(f"palette stream packet was {packet[12:]!r}, expected {expected!r}") - return + return int.from_bytes(packet[:2], "little") raise Failure("no palette packet arrived on the VIC stream within 2 seconds") @@ -649,7 +647,7 @@ def expect_no_palette_packet(sock, addresses: set[str]) -> None: for _, packet, mine in stream_lib.receive([sock], addresses, 0.25): if (mine and len(packet) == VIC_PALETTE_PACKET_SIZE and packet[10:12] == bytes([1, 0])): - raise Failure("VIC stream sent palette data before a runtime palette command") + raise Failure("VIC stream sent palette data without an explicit palette request") def run_palette(session: RestSession, uci: Uci) -> bool: @@ -704,20 +702,26 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if text != STATUS_OK or actual != bytes(changed): raise Failure(f"{scenario}: single-color readback was {actual!r}, expected {bytes(changed)!r}") - with check(f"{scenario}: runtime color change sends the updated RGB palette"): - expect_palette_packet(sock, addresses, bytes(changed)) + with check(f"{scenario}: ordinary VIC stream remains unchanged after a runtime palette command"): + expect_no_palette_packet(sock, addresses) - with check(f"{scenario}: restarted VIC stream repeats the runtime palette"): + with check(f"{scenario}: opted-in VIC stream starts with the current runtime palette"): status, body = session.request("PUT", "/v1/streams/video:stop") if status != 200: raise Failure(f"video stream stop returned HTTP {status}: {body[:200]!r}") stream_started = False status, body = session.request( - "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}"}) + "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}", "palette": 1}) if status != 200: raise Failure(f"video stream restart returned HTTP {status}: {body[:200]!r}") stream_started = True - expect_palette_packet(sock, addresses, bytes(changed)) + generation = expect_palette_packet(sock, addresses, bytes(changed)) + + with check(f"{scenario}: opted-in VIC stream periodically repeats its palette"): + repeated_generation = expect_palette_packet(sock, addresses, bytes(changed)) + if repeated_generation != generation: + raise Failure( + f"palette repeat generation {repeated_generation} differs from initial {generation}") expect(uci, f"{scenario}: color index 16 is rejected", bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE_COLOR, 16, 0, 0, 0]), @@ -733,6 +737,11 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if text != STATUS_OK or actual != replacement: raise Failure(f"{scenario}: full-palette readback was {actual!r}, expected {replacement!r}") + with check(f"{scenario}: palette changes advance the streamed generation"): + replacement_generation = expect_palette_packet(sock, addresses, replacement) + if ((replacement_generation - generation) & 0xFFFF) == 0: + raise Failure("palette generation did not advance after SET_PALETTE") + expect(uci, f"{scenario}: short SET_PALETTE payload is rejected", bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE]) + original[:-1], STATUS_INVALID_PARAMS, reply=b"") @@ -751,9 +760,24 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if text != STATUS_OK or actual != default_palette: raise Failure(f"{scenario}: reset palette readback was {actual!r}, expected {default_palette!r}") + with check(f"{scenario}: RESET_PALETTE is streamed to an opted-in client"): + expect_palette_packet(sock, addresses, default_palette) + expect(uci, f"{scenario}: RESET_PALETTE rejects a payload", bytes([TARGET_CONTROL, CTRL_CMD_RESET_PALETTE, 0]), STATUS_INVALID_PARAMS, reply=b"") + + with check(f"{scenario}: a later ordinary stream is not opted in implicitly"): + status, body = session.request("PUT", "/v1/streams/video:stop") + if status != 200: + raise Failure(f"video stream stop returned HTTP {status}: {body[:200]!r}") + stream_started = False + status, body = session.request( + "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}"}) + if status != 200: + raise Failure(f"video stream restart returned HTTP {status}: {body[:200]!r}") + stream_started = True + expect_no_palette_packet(sock, addresses) finally: try: with check(f"{scenario}: restore the original runtime palette"): From 6b2e3393a7189c7a221d58ad603412af6f71e335 Mon Sep 17 00:00:00 2001 From: Barry Walker Date: Mon, 7 Sep 2026 11:43:15 -0400 Subject: [PATCH 04/13] fix(streams): avoid palette port collision --- software/io/network/data_streamer.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/software/io/network/data_streamer.cc b/software/io/network/data_streamer.cc index 1f607c859..4dde3ea4d 100644 --- a/software/io/network/data_streamer.cc +++ b/software/io/network/data_streamer.cc @@ -370,7 +370,7 @@ bool DataStreamer :: sendVicPalette() memset(&local, 0, sizeof(local)); local.sin_family = AF_INET; local.sin_addr.s_addr = source_ip; - local.sin_port = htons(53248); + local.sin_port = 0; if (bind(palette_socket, (const struct sockaddr *)&local, sizeof(local)) < 0) { lwip_close(palette_socket); palette_socket = -1; From fff6027d5e3c81222b8325913c8b2cbe708ba249 Mon Sep 17 00:00:00 2001 From: Barry Walker Date: Mon, 7 Sep 2026 12:00:30 -0400 Subject: [PATCH 05/13] fix(streams): route palette on active interface --- software/io/network/data_streamer.cc | 27 ++++++++++--------- .../io/command_interface/uci_targets_test.py | 27 +++++++++++-------- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/software/io/network/data_streamer.cc b/software/io/network/data_streamer.cc index 4dde3ea4d..576318ed2 100644 --- a/software/io/network/data_streamer.cc +++ b/software/io/network/data_streamer.cc @@ -356,7 +356,10 @@ bool DataStreamer :: sendVicPalette() return false; } - if ((palette_socket < 0) || (palette_socket_ip != source_ip)) { + // LwIP may route unicast over another active interface, so let it choose + // that packet's source. Multicast keeps the VIC source for client filtering. + const uint32_t socket_ip = (dest_ip & 0x000000F8) == 0x000000E8 ? source_ip : 0; + if ((palette_socket < 0) || (palette_socket_ip != socket_ip)) { if (palette_socket >= 0) { lwip_close(palette_socket); } @@ -365,18 +368,18 @@ bool DataStreamer :: sendVicPalette() if (palette_socket < 0) { return false; } - - struct sockaddr_in local; - memset(&local, 0, sizeof(local)); - local.sin_family = AF_INET; - local.sin_addr.s_addr = source_ip; - local.sin_port = 0; - if (bind(palette_socket, (const struct sockaddr *)&local, sizeof(local)) < 0) { - lwip_close(palette_socket); - palette_socket = -1; - return false; + if (socket_ip) { + struct sockaddr_in local; + memset(&local, 0, sizeof(local)); + local.sin_family = AF_INET; + local.sin_addr.s_addr = socket_ip; + if (bind(palette_socket, (const struct sockaddr *)&local, sizeof(local)) < 0) { + lwip_close(palette_socket); + palette_socket = -1; + return false; + } } - palette_socket_ip = source_ip; + palette_socket_ip = socket_ip; } uint8_t packet[60] = { 0 }; diff --git a/tests/e2e/io/command_interface/uci_targets_test.py b/tests/e2e/io/command_interface/uci_targets_test.py index 7a2bb9601..bc774757e 100755 --- a/tests/e2e/io/command_interface/uci_targets_test.py +++ b/tests/e2e/io/command_interface/uci_targets_test.py @@ -629,9 +629,10 @@ def run_control_target(uci: Uci) -> bool: return True -def expect_palette_packet(sock, addresses: set[str], expected: bytes) -> int: +def expect_palette_packet(sock, addresses: set[str], expected: bytes, + accept_any_source: bool = False) -> int: for _, packet, mine in stream_lib.receive([sock], addresses, 2.0): - if not mine or len(packet) != VIC_PALETTE_PACKET_SIZE: + if (not mine and not accept_any_source) or len(packet) != VIC_PALETTE_PACKET_SIZE: continue if int.from_bytes(packet[4:6], "little") != VIC_PALETTE_LINE: continue @@ -643,9 +644,9 @@ def expect_palette_packet(sock, addresses: set[str], expected: bytes) -> int: raise Failure("no palette packet arrived on the VIC stream within 2 seconds") -def expect_no_palette_packet(sock, addresses: set[str]) -> None: +def expect_no_palette_packet(sock, addresses: set[str], accept_any_source: bool = False) -> None: for _, packet, mine in stream_lib.receive([sock], addresses, 0.25): - if (mine and len(packet) == VIC_PALETTE_PACKET_SIZE and + if ((mine or accept_any_source) and len(packet) == VIC_PALETTE_PACKET_SIZE and packet[10:12] == bytes([1, 0])): raise Failure("VIC stream sent palette data without an explicit palette request") @@ -677,6 +678,10 @@ def run_palette(session: RestSession, uci: Uci) -> bool: group = session.target.video_group port = session.target.video_port + # A dual-homed Ultimate can send the FPGA video from one interface and the + # software palette packet from another. A unicast destination belongs only + # to this socket; multicast still needs source filtering between devices. + accept_any_source = not stream_lib.is_multicast(group) addresses = stream_lib.source_addresses(session.target) if not addresses: raise Failure(f"{scenario}: could not resolve the VIC stream source address") @@ -689,7 +694,7 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if status != 200: raise Failure(f"video stream start returned HTTP {status}: {body[:200]!r}") stream_started = True - expect_no_palette_packet(sock, addresses) + expect_no_palette_packet(sock, addresses, accept_any_source) with check(f"{scenario}: SET_PALETTE_COLOR changes only the requested color"): changed = bytearray(original) @@ -703,7 +708,7 @@ def run_palette(session: RestSession, uci: Uci) -> bool: raise Failure(f"{scenario}: single-color readback was {actual!r}, expected {bytes(changed)!r}") with check(f"{scenario}: ordinary VIC stream remains unchanged after a runtime palette command"): - expect_no_palette_packet(sock, addresses) + expect_no_palette_packet(sock, addresses, accept_any_source) with check(f"{scenario}: opted-in VIC stream starts with the current runtime palette"): status, body = session.request("PUT", "/v1/streams/video:stop") @@ -715,10 +720,10 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if status != 200: raise Failure(f"video stream restart returned HTTP {status}: {body[:200]!r}") stream_started = True - generation = expect_palette_packet(sock, addresses, bytes(changed)) + generation = expect_palette_packet(sock, addresses, bytes(changed), accept_any_source) with check(f"{scenario}: opted-in VIC stream periodically repeats its palette"): - repeated_generation = expect_palette_packet(sock, addresses, bytes(changed)) + repeated_generation = expect_palette_packet(sock, addresses, bytes(changed), accept_any_source) if repeated_generation != generation: raise Failure( f"palette repeat generation {repeated_generation} differs from initial {generation}") @@ -738,7 +743,7 @@ def run_palette(session: RestSession, uci: Uci) -> bool: raise Failure(f"{scenario}: full-palette readback was {actual!r}, expected {replacement!r}") with check(f"{scenario}: palette changes advance the streamed generation"): - replacement_generation = expect_palette_packet(sock, addresses, replacement) + replacement_generation = expect_palette_packet(sock, addresses, replacement, accept_any_source) if ((replacement_generation - generation) & 0xFFFF) == 0: raise Failure("palette generation did not advance after SET_PALETTE") @@ -761,7 +766,7 @@ def run_palette(session: RestSession, uci: Uci) -> bool: raise Failure(f"{scenario}: reset palette readback was {actual!r}, expected {default_palette!r}") with check(f"{scenario}: RESET_PALETTE is streamed to an opted-in client"): - expect_palette_packet(sock, addresses, default_palette) + expect_palette_packet(sock, addresses, default_palette, accept_any_source) expect(uci, f"{scenario}: RESET_PALETTE rejects a payload", bytes([TARGET_CONTROL, CTRL_CMD_RESET_PALETTE, 0]), @@ -777,7 +782,7 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if status != 200: raise Failure(f"video stream restart returned HTTP {status}: {body[:200]!r}") stream_started = True - expect_no_palette_packet(sock, addresses) + expect_no_palette_packet(sock, addresses, accept_any_source) finally: try: with check(f"{scenario}: restore the original runtime palette"): From 3d87e707b2ad9fa2c46011a2ced45ceb7f037ec2 Mon Sep 17 00:00:00 2001 From: Barry Walker Date: Mon, 7 Sep 2026 13:04:45 -0400 Subject: [PATCH 06/13] fix(streams): harden palette snapshots Pair each generation with one atomic RGB snapshot and cap burst updates at one packet per 20 ms. Exercise 240 runtime changes while checking coalescing, packet contents, and uninterrupted video sequencing. --- doc/api/rest_api_openapi_u64.yaml | 4 +- software/api/route_streams.cc | 5 +- software/io/network/data_streamer.cc | 50 +++--- software/io/network/data_streamer.h | 3 +- software/u64/u64_config.cc | 8 +- software/u64/u64_config.h | 2 +- .../io/command_interface/uci_targets_test.py | 144 +++++++++++++++++- .../command_interface/vic_palette_burst.asm | 126 +++++++++++++++ 8 files changed, 304 insertions(+), 38 deletions(-) create mode 100644 tests/e2e/io/command_interface/vic_palette_burst.asm diff --git a/doc/api/rest_api_openapi_u64.yaml b/doc/api/rest_api_openapi_u64.yaml index 3434922be..ef153f057 100644 --- a/doc/api/rest_api_openapi_u64.yaml +++ b/doc/api/rest_api_openapi_u64.yaml @@ -3396,10 +3396,10 @@ paths: schema: $ref: '#/components/schemas/ErrorResponse' examples: - Palette must be 0 or 1: + Palette must be 0 or 1 and is only valid for the video stream: value: errors: - - Palette must be 0 or 1 + - Palette must be 0 or 1 and is only valid for the video stream '403': $ref: '#/components/responses/Forbidden' '404': diff --git a/software/api/route_streams.cc b/software/api/route_streams.cc index 3c5684410..1701957e5 100644 --- a/software/api/route_streams.cc +++ b/software/api/route_streams.cc @@ -33,7 +33,7 @@ API_DOC(PUT, streams, start, PARAM("ip", "string", "Where to send the stream. An address, optionally followed by a port.", "", "192.168.1.10:11000") PARAM("palette", "integer", "For video, request runtime VIC palette packets (0 or 1).", "", "0") RESPONSE("200", "application/json", "ErrorResponse", "The stream is running.", "") - RESPONSE_ERROR("400", "Palette must be 0 or 1", "") + RESPONSE_ERROR("400", "Palette must be 0 or 1 and is only valid for the video stream", "") RESPONSE_ERROR("404", "Unrecognized stream name 'screen'", "") RESPONSE_ERROR("500", "No Operational Network Interface", "") ) @@ -62,8 +62,7 @@ API_CALL(PUT, streams, start, NULL, ARRAY ( { { "ip", P_REQUIRED }, { "palette", const char *paletteArg = args.get_or("palette", NULL); const bool paletteRequested = paletteArg && strcmp(paletteArg, "1") == 0; - if ((paletteArg && strcmp(paletteArg, "0") != 0 && !paletteRequested) || - (streamIndex != 0 && paletteRequested)) { + if (paletteArg && (streamIndex != 0 || (strcmp(paletteArg, "0") != 0 && !paletteRequested))) { resp->error("Palette must be 0 or 1 and is only valid for the video stream"); resp->json_response(HTTP_BAD_REQUEST); return; diff --git a/software/io/network/data_streamer.cc b/software/io/network/data_streamer.cc index 576318ed2..e24b9e8be 100644 --- a/software/io/network/data_streamer.cc +++ b/software/io/network/data_streamer.cc @@ -40,7 +40,6 @@ DataStreamer :: DataStreamer() { my_ip = 0; palette_stream_requested = false; - palette_generation = 0; palette_task_handle = NULL; palette_socket = -1; palette_socket_ip = 0; @@ -219,8 +218,7 @@ SubsysResultCode_e DataStreamer :: startStream(SubsysCommand *cmd) } else { bool ok = false; for(int i=0;i<10;i++) { - const uint8_t probe[2] = { 0, 0 }; - send_udp_packet(query_ip, stream->dest_port, probe, sizeof(probe)); + send_udp_packet(query_ip, stream->dest_port); vTaskDelay(20); if (intf->peekArpTable(query_ip, stream->dest_mac)) { ok = true; @@ -279,6 +277,9 @@ SubsysResultCode_e DataStreamer :: stopStream(SubsysCommand *cmd) taskENTER_CRITICAL(); palette_stream_requested = false; taskEXIT_CRITICAL(); + if (palette_task_handle) { + xTaskNotifyGive(palette_task_handle); + } } calculate_udp_headers(streamID); return SSRET_OK; @@ -314,7 +315,7 @@ void DataStreamer :: update_task_items(bool writablePath) myActions.stopDbg->setHidden(streams[2].enable == 0); } -void DataStreamer :: send_udp_packet(uint32_t ip, uint16_t port, const uint8_t *data, int length) +void DataStreamer :: send_udp_packet(uint32_t ip, uint16_t port) { int sockfd; struct sockaddr_in server; @@ -331,7 +332,9 @@ void DataStreamer :: send_udp_packet(uint32_t ip, uint16_t port, const uint8_t * server.sin_addr.s_addr = ip; server.sin_port = htons(port); - if (sendto(sockfd, data, length, 0, (const struct sockaddr*)&server, sizeof(server)) < 0) { + uint8_t buffer[2] = { 0, 0 }; + printf("Send UDP data...\n"); + if (sendto(sockfd, buffer, 2, 0, (const struct sockaddr*)&server, sizeof(server)) < 0) { printf("Error in sendto()\n"); } // close the socket again @@ -343,16 +346,19 @@ bool DataStreamer :: sendVicPalette() uint32_t source_ip; uint32_t dest_ip; uint16_t dest_port; - uint16_t generation; taskENTER_CRITICAL(); const bool requested = palette_stream_requested && streams[0].enable; source_ip = my_ip; dest_ip = streams[0].dest_ip; dest_port = streams[0].dest_port; - generation = palette_generation; taskEXIT_CRITICAL(); if (!requested || !source_ip || !dest_ip || !dest_port) { + if (palette_socket >= 0) { + lwip_close(palette_socket); + palette_socket = -1; + palette_socket_ip = 0; + } return false; } @@ -383,17 +389,17 @@ bool DataStreamer :: sendVicPalette() } uint8_t packet[60] = { 0 }; + uint8_t rgb[16][3]; + const uint16_t generation = U64Config::get_palette_rgb(rgb); packet[0] = (uint8_t)generation; packet[1] = (uint8_t)(generation >> 8); - packet[4] = 239; - packet[6] = 0x80; + packet[4] = 239; // reserved line number + packet[6] = 0x80; // 384 pixels per line, little endian packet[7] = 0x01; - packet[8] = 1; - packet[9] = 4; - packet[10] = 1; - uint8_t rgb[16][3]; - U64Config::get_palette_rgb(rgb); - memcpy(packet + 12, rgb, 48); + packet[8] = 1; // one palette + packet[9] = 4; // four bits per VIC color index + packet[10] = 1; // RGB palette encoding + memcpy(packet + 12, rgb, sizeof(rgb)); struct sockaddr_in destination; memset(&destination, 0, sizeof(destination)); @@ -412,9 +418,6 @@ bool DataStreamer :: sendVicPalette() void DataStreamer :: vicPaletteChanged() { - taskENTER_CRITICAL(); - palette_generation++; - taskEXIT_CRITICAL(); if (palette_task_handle) { xTaskNotifyGive(palette_task_handle); } @@ -422,12 +425,17 @@ void DataStreamer :: vicPaletteChanged() void DataStreamer :: paletteTask() { - const TickType_t repeat_ticks = 1000 / portTICK_PERIOD_MS; - const TickType_t minimum_ticks = 20 / portTICK_PERIOD_MS; + const TickType_t repeat_ticks = pdMS_TO_TICKS(1000); + // Twenty milliseconds is longer than one PAL or NTSC frame, so even a + // burst of palette writes adds at most one metadata packet per video frame. + const TickType_t minimum_ticks = pdMS_TO_TICKS(20); TickType_t last_send = 0; while (true) { - const uint32_t notified = ulTaskNotifyTake(pdTRUE, repeat_ticks); + taskENTER_CRITICAL(); + const bool requested = palette_stream_requested && streams[0].enable; + taskEXIT_CRITICAL(); + const uint32_t notified = ulTaskNotifyTake(pdTRUE, requested ? repeat_ticks : portMAX_DELAY); if (notified && last_send) { const TickType_t now = xTaskGetTickCount(); const TickType_t elapsed = now - last_send; diff --git a/software/io/network/data_streamer.h b/software/io/network/data_streamer.h index 2b126a006..56dbbfbeb 100644 --- a/software/io/network/data_streamer.h +++ b/software/io/network/data_streamer.h @@ -44,7 +44,6 @@ class DataStreamer : public ObjectWithMenu uint8_t my_mac[6]; uint32_t my_ip; volatile bool palette_stream_requested; - volatile uint16_t palette_generation; TaskHandle_t palette_task_handle; int palette_socket; uint32_t palette_socket_ip; @@ -58,7 +57,7 @@ class DataStreamer : public ObjectWithMenu SubsysResultCode_e stopStream(SubsysCommand *cmd); void calculate_udp_headers(int id); - void send_udp_packet(uint32_t ip, uint16_t port, const uint8_t *data, int length); + void send_udp_packet(uint32_t ip, uint16_t port); bool sendVicPalette(); void paletteTask(); public: diff --git a/software/u64/u64_config.cc b/software/u64/u64_config.cc index c6bc45cb6..77c2f4e3a 100755 --- a/software/u64/u64_config.cc +++ b/software/u64/u64_config.cc @@ -72,6 +72,7 @@ const uint8_t default_colors[16][3] = { static uint8_t active_palette[16][3]; static bool active_palette_valid = false; +static uint16_t active_palette_generation = 0; // static pointer U64Config *u64_configurator = NULL; @@ -2769,6 +2770,7 @@ void U64Config :: set_palette_rgb(const uint8_t rgb[16][3]) taskENTER_CRITICAL(); memcpy(active_palette, rgb, sizeof(active_palette)); active_palette_valid = true; + active_palette_generation++; taskEXIT_CRITICAL(); program_palette_rgb(rgb); if (dataStreamer) { @@ -2776,11 +2778,14 @@ void U64Config :: set_palette_rgb(const uint8_t rgb[16][3]) } } -void U64Config :: get_palette_rgb(uint8_t rgb[16][3]) +uint16_t U64Config :: get_palette_rgb(uint8_t rgb[16][3]) { + // Pair the generation and RGB bytes from one complete palette snapshot. taskENTER_CRITICAL(); memcpy(rgb, active_palette_valid ? active_palette : default_colors, sizeof(active_palette)); + const uint16_t generation = active_palette_generation; taskEXIT_CRITICAL(); + return generation; } void U64Config :: set_palette_color(uint8_t index, const uint8_t rgb[3]) @@ -2791,6 +2796,7 @@ void U64Config :: set_palette_color(uint8_t index, const uint8_t rgb[3]) active_palette_valid = true; } memcpy(active_palette[index], rgb, 3); + active_palette_generation++; taskEXIT_CRITICAL(); program_palette_color(index, rgb); if (dataStreamer) { diff --git a/software/u64/u64_config.h b/software/u64/u64_config.h index d40e1a134..3d26ca2ff 100755 --- a/software/u64/u64_config.h +++ b/software/u64/u64_config.h @@ -161,7 +161,7 @@ class U64Config : public ConfigurableObject, ObjectWithMenu, SubSystem static void get_sid_addresses(ConfigStore *cfg, uint8_t *base, uint8_t *mask, uint8_t *split); static void fix_splits(uint8_t *base, uint8_t *mask, uint8_t *split); static void list_palettes(ConfigItem *it, IndexedList& strings); - static void get_palette_rgb(uint8_t rgb[16][3]); + static uint16_t get_palette_rgb(uint8_t rgb[16][3]); static void set_palette_rgb(const uint8_t rgb[16][3]); static void set_palette_color(uint8_t index, const uint8_t rgb[3]); static void reset_palette(); diff --git a/tests/e2e/io/command_interface/uci_targets_test.py b/tests/e2e/io/command_interface/uci_targets_test.py index bc774757e..b635dd707 100755 --- a/tests/e2e/io/command_interface/uci_targets_test.py +++ b/tests/e2e/io/command_interface/uci_targets_test.py @@ -45,6 +45,7 @@ import ftplib import json import sys +import threading import time import urllib.error import urllib.parse @@ -55,6 +56,7 @@ sys.path.insert(0, str(next(p for p in Path(__file__).resolve().parents if (p / "tests" / "lib").is_dir()) / "tests" / "lib")) import bootstrap # noqa: E402,F401 +import assembler # noqa: E402 import cli # noqa: E402 import ftp as ftp_lib import rest as rest_lib @@ -122,6 +124,11 @@ CTRL_CMD_RESET_PALETTE = 0x54 VIC_PALETTE_PACKET_SIZE = 60 VIC_PALETTE_LINE = 239 +VIC_VIDEO_PACKET_SIZE = 780 +PALETTE_BURST_SOURCE = Path(__file__).with_name("vic_palette_burst.asm") +PALETTE_BURST_STATUS = 0xC000 +PALETTE_BURST_RUNNING = 0xA5 +PALETTE_BURST_DONE = 0x5A SOFTIEC_CMD_IDENTIFY = 0x01 SOFTIEC_CMD_LOAD_SU = 0x10 SOFTIEC_CMD_GET_FATNAME = 0x22 @@ -256,12 +263,15 @@ def __init__(self, host: str, password: str | None, timeout: float) -> None: self.timeout = timeout def request(self, method: str, path: str, params: dict[str, object] | None = None, - repeatable: bool = False, host: str | None = None) -> tuple[int, bytes]: + repeatable: bool = False, host: str | None = None, + data: bytes | None = None) -> tuple[int, bytes]: url = f"http://{host or self.target.host_for(path)}{path}" if params: url += "?" + urllib.parse.urlencode(params) headers = {"X-Password": self.password} if self.password else {} - request = urllib.request.Request(url, headers=headers, method=method) + if data is not None: + headers["Content-Type"] = "application/octet-stream" + request = urllib.request.Request(url, data=data, headers=headers, method=method) # Transport and retry policy come from tests/lib/rest.py; see # rest.may_retry. `repeatable` is this suite's word for idempotent: a # register read applies nothing, so it may go again after the request @@ -280,14 +290,17 @@ def request(self, method: str, path: str, params: dict[str, object] | None = Non except (OSError, TimeoutError, urllib.error.URLError) as exc: raise Failure(f"{method} {url} failed: {format_exception(exc)}") from exc - def peek(self, address: int, repeatable: bool = False) -> int: + def readmem(self, address: int, length: int, repeatable: bool = False) -> bytes: status, body = self.request( - "GET", READMEM_PATH, params={"address": f"{address:04x}", "length": 1}, + "GET", READMEM_PATH, params={"address": f"{address:04x}", "length": length}, repeatable=repeatable, host=self.register_host ) - if status != 200 or len(body) != 1: + if status != 200 or len(body) != length: raise Failure(f"readmem(${address:04X}) failed with HTTP {status}: {body[:200]!r}") - return body[0] + return body + + def peek(self, address: int, repeatable: bool = False) -> int: + return self.readmem(address, 1, repeatable)[0] def poke(self, address: int, value: int) -> None: status, body = self.request( @@ -332,6 +345,11 @@ def reset(self) -> None: if status != 200: raise Failure(f"reset failed with HTTP {status}: {body[:200]!r}") + def run_prg(self, program: bytes) -> None: + status, body = self.request("POST", "/v1/runners:run_prg", data=program) + if status != 200: + raise Failure(f"runners:run_prg returned HTTP {status}: {body[:200]!r}") + class FtpFixture: """Files this suite puts on the device, over FTP because REST cannot delete.""" @@ -651,6 +669,50 @@ def expect_no_palette_packet(sock, addresses: set[str], accept_any_source: bool raise Failure("VIC stream sent palette data without an explicit palette request") +def capture_palette_burst(session: RestSession, sock, addresses: set[str], + accept_any_source: bool) -> tuple[list[tuple[float, int, bytes]], list[int], int, float]: + """Run the 6502 burst fixture while continuously draining the video socket.""" + samples: list[tuple[float, int, bytes]] = [] + video_sequences: list[int] = [] + stop = threading.Event() + + def receive() -> None: + while not stop.is_set(): + for _, packet, mine in stream_lib.receive([sock], addresses, 0.05): + if not mine and not accept_any_source: + continue + now = time.monotonic() + if (len(packet) == VIC_PALETTE_PACKET_SIZE and + int.from_bytes(packet[4:6], "little") == VIC_PALETTE_LINE and + packet[6:12] == bytes([0x80, 0x01, 1, 4, 1, 0])): + samples.append((now, int.from_bytes(packet[:2], "little"), packet[12:])) + elif len(packet) == VIC_VIDEO_PACKET_SIZE: + video_sequences.append(int.from_bytes(packet[:2], "little")) + + program = assembler.assemble(PALETTE_BURST_SOURCE) + session.poke(PALETTE_BURST_STATUS, 0) + receiver = threading.Thread(target=receive, daemon=True) + receiver.start() + started = time.monotonic() + try: + session.run_prg(program) + deadline = started + 15.0 + while True: + result = session.readmem(PALETTE_BURST_STATUS, 3, repeatable=True) + if result[0] == PALETTE_BURST_DONE: + break + if result[0] not in (0, PALETTE_BURST_RUNNING): + raise Failure(f"palette burst reported unexpected status ${result[0]:02X}") + if time.monotonic() >= deadline: + raise Failure("palette burst did not finish within 15 seconds") + time.sleep(0.05) + time.sleep(0.1) # Include the rate-limited packet for the final change. + finally: + stop.set() + receiver.join(timeout=1.0) + return samples, video_sequences, int.from_bytes(result[1:3], "little"), time.monotonic() - started + + def run_palette(session: RestSession, uci: Uci) -> bool: """Exercise the U64 runtime palette protocol without changing saved config.""" scenario = "palette" @@ -688,9 +750,18 @@ def run_palette(session: RestSession, uci: Uci) -> bool: sock = stream_lib.stream_socket(group, port) stream_started = False try: - with check(f"{scenario}: ordinary VIC stream sends no palette packets"): + with check(f"{scenario}: invalid palette stream options are rejected"): + for stream, value in (("video", 2), ("audio", 1), ("audio", 0)): + status, body = session.request( + "PUT", f"/v1/streams/{stream}:start", + params={"ip": f"{group}:{port}", "palette": value}) + if status != 400: + raise Failure( + f"{stream} stream with palette={value} returned HTTP {status}: {body[:200]!r}") + + with check(f"{scenario}: palette=0 sends no palette packets"): status, body = session.request( - "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}"}) + "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}", "palette": 0}) if status != 200: raise Failure(f"video stream start returned HTTP {status}: {body[:200]!r}") stream_started = True @@ -728,6 +799,63 @@ def run_palette(session: RestSession, uci: Uci) -> bool: raise Failure( f"palette repeat generation {repeated_generation} differs from initial {generation}") + with check(f"{scenario}: rapid changes are coalesced without disturbing video"): + # Empty anything queued after the repeat above before measuring the burst. + tuple(stream_lib.receive([sock], addresses, 0.05)) + samples, video_sequences, command_count, seconds = capture_palette_burst( + session, sock, addresses, accept_any_source) + if command_count < 2 or len(samples) < 2: + raise Failure(f"palette burst completed {command_count} changes but streamed {len(samples)} packets") + generation_delta = (samples[-1][1] - generation) & 0xFFFF + # runners:run_prg resets the C64 before loading; depending on the + # active machine settings that reset may also reapply the palette. + if generation_delta not in (command_count, command_count + 1): + raise Failure( + f"{command_count} changes advanced the generation by {generation_delta}") + reset_changes = generation_delta - command_count + for _, sample_generation, sample_palette in samples: + change = ((sample_generation - generation) & 0xFFFF) - reset_changes + if 1 <= change <= command_count: + index = change - 1 + expected = bytes(((13 * index) & 0xFF, + (85 + 29 * index) & 0xFF, + (170 + 47 * index) & 0xFF)) + if sample_palette[18:21] != expected: + raise Failure( + f"generation {sample_generation} carried color {sample_palette[18:21]!r}, " + f"expected {expected!r}") + last = command_count - 1 + expected_color = bytes(((13 * last) & 0xFF, + (85 + 29 * last) & 0xFF, + (170 + 47 * last) & 0xFF)) + readback, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) + if text != STATUS_OK or samples[-1][2] != readback: + raise Failure("final streamed palette did not match the device readback") + if readback[18:21] != expected_color: + raise Failure(f"rapid-change color was {readback[18:21]!r}, expected {expected_color!r}") + distinct = [sample for i, sample in enumerate(samples) + if i == 0 or sample[1] != samples[i - 1][1]] + if len(distinct) >= command_count: + raise Failure(f"{command_count} changes produced {len(distinct)} distinct packets; no coalescing") + intervals = [current[0] - previous[0] for previous, current in zip(samples, samples[1:])] + sample_span = samples[-1][0] - samples[0][0] + packet_rate = (len(samples) - 1) / sample_span if sample_span > 0 else float("inf") + # Host scheduling can timestamp two already-queued UDP datagrams + # close together, so sustained rate is the stable wire-rate check. + if packet_rate > 55.0: + raise Failure(f"palette stream sustained {packet_rate:.1f} packets/s, expected at most 55") + discontinuities = sum( + 1 for previous, current in zip(video_sequences, video_sequences[1:]) + if ((current - previous) & 0xFFFF) != 1) + if len(video_sequences) < 100 or discontinuities: + raise Failure( + f"video during burst had {len(video_sequences)} packets and " + f"{discontinuities} sequence discontinuities") + detail(f"{command_count} palette changes in {seconds:.2f}s -> {len(distinct)} packets; " + f"{packet_rate:.1f} packets/s, minimum observed spacing " + f"{min(intervals) * 1000:.1f} ms; " + f"{len(video_sequences)} consecutive video packets") + expect(uci, f"{scenario}: color index 16 is rejected", bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE_COLOR, 16, 0, 0, 0]), STATUS_INVALID_PARAMS, reply=b"") diff --git a/tests/e2e/io/command_interface/vic_palette_burst.asm b/tests/e2e/io/command_interface/vic_palette_burst.asm new file mode 100644 index 000000000..20c4bd83e --- /dev/null +++ b/tests/e2e/io/command_interface/vic_palette_burst.asm @@ -0,0 +1,126 @@ +; Change VIC color 6 four times per frame through the Ultimate Command +; Interface. The host-side palette E2E test uses this to prove that firmware +; coalesces a burst without disturbing the ordinary video packet sequence. + +CTRL = $DF1C +CMDREG = $DF1D +STATR = $DF1F + +ST_STATE = $30 +ST_LAST = $20 +ST_STAT = $40 + +STATUS = $C000 ; $A5 while running, $5A when complete +COUNT = $C001 ; completed commands, little endian + +FRAMES = 60 +CHANGES_PER_FRAME = 4 + + * = $0801 + +; BASIC line "10 SYS 2061". + .word basic_end, 10 + .byte $9e + .text "2061" + .byte 0 +basic_end + .word 0 + +start + sei + lda #$A5 + sta STATUS + lda #$00 + sta COUNT + sta COUNT+1 + lda #FRAMES + sta frames_left + +frame_loop + jsr wait_frame + lda #CHANGES_PER_FRAME + sta changes_left +change_loop + jsr set_color + inc red + lda red + clc + adc #12 ; net +13 including INC + sta red + lda green + clc + adc #29 + sta green + lda blue + clc + adc #47 + sta blue + inc COUNT + bne count_done + inc COUNT+1 +count_done + dec changes_left + bne change_loop + dec frames_left + bne frame_loop + + lda #$5A + sta STATUS + cli + rts + +; Wait for raster line zero in the low half. $D012 also reads zero at line 256, +; so $D011 bit 7 distinguishes the real frame boundary. +wait_frame +wait_away + bit $D011 + bmi wait_away + lda $D012 + beq wait_away +wait_zero + bit $D011 + bmi wait_zero + lda $D012 + bne wait_zero + rts + +set_color +wait_idle + lda CTRL + and #ST_STATE + bne wait_idle + lda #$04 + sta CMDREG + lda #$53 + sta CMDREG + lda #$06 + sta CMDREG + lda red + sta CMDREG + lda green + sta CMDREG + lda blue + sta CMDREG + lda #$01 + sta CTRL +wait_reply + lda CTRL + and #ST_STATE + cmp #ST_LAST + bne wait_reply +drain_status + lda CTRL + and #ST_STAT + beq accept_reply + lda STATR + jmp drain_status +accept_reply + lda #$02 + sta CTRL + rts + +frames_left .byte 0 +changes_left .byte 0 +red .byte 0 +green .byte 85 +blue .byte 170 From b028d12a47325a7b039ebe39cdad470e83bcc78a Mon Sep 17 00:00:00 2001 From: Barry Walker Date: Mon, 7 Sep 2026 13:17:16 -0400 Subject: [PATCH 07/13] fix(streams): keep palette task polling A conditional indefinite wait can miss a rapid stop/start transition and leave runtime palette delivery asleep. Retain the proven one-second poll while preserving notification coalescing. --- software/io/network/data_streamer.cc | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/software/io/network/data_streamer.cc b/software/io/network/data_streamer.cc index e24b9e8be..34b89ecb5 100644 --- a/software/io/network/data_streamer.cc +++ b/software/io/network/data_streamer.cc @@ -277,9 +277,6 @@ SubsysResultCode_e DataStreamer :: stopStream(SubsysCommand *cmd) taskENTER_CRITICAL(); palette_stream_requested = false; taskEXIT_CRITICAL(); - if (palette_task_handle) { - xTaskNotifyGive(palette_task_handle); - } } calculate_udp_headers(streamID); return SSRET_OK; @@ -354,11 +351,6 @@ bool DataStreamer :: sendVicPalette() taskEXIT_CRITICAL(); if (!requested || !source_ip || !dest_ip || !dest_port) { - if (palette_socket >= 0) { - lwip_close(palette_socket); - palette_socket = -1; - palette_socket_ip = 0; - } return false; } @@ -432,10 +424,7 @@ void DataStreamer :: paletteTask() TickType_t last_send = 0; while (true) { - taskENTER_CRITICAL(); - const bool requested = palette_stream_requested && streams[0].enable; - taskEXIT_CRITICAL(); - const uint32_t notified = ulTaskNotifyTake(pdTRUE, requested ? repeat_ticks : portMAX_DELAY); + const uint32_t notified = ulTaskNotifyTake(pdTRUE, repeat_ticks); if (notified && last_send) { const TickType_t now = xTaskGetTickCount(); const TickType_t elapsed = now - last_send; From b02f075a519143b5008884cd074331fb168a55f0 Mon Sep 17 00:00:00 2001 From: Barry Walker Date: Mon, 7 Sep 2026 14:40:35 -0400 Subject: [PATCH 08/13] test(streams): cover rapid palette updates --- .../io/command_interface/uci_targets_test.py | 151 +++++++++++------- .../command_interface/vic_palette_burst.asm | 17 +- 2 files changed, 110 insertions(+), 58 deletions(-) diff --git a/tests/e2e/io/command_interface/uci_targets_test.py b/tests/e2e/io/command_interface/uci_targets_test.py index b635dd707..15d6e6559 100755 --- a/tests/e2e/io/command_interface/uci_targets_test.py +++ b/tests/e2e/io/command_interface/uci_targets_test.py @@ -44,8 +44,8 @@ import argparse import ftplib import json +import socket import sys -import threading import time import urllib.error import urllib.parse @@ -127,8 +127,10 @@ VIC_VIDEO_PACKET_SIZE = 780 PALETTE_BURST_SOURCE = Path(__file__).with_name("vic_palette_burst.asm") PALETTE_BURST_STATUS = 0xC000 +PALETTE_BURST_READY = 0xA4 PALETTE_BURST_RUNNING = 0xA5 PALETTE_BURST_DONE = 0x5A +PALETTE_BURST_GO = 0xC003 SOFTIEC_CMD_IDENTIFY = 0x01 SOFTIEC_CMD_LOAD_SU = 0x10 SOFTIEC_CMD_GET_FATNAME = 0x22 @@ -648,8 +650,9 @@ def run_control_target(uci: Uci) -> bool: def expect_palette_packet(sock, addresses: set[str], expected: bytes, - accept_any_source: bool = False) -> int: - for _, packet, mine in stream_lib.receive([sock], addresses, 2.0): + accept_any_source: bool = False, timeout: float = 2.0) -> int: + last_unexpected = None + for _, packet, mine in stream_lib.receive([sock], addresses, timeout): if (not mine and not accept_any_source) or len(packet) != VIC_PALETTE_PACKET_SIZE: continue if int.from_bytes(packet[4:6], "little") != VIC_PALETTE_LINE: @@ -657,9 +660,12 @@ def expect_palette_packet(sock, addresses: set[str], expected: bytes, if packet[6:12] != bytes([0x80, 0x01, 1, 4, 1, 0]): raise Failure(f"palette stream packet has invalid format bytes {packet[6:12]!r}") if packet[12:] != expected: - raise Failure(f"palette stream packet was {packet[12:]!r}, expected {expected!r}") + last_unexpected = packet[12:] + continue return int.from_bytes(packet[:2], "little") - raise Failure("no palette packet arrived on the VIC stream within 2 seconds") + if last_unexpected is not None: + raise Failure(f"last palette packet was {last_unexpected!r}, expected {expected!r}") + raise Failure(f"no palette packet arrived on the VIC stream within {timeout:g} seconds") def expect_no_palette_packet(sock, addresses: set[str], accept_any_source: bool = False) -> None: @@ -674,42 +680,31 @@ def capture_palette_burst(session: RestSession, sock, addresses: set[str], """Run the 6502 burst fixture while continuously draining the video socket.""" samples: list[tuple[float, int, bytes]] = [] video_sequences: list[int] = [] - stop = threading.Event() - - def receive() -> None: - while not stop.is_set(): - for _, packet, mine in stream_lib.receive([sock], addresses, 0.05): - if not mine and not accept_any_source: - continue - now = time.monotonic() - if (len(packet) == VIC_PALETTE_PACKET_SIZE and - int.from_bytes(packet[4:6], "little") == VIC_PALETTE_LINE and - packet[6:12] == bytes([0x80, 0x01, 1, 4, 1, 0])): - samples.append((now, int.from_bytes(packet[:2], "little"), packet[12:])) - elif len(packet) == VIC_VIDEO_PACKET_SIZE: - video_sequences.append(int.from_bytes(packet[:2], "little")) - - program = assembler.assemble(PALETTE_BURST_SOURCE) - session.poke(PALETTE_BURST_STATUS, 0) - receiver = threading.Thread(target=receive, daemon=True) - receiver.start() + + def receive(timeout: float) -> None: + for _, packet, mine in stream_lib.receive([sock], addresses, timeout): + if not mine and not accept_any_source: + continue + now = time.monotonic() + if (len(packet) == VIC_PALETTE_PACKET_SIZE and + int.from_bytes(packet[4:6], "little") == VIC_PALETTE_LINE and + packet[6:12] == bytes([0x80, 0x01, 1, 4, 1, 0])): + samples.append((now, int.from_bytes(packet[:2], "little"), packet[12:])) + elif len(packet) == VIC_VIDEO_PACKET_SIZE: + video_sequences.append(int.from_bytes(packet[:2], "little")) + started = time.monotonic() - try: - session.run_prg(program) - deadline = started + 15.0 - while True: - result = session.readmem(PALETTE_BURST_STATUS, 3, repeatable=True) - if result[0] == PALETTE_BURST_DONE: - break - if result[0] not in (0, PALETTE_BURST_RUNNING): - raise Failure(f"palette burst reported unexpected status ${result[0]:02X}") - if time.monotonic() >= deadline: - raise Failure("palette burst did not finish within 15 seconds") - time.sleep(0.05) - time.sleep(0.1) # Include the rate-limited packet for the final change. - finally: - stop.set() - receiver.join(timeout=1.0) + session.poke(PALETTE_BURST_GO, 1) + deadline = started + 15.0 + while True: + receive(0.05) + result = session.readmem(PALETTE_BURST_STATUS, 3, repeatable=True) + if result[0] == PALETTE_BURST_DONE: + break + if result[0] != PALETTE_BURST_RUNNING: + raise Failure(f"palette burst reported unexpected status ${result[0]:02X}") + if time.monotonic() >= deadline: + raise Failure("palette burst did not finish within 15 seconds") return samples, video_sequences, int.from_bytes(result[1:3], "little"), time.monotonic() - started @@ -748,6 +743,7 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if not addresses: raise Failure(f"{scenario}: could not resolve the VIC stream source address") sock = stream_lib.stream_socket(group, port) + sock.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, 4 * 1024 * 1024) stream_started = False try: with check(f"{scenario}: invalid palette stream options are rejected"): @@ -794,27 +790,59 @@ def run_palette(session: RestSession, uci: Uci) -> bool: generation = expect_palette_packet(sock, addresses, bytes(changed), accept_any_source) with check(f"{scenario}: opted-in VIC stream periodically repeats its palette"): - repeated_generation = expect_palette_packet(sock, addresses, bytes(changed), accept_any_source) - if repeated_generation != generation: - raise Failure( - f"palette repeat generation {repeated_generation} differs from initial {generation}") + for _ in range(2): + repeated_generation = expect_palette_packet( + sock, addresses, bytes(changed), accept_any_source, timeout=3.0) + if repeated_generation != generation: + raise Failure( + f"palette repeat generation {repeated_generation} differs from initial {generation}") with check(f"{scenario}: rapid changes are coalesced without disturbing video"): - # Empty anything queued after the repeat above before measuring the burst. + session.poke(PALETTE_BURST_GO, 0) + session.poke(PALETTE_BURST_STATUS, 0) + session.run_prg(assembler.assemble(PALETTE_BURST_SOURCE)) + deadline = time.monotonic() + 5.0 + while session.peek(PALETTE_BURST_STATUS, repeatable=True) != PALETTE_BURST_READY: + if time.monotonic() >= deadline: + raise Failure("palette burst fixture did not reach its ready state") + time.sleep(0.05) + burst_palette, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) + if text != STATUS_OK: + raise Failure(f"GET_PALETTE before burst returned {text!r}") + status, body = session.request( + "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}", "palette": 1}) + if status != 200: + raise Failure(f"video stream restart returned HTTP {status}: {body[:200]!r}") + generation = expect_palette_packet( + sock, addresses, burst_palette, accept_any_source) + # Empty video queued around the stream restart before measuring. tuple(stream_lib.receive([sock], addresses, 0.05)) samples, video_sequences, command_count, seconds = capture_palette_burst( session, sock, addresses, accept_any_source) if command_count < 2 or len(samples) < 2: - raise Failure(f"palette burst completed {command_count} changes but streamed {len(samples)} packets") - generation_delta = (samples[-1][1] - generation) & 0xFFFF - # runners:run_prg resets the C64 before loading; depending on the - # active machine settings that reset may also reapply the palette. - if generation_delta not in (command_count, command_count + 1): + raise Failure( + f"palette burst completed {command_count} changes but captured " + f"{len(samples)} palette and {len(video_sequences)} video packets") + readback, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) + if text != STATUS_OK: + raise Failure(f"GET_PALETTE after burst returned {text!r}") + tuple(stream_lib.receive([sock], addresses, 0.05)) + status, body = session.request( + "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}", "palette": 1}) + if status != 200: + raise Failure(f"video stream restart returned HTTP {status}: {body[:200]!r}") + final_generation = expect_palette_packet( + sock, addresses, readback, accept_any_source) + session.poke(PALETTE_BURST_GO, 2) + generation_delta = (final_generation - generation) & 0xFFFF + detail(f"captured {len(samples)} palette and {len(video_sequences)} video packets; " + f"last live generation delta {(samples[-1][1] - generation) & 0xFFFF}, " + f"final delta {generation_delta}") + if generation_delta != command_count: raise Failure( f"{command_count} changes advanced the generation by {generation_delta}") - reset_changes = generation_delta - command_count for _, sample_generation, sample_palette in samples: - change = ((sample_generation - generation) & 0xFFFF) - reset_changes + change = (sample_generation - generation) & 0xFFFF if 1 <= change <= command_count: index = change - 1 expected = bytes(((13 * index) & 0xFF, @@ -828,9 +856,6 @@ def run_palette(session: RestSession, uci: Uci) -> bool: expected_color = bytes(((13 * last) & 0xFF, (85 + 29 * last) & 0xFF, (170 + 47 * last) & 0xFF)) - readback, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) - if text != STATUS_OK or samples[-1][2] != readback: - raise Failure("final streamed palette did not match the device readback") if readback[18:21] != expected_color: raise Failure(f"rapid-change color was {readback[18:21]!r}, expected {expected_color!r}") distinct = [sample for i, sample in enumerate(samples) @@ -855,6 +880,17 @@ def run_palette(session: RestSession, uci: Uci) -> bool: f"{packet_rate:.1f} packets/s, minimum observed spacing " f"{min(intervals) * 1000:.1f} ms; " f"{len(video_sequences)} consecutive video packets") + session.reset() + uci.release() + resumed_palette, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) + if text != STATUS_OK: + raise Failure(f"GET_PALETTE after fixture cleanup returned {text!r}") + status, body = session.request( + "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}", "palette": 1}) + if status != 200: + raise Failure(f"video stream resume returned HTTP {status}: {body[:200]!r}") + generation = expect_palette_packet( + sock, addresses, resumed_palette, accept_any_source) expect(uci, f"{scenario}: color index 16 is rejected", bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE_COLOR, 16, 0, 0, 0]), @@ -905,6 +941,7 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if status != 200: raise Failure(f"video stream stop returned HTTP {status}: {body[:200]!r}") stream_started = False + tuple(stream_lib.receive([sock], addresses, 0.05)) status, body = session.request( "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}"}) if status != 200: @@ -913,6 +950,10 @@ def run_palette(session: RestSession, uci: Uci) -> bool: expect_no_palette_packet(sock, addresses, accept_any_source) finally: try: + try: + session.poke(PALETTE_BURST_GO, 2) + except Failure: + pass with check(f"{scenario}: restore the original runtime palette"): reply, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE]) + original) if text != STATUS_OK or reply: diff --git a/tests/e2e/io/command_interface/vic_palette_burst.asm b/tests/e2e/io/command_interface/vic_palette_burst.asm index 20c4bd83e..c229c559a 100644 --- a/tests/e2e/io/command_interface/vic_palette_burst.asm +++ b/tests/e2e/io/command_interface/vic_palette_burst.asm @@ -10,8 +10,9 @@ ST_STATE = $30 ST_LAST = $20 ST_STAT = $40 -STATUS = $C000 ; $A5 while running, $5A when complete +STATUS = $C000 ; $A4 ready, $A5 running, $5A complete COUNT = $C001 ; completed commands, little endian +GO = $C003 ; host writes nonzero to release the burst FRAMES = 60 CHANGES_PER_FRAME = 4 @@ -28,11 +29,17 @@ basic_end start sei - lda #$A5 - sta STATUS lda #$00 sta COUNT sta COUNT+1 + sta GO + lda #$A4 + sta STATUS +wait_go + lda GO + beq wait_go + lda #$A5 + sta STATUS lda #FRAMES sta frames_left @@ -67,6 +74,10 @@ count_done lda #$5A sta STATUS cli +wait_exit + lda GO + cmp #$02 + bne wait_exit rts ; Wait for raster line zero in the low half. $D012 also reads zero at line 256, From 82a8d372422252d1cb4e6ef812f61902aa4b6a87 Mon Sep 17 00:00:00 2001 From: Barry Walker Date: Mon, 7 Sep 2026 14:52:20 -0400 Subject: [PATCH 09/13] test(streams): use pairwise for packet checks --- tests/e2e/io/command_interface/uci_targets_test.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/e2e/io/command_interface/uci_targets_test.py b/tests/e2e/io/command_interface/uci_targets_test.py index 15d6e6559..9d7e5870d 100755 --- a/tests/e2e/io/command_interface/uci_targets_test.py +++ b/tests/e2e/io/command_interface/uci_targets_test.py @@ -43,6 +43,7 @@ import argparse import ftplib +import itertools import json import socket import sys @@ -862,7 +863,8 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if i == 0 or sample[1] != samples[i - 1][1]] if len(distinct) >= command_count: raise Failure(f"{command_count} changes produced {len(distinct)} distinct packets; no coalescing") - intervals = [current[0] - previous[0] for previous, current in zip(samples, samples[1:])] + intervals = [current[0] - previous[0] + for previous, current in itertools.pairwise(samples)] sample_span = samples[-1][0] - samples[0][0] packet_rate = (len(samples) - 1) / sample_span if sample_span > 0 else float("inf") # Host scheduling can timestamp two already-queued UDP datagrams @@ -870,7 +872,7 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if packet_rate > 55.0: raise Failure(f"palette stream sustained {packet_rate:.1f} packets/s, expected at most 55") discontinuities = sum( - 1 for previous, current in zip(video_sequences, video_sequences[1:]) + 1 for previous, current in itertools.pairwise(video_sequences) if ((current - previous) & 0xFFFF) != 1) if len(video_sequences) < 100 or discontinuities: raise Failure( From e94efbf7250535efe9022f9fb762d5fad525369a Mon Sep 17 00:00:00 2001 From: Barry Walker Date: Mon, 7 Sep 2026 17:13:01 -0400 Subject: [PATCH 10/13] fix(streams): bind palette source address --- software/io/network/data_streamer.cc | 26 +++++++++---------- .../io/command_interface/uci_targets_test.py | 9 +++---- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/software/io/network/data_streamer.cc b/software/io/network/data_streamer.cc index 34b89ecb5..2514feecf 100644 --- a/software/io/network/data_streamer.cc +++ b/software/io/network/data_streamer.cc @@ -354,9 +354,9 @@ bool DataStreamer :: sendVicPalette() return false; } - // LwIP may route unicast over another active interface, so let it choose - // that packet's source. Multicast keeps the VIC source for client filtering. - const uint32_t socket_ip = (dest_ip & 0x000000F8) == 0x000000E8 ? source_ip : 0; + // Match the FPGA VIC stream source so receivers can apply the same peer + // filter to video and palette packets. + const uint32_t socket_ip = source_ip; if ((palette_socket < 0) || (palette_socket_ip != socket_ip)) { if (palette_socket >= 0) { lwip_close(palette_socket); @@ -366,16 +366,14 @@ bool DataStreamer :: sendVicPalette() if (palette_socket < 0) { return false; } - if (socket_ip) { - struct sockaddr_in local; - memset(&local, 0, sizeof(local)); - local.sin_family = AF_INET; - local.sin_addr.s_addr = socket_ip; - if (bind(palette_socket, (const struct sockaddr *)&local, sizeof(local)) < 0) { - lwip_close(palette_socket); - palette_socket = -1; - return false; - } + struct sockaddr_in local; + memset(&local, 0, sizeof(local)); + local.sin_family = AF_INET; + local.sin_addr.s_addr = socket_ip; + if (bind(palette_socket, (const struct sockaddr *)&local, sizeof(local)) < 0) { + lwip_close(palette_socket); + palette_socket = -1; + return false; } palette_socket_ip = socket_ip; } @@ -390,7 +388,7 @@ bool DataStreamer :: sendVicPalette() packet[7] = 0x01; packet[8] = 1; // one palette packet[9] = 4; // four bits per VIC color index - packet[10] = 1; // RGB palette encoding + packet[10] = 1; // palette packet type indicator memcpy(packet + 12, rgb, sizeof(rgb)); struct sockaddr_in destination; diff --git a/tests/e2e/io/command_interface/uci_targets_test.py b/tests/e2e/io/command_interface/uci_targets_test.py index 9d7e5870d..2c10fd378 100755 --- a/tests/e2e/io/command_interface/uci_targets_test.py +++ b/tests/e2e/io/command_interface/uci_targets_test.py @@ -672,7 +672,8 @@ def expect_palette_packet(sock, addresses: set[str], expected: bytes, def expect_no_palette_packet(sock, addresses: set[str], accept_any_source: bool = False) -> None: for _, packet, mine in stream_lib.receive([sock], addresses, 0.25): if ((mine or accept_any_source) and len(packet) == VIC_PALETTE_PACKET_SIZE and - packet[10:12] == bytes([1, 0])): + int.from_bytes(packet[4:6], "little") == VIC_PALETTE_LINE and + packet[6:12] == bytes([0x80, 0x01, 1, 4, 1, 0])): raise Failure("VIC stream sent palette data without an explicit palette request") @@ -736,10 +737,8 @@ def run_palette(session: RestSession, uci: Uci) -> bool: group = session.target.video_group port = session.target.video_port - # A dual-homed Ultimate can send the FPGA video from one interface and the - # software palette packet from another. A unicast destination belongs only - # to this socket; multicast still needs source filtering between devices. - accept_any_source = not stream_lib.is_multicast(group) + # Palette metadata uses the same source address as the FPGA VIC stream. + accept_any_source = False addresses = stream_lib.source_addresses(session.target) if not addresses: raise Failure(f"{scenario}: could not resolve the VIC stream source address") From d07ec59490290a78966a0ee51ddcabaab6aa98a7 Mon Sep 17 00:00:00 2001 From: Barry Walker Date: Mon, 7 Sep 2026 17:40:38 -0400 Subject: [PATCH 11/13] fix(streams): restore unicast routing Binding the palette socket to the FPGA wired address drops unicast packets when a dual-homed Ultimate routes software UDP through Wi-Fi. Keep multicast bound to the FPGA source and let LwIP select the unicast interface. --- software/io/network/data_streamer.cc | 24 ++++++++++--------- .../io/command_interface/uci_targets_test.py | 6 +++-- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/software/io/network/data_streamer.cc b/software/io/network/data_streamer.cc index 2514feecf..6be867ce9 100644 --- a/software/io/network/data_streamer.cc +++ b/software/io/network/data_streamer.cc @@ -354,9 +354,9 @@ bool DataStreamer :: sendVicPalette() return false; } - // Match the FPGA VIC stream source so receivers can apply the same peer - // filter to video and palette packets. - const uint32_t socket_ip = source_ip; + // LwIP may route unicast over another active interface, so let it choose + // that packet's source. Multicast keeps the VIC source for client filtering. + const uint32_t socket_ip = (dest_ip & 0x000000F8) == 0x000000E8 ? source_ip : 0; if ((palette_socket < 0) || (palette_socket_ip != socket_ip)) { if (palette_socket >= 0) { lwip_close(palette_socket); @@ -366,14 +366,16 @@ bool DataStreamer :: sendVicPalette() if (palette_socket < 0) { return false; } - struct sockaddr_in local; - memset(&local, 0, sizeof(local)); - local.sin_family = AF_INET; - local.sin_addr.s_addr = socket_ip; - if (bind(palette_socket, (const struct sockaddr *)&local, sizeof(local)) < 0) { - lwip_close(palette_socket); - palette_socket = -1; - return false; + if (socket_ip) { + struct sockaddr_in local; + memset(&local, 0, sizeof(local)); + local.sin_family = AF_INET; + local.sin_addr.s_addr = socket_ip; + if (bind(palette_socket, (const struct sockaddr *)&local, sizeof(local)) < 0) { + lwip_close(palette_socket); + palette_socket = -1; + return false; + } } palette_socket_ip = socket_ip; } diff --git a/tests/e2e/io/command_interface/uci_targets_test.py b/tests/e2e/io/command_interface/uci_targets_test.py index 2c10fd378..80d3d6d78 100755 --- a/tests/e2e/io/command_interface/uci_targets_test.py +++ b/tests/e2e/io/command_interface/uci_targets_test.py @@ -737,8 +737,10 @@ def run_palette(session: RestSession, uci: Uci) -> bool: group = session.target.video_group port = session.target.video_port - # Palette metadata uses the same source address as the FPGA VIC stream. - accept_any_source = False + # A dual-homed Ultimate can send the FPGA video from one interface and the + # software palette packet from another. A unicast destination belongs only + # to this socket; multicast still needs source filtering between devices. + accept_any_source = not stream_lib.is_multicast(group) addresses = stream_lib.source_addresses(session.target) if not addresses: raise Failure(f"{scenario}: could not resolve the VIC stream source address") From be840ac131acd2ea1e13ea8d9ffd17446069e1e1 Mon Sep 17 00:00:00 2001 From: Barry Walker Date: Tue, 22 Sep 2026 18:25:11 -0400 Subject: [PATCH 12/13] fix(streams): harden runtime palette delivery after review Firmware: - Resolve a stream start into a copy and commit the source, destination, enable and palette opt-in together, so a failed start leaves the running stream and its palette packets untouched. - Carry the opt-in as a mode bit instead of the command's filename, which the task menu fills with the name of the highlighted browser entry. - Validate the palette parameter before the debug stream is stopped, so a rejected request changes nothing. - Bind the palette socket to the VIC stream's interface and its source port 53248, so palette and video arrive from one address and port for unicast and multicast alike. Connect it to the destination, drain it, and close it when the stream stops, so datagrams sent to it cannot hold lwIP's netbufs. - Keep the socket across transient send failures and pace failed attempts. - Program the palette registers inside the same critical section as the shadow copy and generation, so the VIC, GET_PALETTE and the stream agree. - Treat all of 224.0.0.0/4 as multicast, not only 232-239. - Space palette packets at least 20 ms apart and leave the palette task asleep when no stream has opted in. - Document that the opt-in is device-wide and the latest start sets it. Tests: - Move the stream checks into their own palette-stream scenario after the command interface scenarios, skipping when the firmware, the link or the -H address cannot carry the stream. - Drain the stream on a thread, judge the video by stalls rather than host drops, and use video as the positive control for the no-palette checks. - Require prompt palette packets on start, change and reset, and check the repeat interval. - Start the burst fixture only on GO=1 and reset before restoring the palette when a failure leaves it parked. - Share one palette parser in streams.py and pass stream start parameters through Arming. Refs #850 --- doc/api/rest_api_openapi_u64.yaml | 2 +- software/api/route_streams.cc | 20 +- software/io/network/data_streamer.cc | 215 +++--- software/io/network/data_streamer.h | 10 +- software/io/network/network_interface.cc | 7 + software/io/network/network_interface.h | 1 + software/u64/u64_config.cc | 8 +- .../io/command_interface/uci_targets_test.py | 621 +++++++++++------- .../command_interface/vic_palette_burst.asm | 6 +- tests/e2e/lib/streams.py | 27 +- 10 files changed, 565 insertions(+), 352 deletions(-) diff --git a/doc/api/rest_api_openapi_u64.yaml b/doc/api/rest_api_openapi_u64.yaml index ef153f057..85818528a 100644 --- a/doc/api/rest_api_openapi_u64.yaml +++ b/doc/api/rest_api_openapi_u64.yaml @@ -3378,7 +3378,7 @@ paths: - name: palette in: query required: false - description: For video, request runtime VIC palette packets (0 or 1). + description: 'For video, request runtime VIC palette packets (0 or 1). The setting is device-wide: it applies to every receiver of the stream, and the most recent start sets it.' schema: type: integer example: 0 diff --git a/software/api/route_streams.cc b/software/api/route_streams.cc index 1701957e5..78015645b 100644 --- a/software/api/route_streams.cc +++ b/software/api/route_streams.cc @@ -31,7 +31,7 @@ API_DOC(PUT, streams, start, PATH_PARAM("stream", "string", "Which stream to act on.", "video") PATH_PARAM_ENUM("stream", "video,audio,debug") PARAM("ip", "string", "Where to send the stream. An address, optionally followed by a port.", "", "192.168.1.10:11000") - PARAM("palette", "integer", "For video, request runtime VIC palette packets (0 or 1).", "", "0") + PARAM("palette", "integer", "For video, request runtime VIC palette packets (0 or 1). The setting is device-wide: it applies to every receiver of the stream, and the most recent start sets it.", "", "0") RESPONSE("200", "application/json", "ErrorResponse", "The stream is running.", "") RESPONSE_ERROR("400", "Palette must be 0 or 1 and is only valid for the video stream", "") RESPONSE_ERROR("404", "Unrecognized stream name 'screen'", "") @@ -54,12 +54,7 @@ API_CALL(PUT, streams, start, NULL, ARRAY ( { { "ip", P_REQUIRED }, { "palette", return; } - if (streamIndex == 0) { // video streams require debug to be off - sys_command = new SubsysCommand(NULL, -1, (int)dataStreamer, 2, "", ""); - sys_command->direct_call = DataStreamer :: S_stopStream; - sys_command->execute(); - } - + // Checked before anything is stopped, so a rejected request changes nothing. const char *paletteArg = args.get_or("palette", NULL); const bool paletteRequested = paletteArg && strcmp(paletteArg, "1") == 0; if (paletteArg && (streamIndex != 0 || (strcmp(paletteArg, "0") != 0 && !paletteRequested))) { @@ -67,8 +62,15 @@ API_CALL(PUT, streams, start, NULL, ARRAY ( { { "ip", P_REQUIRED }, { "palette", resp->json_response(HTTP_BAD_REQUEST); return; } - const char *palette = paletteRequested ? "1" : ""; - sys_command = new SubsysCommand(NULL, -1, (int)dataStreamer, streamIndex, args["ip"], palette); + + if (streamIndex == 0) { // video streams require debug to be off + sys_command = new SubsysCommand(NULL, -1, (int)dataStreamer, 2, "", ""); + sys_command->direct_call = DataStreamer :: S_stopStream; + sys_command->execute(); + } + + const int mode = streamIndex | (paletteRequested ? STREAM_MODE_PALETTE : 0); + sys_command = new SubsysCommand(NULL, -1, (int)dataStreamer, mode, args["ip"], ""); sys_command->direct_call = DataStreamer :: S_startStream; SubsysResultCode_t retval = sys_command->execute(); resp->error(SubsysCommand::error_string(retval.status)); diff --git a/software/io/network/data_streamer.cc b/software/io/network/data_streamer.cc index 6be867ce9..de0f2f7de 100644 --- a/software/io/network/data_streamer.cc +++ b/software/io/network/data_streamer.cc @@ -42,7 +42,8 @@ DataStreamer :: DataStreamer() palette_stream_requested = false; palette_task_handle = NULL; palette_socket = -1; - palette_socket_ip = 0; + palette_socket_dest_ip = 0; + palette_socket_dest_port = 0; memset(streams, 0, 4*sizeof(stream_config_t)); cfg = ConfigManager :: getConfigManager()->register_store(0x44617461, "Data Streams", stream_cfg, NULL); @@ -62,9 +63,7 @@ DataStreamer :: DataStreamer() // This should never be called DataStreamer :: ~DataStreamer() { - if (palette_socket >= 0) { - lwip_close(palette_socket); - } + closePaletteSocket(); } DataStreamer *dataStreamer; @@ -94,6 +93,10 @@ void DataStreamer :: S_timer(TimerHandle_t a) if ((streamID >= 0) && (streamID <= 3)) { stream_config_t *stream = &(dataStreamer->streams[streamID]); stream->enable = 0; + if (streamID == 0) { + dataStreamer->palette_stream_requested = false; + dataStreamer->wakePaletteTask(); + } dataStreamer->calculate_udp_headers(streamID); } } @@ -115,12 +118,16 @@ SubsysResultCode_e DataStreamer :: startStream(SubsysCommand *cmd) return SSRET_NO_NETWORK; // shouldn't happen } - int streamID = cmd->mode; + int streamID = cmd->mode & 0xFF; if ((streamID < 0) || (streamID > 3)) { if (cmd->user_interface) cmd->user_interface->popup("Invalid Stream ID", BUTTON_OK); return SSRET_INVALID_PARAMETER; } + const bool palette = (streamID == 0) && (cmd->mode & STREAM_MODE_PALETTE); stream_config_t *stream = &streams[streamID]; + // Resolve into a copy. Until this start succeeds, the running stream and + // the palette task keep the destination they have. + stream_config_t next = *stream; union { uint32_t ipaddr32[3]; @@ -129,11 +136,12 @@ SubsysResultCode_e DataStreamer :: startStream(SubsysCommand *cmd) uint8_t his_mac[6]; memset(his_mac, 0, 6); + uint8_t source_mac[6]; intf->getIpAddr(ip.ipaddr); - intf->getMacAddr(my_mac); - my_ip = ip.ipaddr32[0]; + intf->getMacAddr(source_mac); + const uint32_t source_ip = ip.ipaddr32[0]; - if (!(intf->is_link_up()) || (my_ip == 0)) { + if (!(intf->is_link_up()) || (source_ip == 0)) { if (cmd->user_interface) cmd->user_interface->popup("No (valid) link", BUTTON_OK); return SSRET_NO_NETWORK; } @@ -187,11 +195,11 @@ SubsysResultCode_e DataStreamer :: startStream(SubsysCommand *cmd) } uint32_t *addrs = (uint32_t *)*(ret_host->h_addr_list); - stream->dest_ip = addrs[0]; + next.dest_ip = addrs[0]; - uint32_t query_ip = stream->dest_ip; - if ((stream->dest_ip & ip.ipaddr32[1]) != (ip.ipaddr32[2] & ip.ipaddr32[1])) { - printf("You requested an external IP address (%08X)\n", stream->dest_ip); + uint32_t query_ip = next.dest_ip; + if ((next.dest_ip & ip.ipaddr32[1]) != (ip.ipaddr32[2] & ip.ipaddr32[1])) { + printf("You requested an external IP address (%08X)\n", next.dest_ip); query_ip = ip.ipaddr32[2]; } @@ -201,49 +209,51 @@ SubsysResultCode_e DataStreamer :: startStream(SubsysCommand *cmd) vic_dest_ip = vic_dest.addr; */ - stream->dest_port = 11000 + streamID; + next.dest_port = 11000 + streamID; if (behind_colon) { - sscanf(behind_colon, "%d", &stream->dest_port); + sscanf(behind_colon, "%d", &next.dest_port); } - if (!stream->dest_port) { + if (!next.dest_port) { if (cmd->user_interface) cmd->user_interface->popup("Destination Port cannot be 0.", BUTTON_OK); return SSRET_INVALID_PARAMETER; } // destination mac - if ((stream->dest_ip & 0x000000F8) == 0x000000E8) { + if ((next.dest_ip & 0x000000F0) == 0x000000E0) { printf("** User requested Multicast stream\n"); - } else if (stream->dest_ip == 0xFFFFFFFF) { + } else if (next.dest_ip == 0xFFFFFFFF) { printf("** User requested Broadcast stream\n"); } else { bool ok = false; for(int i=0;i<10;i++) { - send_udp_packet(query_ip, stream->dest_port); + send_udp_packet(query_ip, next.dest_port); vTaskDelay(20); - if (intf->peekArpTable(query_ip, stream->dest_mac)) { + if (intf->peekArpTable(query_ip, next.dest_mac)) { ok = true; break; } } if (ok) { - printf("** Your MAC address is %b:%b:%b:%b:%b:%b. Got ya!\n", stream->dest_mac[0], stream->dest_mac[1], stream->dest_mac[2], - stream->dest_mac[3], stream->dest_mac[4], stream->dest_mac[5]); + printf("** Your MAC address is %b:%b:%b:%b:%b:%b. Got ya!\n", next.dest_mac[0], next.dest_mac[1], next.dest_mac[2], + next.dest_mac[3], next.dest_mac[4], next.dest_mac[5]); } else { if (cmd->user_interface) cmd->user_interface->popup("Cannot find MAC of specified host.", BUTTON_OK); return SSRET_NETWORK_RESOLVE_ERROR; } } - stream->enable = 1; - + // All of it resolved, so commit it in one step: the palette task reads + // the destination, the enable and the opt-in together. + next.enable = 1; + taskENTER_CRITICAL(); + my_ip = source_ip; + memcpy(my_mac, source_mac, 6); + *stream = next; if (streamID == 0) { - const bool requested = strcmp(cmd->filename.c_str(), "1") == 0; - taskENTER_CRITICAL(); - palette_stream_requested = requested; - taskEXIT_CRITICAL(); - - if (requested && palette_task_handle) { - xTaskNotifyGive(palette_task_handle); - } + palette_stream_requested = palette; + } + taskEXIT_CRITICAL(); + if (streamID == 0) { + wakePaletteTask(); } // start stream! @@ -274,9 +284,8 @@ SubsysResultCode_e DataStreamer :: stopStream(SubsysCommand *cmd) stream_config_t *stream = &streams[streamID]; stream->enable = 0; if (streamID == 0) { - taskENTER_CRITICAL(); palette_stream_requested = false; - taskEXIT_CRITICAL(); + wakePaletteTask(); } calculate_udp_headers(streamID); return SSRET_OK; @@ -338,46 +347,76 @@ void DataStreamer :: send_udp_packet(uint32_t ip, uint16_t port) lwip_close(sockfd); } +bool DataStreamer :: openPaletteSocket() +{ + NetworkInterface *intf = NetworkInterface :: getInterface(0); + if (!intf) { + return false; + } + int sock = socket(AF_INET, SOCK_DGRAM, 0); + if (sock < 0) { + return false; + } + // Leave through the interface the FPGA VIC stream uses, from its source + // port, so palette and video arrive from one address and port. Left to + // itself, lwIP would route unicast over whichever interface it prefers. + struct ifreq iface; + memset(&iface, 0, sizeof(iface)); + intf->getNetifName(iface.ifr_name, sizeof(iface.ifr_name)); + struct sockaddr_in local; + memset(&local, 0, sizeof(local)); + local.sin_family = AF_INET; + local.sin_addr.s_addr = INADDR_ANY; + local.sin_port = htons(53248); + if ((setsockopt(sock, SOL_SOCKET, SO_BINDTODEVICE, &iface, sizeof(iface)) < 0) || + (bind(sock, (const struct sockaddr *)&local, sizeof(local)) < 0)) { + lwip_close(sock); + return false; + } + palette_socket = sock; + palette_socket_dest_ip = 0; + palette_socket_dest_port = 0; + return true; +} + +void DataStreamer :: closePaletteSocket() +{ + if (palette_socket >= 0) { + lwip_close(palette_socket); + palette_socket = -1; + } +} + +// Returns whether a send was attempted, so that failed attempts are paced too. bool DataStreamer :: sendVicPalette() { - uint32_t source_ip; - uint32_t dest_ip; - uint16_t dest_port; taskENTER_CRITICAL(); const bool requested = palette_stream_requested && streams[0].enable; - source_ip = my_ip; - dest_ip = streams[0].dest_ip; - dest_port = streams[0].dest_port; + const uint32_t dest_ip = streams[0].dest_ip; + const int dest_port = streams[0].dest_port; taskEXIT_CRITICAL(); - if (!requested || !source_ip || !dest_ip || !dest_port) { + if (!requested) { + closePaletteSocket(); return false; } - - // LwIP may route unicast over another active interface, so let it choose - // that packet's source. Multicast keeps the VIC source for client filtering. - const uint32_t socket_ip = (dest_ip & 0x000000F8) == 0x000000E8 ? source_ip : 0; - if ((palette_socket < 0) || (palette_socket_ip != socket_ip)) { - if (palette_socket >= 0) { - lwip_close(palette_socket); - } - palette_socket = socket(AF_INET, SOCK_DGRAM, 0); - palette_socket_ip = 0; - if (palette_socket < 0) { - return false; - } - if (socket_ip) { - struct sockaddr_in local; - memset(&local, 0, sizeof(local)); - local.sin_family = AF_INET; - local.sin_addr.s_addr = socket_ip; - if (bind(palette_socket, (const struct sockaddr *)&local, sizeof(local)) < 0) { - lwip_close(palette_socket); - palette_socket = -1; - return false; - } + if ((palette_socket < 0) && !openPaletteSocket()) { + return true; + } + // Connected, so lwIP accepts datagrams only from the destination, and + // drained below, so not even those can pin its few netbufs. + if ((palette_socket_dest_ip != dest_ip) || (palette_socket_dest_port != dest_port)) { + struct sockaddr_in destination; + memset(&destination, 0, sizeof(destination)); + destination.sin_family = AF_INET; + destination.sin_addr.s_addr = dest_ip; + destination.sin_port = htons(dest_port); + if (connect(palette_socket, (const struct sockaddr *)&destination, sizeof(destination)) < 0) { + closePaletteSocket(); + return true; } - palette_socket_ip = socket_ip; + palette_socket_dest_ip = dest_ip; + palette_socket_dest_port = dest_port; } uint8_t packet[60] = { 0 }; @@ -393,47 +432,53 @@ bool DataStreamer :: sendVicPalette() packet[10] = 1; // palette packet type indicator memcpy(packet + 12, rgb, sizeof(rgb)); - struct sockaddr_in destination; - memset(&destination, 0, sizeof(destination)); - destination.sin_family = AF_INET; - destination.sin_addr.s_addr = dest_ip; - destination.sin_port = htons(dest_port); - if (sendto(palette_socket, packet, sizeof(packet), 0, - (const struct sockaddr *)&destination, sizeof(destination)) < 0) { - lwip_close(palette_socket); - palette_socket = -1; - palette_socket_ip = 0; - return false; - } + // A failed send leaves nothing behind on a UDP socket; the next wake + // simply tries again. + send(palette_socket, packet, sizeof(packet), 0); + + uint8_t discard[16]; + while (recv(palette_socket, discard, sizeof(discard), MSG_DONTWAIT) > 0) + ; return true; } -void DataStreamer :: vicPaletteChanged() +void DataStreamer :: wakePaletteTask() { if (palette_task_handle) { xTaskNotifyGive(palette_task_handle); } } +void DataStreamer :: vicPaletteChanged() +{ + if (palette_stream_requested) { + wakePaletteTask(); + } +} + void DataStreamer :: paletteTask() { const TickType_t repeat_ticks = pdMS_TO_TICKS(1000); - // Twenty milliseconds is longer than one PAL or NTSC frame, so even a - // burst of palette writes adds at most one metadata packet per video frame. - const TickType_t minimum_ticks = pdMS_TO_TICKS(20); + // A send happens partway through a tick, so counting whole ticks takes one + // more than 20 ms to keep two packets out of one PAL or NTSC frame. + const TickType_t minimum_ticks = pdMS_TO_TICKS(20) + 1; TickType_t last_send = 0; + bool sent = false; while (true) { - const uint32_t notified = ulTaskNotifyTake(pdTRUE, repeat_ticks); - if (notified && last_send) { - const TickType_t now = xTaskGetTickCount(); - const TickType_t elapsed = now - last_send; + // Notifications latch: a start between this read and the wait still + // ends the wait at once. + const TickType_t wait = palette_stream_requested ? repeat_ticks : portMAX_DELAY; + const uint32_t notified = ulTaskNotifyTake(pdTRUE, wait); + if (notified && sent) { + const TickType_t elapsed = xTaskGetTickCount() - last_send; if (elapsed < minimum_ticks) { vTaskDelay(minimum_ticks - elapsed); } } if (sendVicPalette()) { last_send = xTaskGetTickCount(); + sent = true; } } } @@ -502,7 +547,7 @@ void DataStreamer :: calculate_udp_headers(int id) header[30] = (uint8_t)(stream->dest_ip >> 0); // destination mac - if ((stream->dest_ip & 0x000000F8) == 0x000000E8) { + if ((stream->dest_ip & 0x000000F0) == 0x000000E0) { header[3] = header[31] & 0x7F; header[4] = header[32]; header[5] = header[33]; diff --git a/software/io/network/data_streamer.h b/software/io/network/data_streamer.h index 56dbbfbeb..9c3cb657b 100644 --- a/software/io/network/data_streamer.h +++ b/software/io/network/data_streamer.h @@ -27,6 +27,10 @@ typedef struct { uint8_t enable; } stream_config_t; +// Or'ed into SubsysCommand::mode to start the VIC stream with runtime palette +// packets. The stream ID is the low byte. +#define STREAM_MODE_PALETTE 0x100 + class DataStreamer : public ObjectWithMenu { @@ -46,7 +50,8 @@ class DataStreamer : public ObjectWithMenu volatile bool palette_stream_requested; TaskHandle_t palette_task_handle; int palette_socket; - uint32_t palette_socket_ip; + uint32_t palette_socket_dest_ip; + int palette_socket_dest_port; stream_config_t streams[4]; TimerHandle_t timers[4]; @@ -58,8 +63,11 @@ class DataStreamer : public ObjectWithMenu void calculate_udp_headers(int id); void send_udp_packet(uint32_t ip, uint16_t port); + bool openPaletteSocket(); + void closePaletteSocket(); bool sendVicPalette(); void paletteTask(); + void wakePaletteTask(); public: DataStreamer(); virtual ~DataStreamer(); diff --git a/software/io/network/network_interface.cc b/software/io/network/network_interface.cc index c495c3c88..ff6110aa2 100644 --- a/software/io/network/network_interface.cc +++ b/software/io/network/network_interface.cc @@ -459,6 +459,13 @@ void NetworkInterface :: getMacAddr(uint8_t *buf) memcpy(buf, &my_net_if.hwaddr, 6); } +// The name netif_find() looks up, and so the one SO_BINDTODEVICE takes: +// the interface's two letters followed by its number. +void NetworkInterface :: getNetifName(char *name, int size) +{ + snprintf(name, size, "%c%c%u", my_net_if.name[0], my_net_if.name[1], my_net_if.num); +} + void NetworkInterface :: setIpAddr(uint8_t *buf) { memcpy(&my_ip.addr, &buf[0], 4); diff --git a/software/io/network/network_interface.h b/software/io/network/network_interface.h index 5d763ef58..6596c97e8 100644 --- a/software/io/network/network_interface.h +++ b/software/io/network/network_interface.h @@ -136,6 +136,7 @@ class NetworkInterface : protected ConfigurableObject void getIpAddr(uint8_t *a); void getMacAddr(uint8_t *a); + void getNetifName(char *name, int size); void setIpAddr(uint8_t *a); char *getIpAddrString(char *buf, int buflen); bool peekArpTable(uint32_t ipToQuery, uint8_t *mac); diff --git a/software/u64/u64_config.cc b/software/u64/u64_config.cc index 77c2f4e3a..6ec08adbc 100755 --- a/software/u64/u64_config.cc +++ b/software/u64/u64_config.cc @@ -2767,12 +2767,16 @@ static void program_palette_color(uint8_t index, const uint8_t rgb[3]) void U64Config :: set_palette_rgb(const uint8_t rgb[16][3]) { + // The registers are written inside the same critical section, so that + // what the VIC shows, GET_PALETTE and the streamed generation cannot + // disagree when two writers race. That is 16 colours of register writes + // and integer YUV conversion; nothing in it blocks. taskENTER_CRITICAL(); memcpy(active_palette, rgb, sizeof(active_palette)); active_palette_valid = true; active_palette_generation++; - taskEXIT_CRITICAL(); program_palette_rgb(rgb); + taskEXIT_CRITICAL(); if (dataStreamer) { dataStreamer->vicPaletteChanged(); } @@ -2797,8 +2801,8 @@ void U64Config :: set_palette_color(uint8_t index, const uint8_t rgb[3]) } memcpy(active_palette[index], rgb, 3); active_palette_generation++; - taskEXIT_CRITICAL(); program_palette_color(index, rgb); + taskEXIT_CRITICAL(); if (dataStreamer) { dataStreamer->vicPaletteChanged(); } diff --git a/tests/e2e/io/command_interface/uci_targets_test.py b/tests/e2e/io/command_interface/uci_targets_test.py index 80d3d6d78..735f47698 100755 --- a/tests/e2e/io/command_interface/uci_targets_test.py +++ b/tests/e2e/io/command_interface/uci_targets_test.py @@ -10,8 +10,9 @@ It also covers the transport state machine, the control target's rejection paths, and reply framing on the SoftIEC target, whose single-part replies were announced as "Data More" and left a client waiting for a block that is never sent. On -Ultimate 64 hardware it verifies the runtime RGB palette commands and restores -the palette before exiting. +Ultimate 64 hardware it verifies the runtime RGB palette commands, and the +palette packets a VIC stream opted into with palette=1 carries, and restores the +palette before exiting. Every expected value here was taken from the firmware and confirmed against a real device. The manuals under doc/ ("Ultimate Command Interface - Register API", @@ -45,8 +46,10 @@ import ftplib import itertools import json +import select import socket import sys +import threading import time import urllib.error import urllib.parse @@ -59,6 +62,7 @@ import bootstrap # noqa: E402,F401 import assembler # noqa: E402 import cli # noqa: E402 +from api import UltimateApi # noqa: E402 import ftp as ftp_lib import rest as rest_lib import streams as stream_lib @@ -123,9 +127,16 @@ CTRL_CMD_SET_PALETTE = 0x52 CTRL_CMD_SET_PALETTE_COLOR = 0x53 CTRL_CMD_RESET_PALETTE = 0x54 -VIC_PALETTE_PACKET_SIZE = 60 -VIC_PALETTE_LINE = 239 -VIC_VIDEO_PACKET_SIZE = 780 +# The debug stream's default destination, from the firmware's stream settings. +DEBUG_GROUP = "239.0.1.66" +DEBUG_PORT = 11002 +# Longer than the one-second palette repeat, so a leaked opt-in cannot hide. +PALETTE_QUIET_SECONDS = 1.5 +# From the device answering to the palette packet arriving. The repeat is a +# second, so only a send on the event itself makes it this soon. +PALETTE_PROMPT_SECONDS = 0.3 +# A gap in the video longer than this is the stream stopping, not the network. +VIDEO_STALL_SECONDS = 0.1 PALETTE_BURST_SOURCE = Path(__file__).with_name("vic_palette_burst.asm") PALETTE_BURST_STATUS = 0xC000 PALETTE_BURST_READY = 0xA4 @@ -231,6 +242,7 @@ "get-drvinfo", "softiec-x00-name", "softiec-setting-modes", + "palette-stream", "interface-usable-after", ] @@ -650,67 +662,7 @@ def run_control_target(uci: Uci) -> bool: return True -def expect_palette_packet(sock, addresses: set[str], expected: bytes, - accept_any_source: bool = False, timeout: float = 2.0) -> int: - last_unexpected = None - for _, packet, mine in stream_lib.receive([sock], addresses, timeout): - if (not mine and not accept_any_source) or len(packet) != VIC_PALETTE_PACKET_SIZE: - continue - if int.from_bytes(packet[4:6], "little") != VIC_PALETTE_LINE: - continue - if packet[6:12] != bytes([0x80, 0x01, 1, 4, 1, 0]): - raise Failure(f"palette stream packet has invalid format bytes {packet[6:12]!r}") - if packet[12:] != expected: - last_unexpected = packet[12:] - continue - return int.from_bytes(packet[:2], "little") - if last_unexpected is not None: - raise Failure(f"last palette packet was {last_unexpected!r}, expected {expected!r}") - raise Failure(f"no palette packet arrived on the VIC stream within {timeout:g} seconds") - - -def expect_no_palette_packet(sock, addresses: set[str], accept_any_source: bool = False) -> None: - for _, packet, mine in stream_lib.receive([sock], addresses, 0.25): - if ((mine or accept_any_source) and len(packet) == VIC_PALETTE_PACKET_SIZE and - int.from_bytes(packet[4:6], "little") == VIC_PALETTE_LINE and - packet[6:12] == bytes([0x80, 0x01, 1, 4, 1, 0])): - raise Failure("VIC stream sent palette data without an explicit palette request") - - -def capture_palette_burst(session: RestSession, sock, addresses: set[str], - accept_any_source: bool) -> tuple[list[tuple[float, int, bytes]], list[int], int, float]: - """Run the 6502 burst fixture while continuously draining the video socket.""" - samples: list[tuple[float, int, bytes]] = [] - video_sequences: list[int] = [] - - def receive(timeout: float) -> None: - for _, packet, mine in stream_lib.receive([sock], addresses, timeout): - if not mine and not accept_any_source: - continue - now = time.monotonic() - if (len(packet) == VIC_PALETTE_PACKET_SIZE and - int.from_bytes(packet[4:6], "little") == VIC_PALETTE_LINE and - packet[6:12] == bytes([0x80, 0x01, 1, 4, 1, 0])): - samples.append((now, int.from_bytes(packet[:2], "little"), packet[12:])) - elif len(packet) == VIC_VIDEO_PACKET_SIZE: - video_sequences.append(int.from_bytes(packet[:2], "little")) - - started = time.monotonic() - session.poke(PALETTE_BURST_GO, 1) - deadline = started + 15.0 - while True: - receive(0.05) - result = session.readmem(PALETTE_BURST_STATUS, 3, repeatable=True) - if result[0] == PALETTE_BURST_DONE: - break - if result[0] != PALETTE_BURST_RUNNING: - raise Failure(f"palette burst reported unexpected status ${result[0]:02X}") - if time.monotonic() >= deadline: - raise Failure("palette burst did not finish within 15 seconds") - return samples, video_sequences, int.from_bytes(result[1:3], "little"), time.monotonic() - started - - -def run_palette(session: RestSession, uci: Uci) -> bool: +def run_palette(uci: Uci) -> bool: """Exercise the U64 runtime palette protocol without changing saved config.""" scenario = "palette" product, status = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_HWINFO, 0x00])) @@ -735,36 +687,7 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if len(original) != 48: raise Failure(f"{scenario}: expected 48 palette bytes, got {len(original)}") - group = session.target.video_group - port = session.target.video_port - # A dual-homed Ultimate can send the FPGA video from one interface and the - # software palette packet from another. A unicast destination belongs only - # to this socket; multicast still needs source filtering between devices. - accept_any_source = not stream_lib.is_multicast(group) - addresses = stream_lib.source_addresses(session.target) - if not addresses: - raise Failure(f"{scenario}: could not resolve the VIC stream source address") - sock = stream_lib.stream_socket(group, port) - sock.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, 4 * 1024 * 1024) - stream_started = False try: - with check(f"{scenario}: invalid palette stream options are rejected"): - for stream, value in (("video", 2), ("audio", 1), ("audio", 0)): - status, body = session.request( - "PUT", f"/v1/streams/{stream}:start", - params={"ip": f"{group}:{port}", "palette": value}) - if status != 400: - raise Failure( - f"{stream} stream with palette={value} returned HTTP {status}: {body[:200]!r}") - - with check(f"{scenario}: palette=0 sends no palette packets"): - status, body = session.request( - "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}", "palette": 0}) - if status != 200: - raise Failure(f"video stream start returned HTTP {status}: {body[:200]!r}") - stream_started = True - expect_no_palette_packet(sock, addresses, accept_any_source) - with check(f"{scenario}: SET_PALETTE_COLOR changes only the requested color"): changed = bytearray(original) changed[-3:] = bytes(component ^ 0x5A for component in changed[-3:]) @@ -776,125 +699,6 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if text != STATUS_OK or actual != bytes(changed): raise Failure(f"{scenario}: single-color readback was {actual!r}, expected {bytes(changed)!r}") - with check(f"{scenario}: ordinary VIC stream remains unchanged after a runtime palette command"): - expect_no_palette_packet(sock, addresses, accept_any_source) - - with check(f"{scenario}: opted-in VIC stream starts with the current runtime palette"): - status, body = session.request("PUT", "/v1/streams/video:stop") - if status != 200: - raise Failure(f"video stream stop returned HTTP {status}: {body[:200]!r}") - stream_started = False - status, body = session.request( - "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}", "palette": 1}) - if status != 200: - raise Failure(f"video stream restart returned HTTP {status}: {body[:200]!r}") - stream_started = True - generation = expect_palette_packet(sock, addresses, bytes(changed), accept_any_source) - - with check(f"{scenario}: opted-in VIC stream periodically repeats its palette"): - for _ in range(2): - repeated_generation = expect_palette_packet( - sock, addresses, bytes(changed), accept_any_source, timeout=3.0) - if repeated_generation != generation: - raise Failure( - f"palette repeat generation {repeated_generation} differs from initial {generation}") - - with check(f"{scenario}: rapid changes are coalesced without disturbing video"): - session.poke(PALETTE_BURST_GO, 0) - session.poke(PALETTE_BURST_STATUS, 0) - session.run_prg(assembler.assemble(PALETTE_BURST_SOURCE)) - deadline = time.monotonic() + 5.0 - while session.peek(PALETTE_BURST_STATUS, repeatable=True) != PALETTE_BURST_READY: - if time.monotonic() >= deadline: - raise Failure("palette burst fixture did not reach its ready state") - time.sleep(0.05) - burst_palette, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) - if text != STATUS_OK: - raise Failure(f"GET_PALETTE before burst returned {text!r}") - status, body = session.request( - "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}", "palette": 1}) - if status != 200: - raise Failure(f"video stream restart returned HTTP {status}: {body[:200]!r}") - generation = expect_palette_packet( - sock, addresses, burst_palette, accept_any_source) - # Empty video queued around the stream restart before measuring. - tuple(stream_lib.receive([sock], addresses, 0.05)) - samples, video_sequences, command_count, seconds = capture_palette_burst( - session, sock, addresses, accept_any_source) - if command_count < 2 or len(samples) < 2: - raise Failure( - f"palette burst completed {command_count} changes but captured " - f"{len(samples)} palette and {len(video_sequences)} video packets") - readback, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) - if text != STATUS_OK: - raise Failure(f"GET_PALETTE after burst returned {text!r}") - tuple(stream_lib.receive([sock], addresses, 0.05)) - status, body = session.request( - "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}", "palette": 1}) - if status != 200: - raise Failure(f"video stream restart returned HTTP {status}: {body[:200]!r}") - final_generation = expect_palette_packet( - sock, addresses, readback, accept_any_source) - session.poke(PALETTE_BURST_GO, 2) - generation_delta = (final_generation - generation) & 0xFFFF - detail(f"captured {len(samples)} palette and {len(video_sequences)} video packets; " - f"last live generation delta {(samples[-1][1] - generation) & 0xFFFF}, " - f"final delta {generation_delta}") - if generation_delta != command_count: - raise Failure( - f"{command_count} changes advanced the generation by {generation_delta}") - for _, sample_generation, sample_palette in samples: - change = (sample_generation - generation) & 0xFFFF - if 1 <= change <= command_count: - index = change - 1 - expected = bytes(((13 * index) & 0xFF, - (85 + 29 * index) & 0xFF, - (170 + 47 * index) & 0xFF)) - if sample_palette[18:21] != expected: - raise Failure( - f"generation {sample_generation} carried color {sample_palette[18:21]!r}, " - f"expected {expected!r}") - last = command_count - 1 - expected_color = bytes(((13 * last) & 0xFF, - (85 + 29 * last) & 0xFF, - (170 + 47 * last) & 0xFF)) - if readback[18:21] != expected_color: - raise Failure(f"rapid-change color was {readback[18:21]!r}, expected {expected_color!r}") - distinct = [sample for i, sample in enumerate(samples) - if i == 0 or sample[1] != samples[i - 1][1]] - if len(distinct) >= command_count: - raise Failure(f"{command_count} changes produced {len(distinct)} distinct packets; no coalescing") - intervals = [current[0] - previous[0] - for previous, current in itertools.pairwise(samples)] - sample_span = samples[-1][0] - samples[0][0] - packet_rate = (len(samples) - 1) / sample_span if sample_span > 0 else float("inf") - # Host scheduling can timestamp two already-queued UDP datagrams - # close together, so sustained rate is the stable wire-rate check. - if packet_rate > 55.0: - raise Failure(f"palette stream sustained {packet_rate:.1f} packets/s, expected at most 55") - discontinuities = sum( - 1 for previous, current in itertools.pairwise(video_sequences) - if ((current - previous) & 0xFFFF) != 1) - if len(video_sequences) < 100 or discontinuities: - raise Failure( - f"video during burst had {len(video_sequences)} packets and " - f"{discontinuities} sequence discontinuities") - detail(f"{command_count} palette changes in {seconds:.2f}s -> {len(distinct)} packets; " - f"{packet_rate:.1f} packets/s, minimum observed spacing " - f"{min(intervals) * 1000:.1f} ms; " - f"{len(video_sequences)} consecutive video packets") - session.reset() - uci.release() - resumed_palette, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) - if text != STATUS_OK: - raise Failure(f"GET_PALETTE after fixture cleanup returned {text!r}") - status, body = session.request( - "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}", "palette": 1}) - if status != 200: - raise Failure(f"video stream resume returned HTTP {status}: {body[:200]!r}") - generation = expect_palette_packet( - sock, addresses, resumed_palette, accept_any_source) - expect(uci, f"{scenario}: color index 16 is rejected", bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE_COLOR, 16, 0, 0, 0]), STATUS_INVALID_PARAMS, reply=b"") @@ -909,11 +713,6 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if text != STATUS_OK or actual != replacement: raise Failure(f"{scenario}: full-palette readback was {actual!r}, expected {replacement!r}") - with check(f"{scenario}: palette changes advance the streamed generation"): - replacement_generation = expect_palette_packet(sock, addresses, replacement, accept_any_source) - if ((replacement_generation - generation) & 0xFFFF) == 0: - raise Failure("palette generation did not advance after SET_PALETTE") - expect(uci, f"{scenario}: short SET_PALETTE payload is rejected", bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE]) + original[:-1], STATUS_INVALID_PARAMS, reply=b"") @@ -932,47 +731,370 @@ def run_palette(session: RestSession, uci: Uci) -> bool: if text != STATUS_OK or actual != default_palette: raise Failure(f"{scenario}: reset palette readback was {actual!r}, expected {default_palette!r}") - with check(f"{scenario}: RESET_PALETTE is streamed to an opted-in client"): - expect_palette_packet(sock, addresses, default_palette, accept_any_source) - expect(uci, f"{scenario}: RESET_PALETTE rejects a payload", bytes([TARGET_CONTROL, CTRL_CMD_RESET_PALETTE, 0]), STATUS_INVALID_PARAMS, reply=b"") + finally: + with check(f"{scenario}: restore the original runtime palette"): + reply, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE]) + original) + if text != STATUS_OK or reply: + raise Failure(f"{scenario}: restore returned data {reply!r}, status {text!r}") + actual, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) + if text != STATUS_OK or actual != original: + raise Failure(f"{scenario}: restored palette readback was {actual!r}") + return True + + +class VicListener: + """Drain the VIC stream on a thread of its own. - with check(f"{scenario}: a later ordinary stream is not opted in implicitly"): - status, body = session.request("PUT", "/v1/streams/video:stop") - if status != 200: - raise Failure(f"video stream stop returned HTTP {status}: {body[:200]!r}") - stream_started = False - tuple(stream_lib.receive([sock], addresses, 0.05)) - status, body = session.request( - "PUT", "/v1/streams/video:start", params={"ip": f"{group}:{port}"}) - if status != 200: - raise Failure(f"video stream restart returned HTTP {status}: {body[:200]!r}") - stream_started = True - expect_no_palette_packet(sock, addresses, accept_any_source) + A REST call with the stream running takes 80 ms or more, and one PAL frame + is 50 datagrams every 20 ms, so a socket read only between calls overflows + any receive buffer Linux grants without raising `net.core.rmem_max`. A + thread keeps it drained whatever the main thread is waiting on. + """ + + def __init__(self, sock: socket.socket, addresses: set[str]) -> None: + self.sock = sock + self.addresses = addresses + self.lock = threading.Lock() + self.palettes: list[tuple[float, int, bytes]] = [] + self.video: list[tuple[float, int]] = [] + self.foreign: set[str] = set() + self.error: Failure | None = None + self._stop = threading.Event() + self._thread = threading.Thread(target=self._run, name="vic-listener", daemon=True) + + def _run(self) -> None: + while not self._stop.is_set(): + ready, _, _ = select.select([self.sock], (), (), 0.1) + if not ready: + continue + try: + data, sender = self.sock.recvfrom(2048) + except OSError: + continue + now = time.monotonic() + if sender[0] not in self.addresses: + with self.lock: + self.foreign.add(sender[0]) + continue + try: + palette = stream_lib.palette_packet(data) + except Failure as exc: + with self.lock: + self.error = self.error or exc + continue + with self.lock: + if palette is not None: + self.palettes.append((now, *palette)) + elif len(data) == stream_lib.PACKET_SIZE: + self.video.append((now, int.from_bytes(data[:2], "little"))) + + def start(self) -> None: + self._thread.start() + + def stop(self) -> None: + self._stop.set() + self._thread.join(timeout=2.0) + + def raise_error(self) -> None: + with self.lock: + if self.error is not None: + raise self.error + + def palettes_since(self, since: float) -> list[tuple[float, int, bytes]]: + with self.lock: + return [p for p in self.palettes if p[0] >= since] + + def video_since(self, since: float) -> list[tuple[float, int]]: + with self.lock: + return [v for v in self.video if v[0] >= since] + + def wait_palette(self, since: float, expected: bytes | None = None, + timeout: float = 2.0) -> tuple[float, int, bytes]: + """The first palette packet after `since` carrying `expected`, if given.""" + deadline = time.monotonic() + timeout + while True: + self.raise_error() + for packet in self.palettes_since(since): + if expected is None or packet[2] == expected: + return packet + if time.monotonic() >= deadline: + seen = self.palettes_since(since) + if seen: + raise Failure(f"last palette packet carried {seen[-1][2].hex()}, " + f"expected {expected.hex() if expected else 'any'}") + raise Failure(f"no palette packet arrived within {timeout:g} seconds") + time.sleep(0.01) + + def expect_no_palette(self, seconds: float = PALETTE_QUIET_SECONDS) -> None: + """Listen longer than one repeat, with video as the positive control.""" + since = time.monotonic() + time.sleep(seconds) + self.raise_error() + video = self.video_since(since) + if len(video) < 100: + raise Failure(f"only {len(video)} video packets in {seconds:g} s, so the " + f"absence of palette packets proves nothing") + palettes = self.palettes_since(since) + if palettes: + raise Failure(f"{len(palettes)} palette packets arrived on a stream that " + f"did not ask for them") + + +def expected_burst_color(index: int) -> bytes: + """Color 6 after the burst fixture's `index`th change, counting from 0.""" + return bytes(((13 * index) & 0xFF, (85 + 29 * index) & 0xFF, (170 + 47 * index) & 0xFF)) + + +def run_palette_stream(session: RestSession, uci: Uci) -> bool: + """Runtime palette packets on the VIC stream (#850), opted into with palette=1. + + Runs after the command-interface scenarios, so a machine that cannot stream + to this host costs nothing but this scenario. + """ + scenario = "palette-stream" + _, probe_status = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) + if probe_status == STATUS_UNKNOWN_COMMAND: + check_start(f"{scenario}: the machine has a runtime palette") + check_skip("GET_PALETTE is not served") + return True + original, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) + if text != STATUS_OK or len(original) != 48: + raise Failure(f"{scenario}: GET_PALETTE returned {len(original)} bytes, status {text!r}") + + video_address = f"{session.target.video_group}:{session.target.video_port}" + # The rejected value doubles as the capability probe: firmware without + # palette streams refuses the unknown parameter in its own words. + status, body = session.request("PUT", "/v1/streams/video:start", + params={"ip": video_address, "palette": 2}) + if status == 400 and b"Palette must be 0 or 1" not in body: + check_start(f"{scenario}: the firmware offers palette streams") + check_skip(f"video:start does not take a palette parameter: {body[:120]!r}") + return True + if status != 400: + raise Failure(f"{scenario}: video:start with palette=2 returned HTTP {status}: {body[:200]!r}") + + api = UltimateApi(session.target, session.password) + arming = stream_lib.Arming(api, session.target) + addresses = stream_lib.source_addresses(session.target) + if not addresses: + raise Failure(f"{scenario}: could not resolve the VIC stream source address") + sock = stream_lib.stream_socket(session.target.video_group, session.target.video_port) + sock.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, 4 * 1024 * 1024) + listener = VicListener(sock, addresses) + listener.start() + fixture_loaded = False + fixture_released = False + try: + # Whether this host can see the stream at all is a property of the + # bench, not of the firmware, so it decides a skip rather than a failure. + if not arming.start("video", palette=0): + reason = arming.failures.get("video", "") + if "No Operational Network Interface" not in reason: + raise Failure(f"{scenario}: video:start failed: {reason}") + skip = "the VIC stream's network interface has no link" + else: + since = time.monotonic() + time.sleep(1.0) + skip = None + if len(listener.video_since(since)) < 100: + with listener.lock: + foreign = sorted(listener.foreign) + skip = (f"the VIC stream arrives from {', '.join(foreign)}, not from " + f"{', '.join(sorted(addresses))}; name that address with -H" + if foreign else "no VIC stream packets reach this host") + if skip: + check_start(f"{scenario}: the VIC stream reaches this host") + check_skip(skip) + return True + + with check(f"{scenario}: a refused palette value leaves the debug stream running"): + arming.stop("video") + debug_sock = stream_lib.stream_socket(DEBUG_GROUP, DEBUG_PORT) + try: + status, body = session.request("PUT", "/v1/streams/debug:start", + params={"ip": f"{DEBUG_GROUP}:{DEBUG_PORT}"}) + if status != 200: + raise Failure(f"debug:start returned HTTP {status}: {body[:200]!r}") + for stream, value in (("video", 2), ("audio", 1), ("audio", 0)): + status, body = session.request( + "PUT", f"/v1/streams/{stream}:start", + params={"ip": video_address, "palette": value}) + if status != 400: + raise Failure(f"{stream} stream with palette={value} returned " + f"HTTP {status}: {body[:200]!r}") + tuple(stream_lib.receive([debug_sock], addresses, 0.1)) + debug = sum(1 for _, _, mine in stream_lib.receive([debug_sock], addresses, 0.5) + if mine) + if not debug: + raise Failure("the debug stream stopped for a request that was refused") + finally: + session.request("PUT", "/v1/streams/debug:stop") + debug_sock.close() + if not arming.start("video", palette=0): + raise Failure(f"video:start failed: {arming.failures.get('video')}") + + with check(f"{scenario}: palette=0 sends no palette packets"): + listener.expect_no_palette() + + with check(f"{scenario}: a palette change on an ordinary stream sends nothing"): + changed = bytearray(original) + changed[-3:] = bytes(component ^ 0x5A for component in changed[-3:]) + reply, text = uci.transact( + bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE_COLOR, 15]) + changed[-3:]) + if text != STATUS_OK or reply: + raise Failure(f"SET_PALETTE_COLOR returned data {reply!r}, status {text!r}") + listener.expect_no_palette() + + with check(f"{scenario}: an opted-in start sends the current palette at once"): + arming.stop("video") + requested = time.monotonic() + if not arming.start("video", palette=1): + raise Failure(f"video:start with palette=1 failed: {arming.failures.get('video')}") + answered = time.monotonic() + arrived, generation, _ = listener.wait_palette(requested, bytes(changed)) + if arrived - answered > PALETTE_PROMPT_SECONDS: + raise Failure(f"the first palette packet came {arrived - answered:.2f} s after " + f"the start was answered") + + with check(f"{scenario}: the palette repeats once a second"): + first = listener.wait_palette(arrived + 0.01, bytes(changed), timeout=2.0) + second = listener.wait_palette(first[0] + 0.01, bytes(changed), timeout=2.0) + interval = second[0] - first[0] + if {first[1], second[1]} != {generation}: + raise Failure(f"repeats carried generations {first[1]} and {second[1]}, " + f"expected {generation}") + if not 0.8 <= interval <= 1.3: + raise Failure(f"repeats came {interval:.2f} s apart") + + with check(f"{scenario}: a palette change is sent at once"): + previous = listener.wait_palette(second[0] + 0.01, bytes(changed), timeout=2.0) + changed[-3:] = bytes(component ^ 0xFF for component in changed[-3:]) + reply, text = uci.transact( + bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE_COLOR, 15]) + changed[-3:]) + done = time.monotonic() + if text != STATUS_OK or reply: + raise Failure(f"SET_PALETTE_COLOR returned data {reply!r}, status {text!r}") + arrived, generation, _ = listener.wait_palette(previous[0] + 0.01, bytes(changed)) + if arrived - done > PALETTE_PROMPT_SECONDS: + raise Failure(f"the changed palette came {arrived - done:.2f} s after the change") + # Only a send on change can arrive before the next repeat is due. + if done - previous[0] < 0.6 and arrived - previous[0] > 0.9: + raise Failure(f"the changed palette came {arrived - previous[0]:.2f} s after the " + f"last repeat, which is when the repeat was due") + + with check(f"{scenario}: rapid changes are coalesced without stopping the video"): + session.poke(PALETTE_BURST_GO, 0) + session.poke(PALETTE_BURST_STATUS, 0) + session.run_prg(assembler.assemble(PALETTE_BURST_SOURCE)) + fixture_loaded = True + deadline = time.monotonic() + 5.0 + while session.peek(PALETTE_BURST_STATUS, repeatable=True) != PALETTE_BURST_READY: + if time.monotonic() >= deadline: + raise Failure("the burst fixture did not reach its ready state") + time.sleep(0.05) + baseline_palette, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) + if text != STATUS_OK: + raise Failure(f"GET_PALETTE before the burst returned {text!r}") + _, baseline, _ = listener.wait_palette(time.monotonic(), baseline_palette) + started = time.monotonic() + session.poke(PALETTE_BURST_GO, 1) + deadline = started + 15.0 + while True: + result = session.readmem(PALETTE_BURST_STATUS, 3, repeatable=True) + if result[0] == PALETTE_BURST_DONE: + break + if result[0] not in (PALETTE_BURST_READY, PALETTE_BURST_RUNNING): + raise Failure(f"the burst fixture reported status ${result[0]:02X}") + if time.monotonic() >= deadline: + raise Failure("the burst did not finish within 15 seconds") + time.sleep(0.05) + finished = time.monotonic() + session.poke(PALETTE_BURST_GO, 2) + fixture_released = True + count = int.from_bytes(result[1:3], "little") + final_palette, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) + if text != STATUS_OK: + raise Failure(f"GET_PALETTE after the burst returned {text!r}") + final = listener.wait_palette(started, final_palette) + samples = [p for p in listener.palettes_since(started) if p[0] <= final[0]] + video = [v for v in listener.video_since(started) if v[0] <= finished] + + delta = (final[1] - baseline) & 0xFFFF + if count < 2 or delta != count: + raise Failure(f"{count} changes advanced the generation by {delta}") + if final_palette[18:21] != expected_burst_color(count - 1): + raise Failure(f"color 6 ended as {final_palette[18:21].hex()}, " + f"expected {expected_burst_color(count - 1).hex()}") + for _, sample_generation, sample_palette in samples: + change = (sample_generation - baseline) & 0xFFFF + if 1 <= change <= count and sample_palette[18:21] != expected_burst_color(change - 1): + raise Failure(f"generation {sample_generation} carried color 6 as " + f"{sample_palette[18:21].hex()}, expected " + f"{expected_burst_color(change - 1).hex()}") + distinct = len({sample[1] for sample in samples}) + if distinct >= count: + raise Failure(f"{count} changes produced {distinct} distinct palette packets; " + f"nothing was coalesced") + span = samples[-1][0] - samples[0][0] if len(samples) > 1 else 0.0 + rate = (len(samples) - 1) / span if span > 0 else 0.0 + # Host scheduling can deliver two queued datagrams back to back, so + # the sustained rate is the stable measure of the wire rate. + if rate > 55.0: + raise Failure(f"palette packets came at {rate:.1f} per second, expected at most 55") + # Datagrams the host drops are the host's; the firmware's part is to + # keep the video coming, so a stall is what fails. + stall = max((b[0] - a[0] for a, b in itertools.pairwise(video)), default=0.0) + if len(video) < 100 or stall > VIDEO_STALL_SECONDS: + raise Failure(f"{len(video)} video packets during the burst, longest gap " + f"{stall * 1000:.0f} ms") + lost = sum(((b[1] - a[1]) & 0xFFFF) - 1 for a, b in itertools.pairwise(video) + if ((b[1] - a[1]) & 0xFFFF) < 0x8000) + detail(f"{count} changes in {finished - started:.2f} s -> {distinct} palette " + f"packets at {rate:.1f}/s; {len(video)} video packets, {lost} lost on " + f"the way here, longest gap {stall * 1000:.0f} ms") + + with check(f"{scenario}: RESET_PALETTE reaches an opted-in client at once"): + session.reset() + uci.release() + fixture_loaded = False + requested = time.monotonic() + reply, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_RESET_PALETTE])) + done = time.monotonic() + if text != STATUS_OK or reply: + raise Failure(f"RESET_PALETTE returned data {reply!r}, status {text!r}") + default_palette, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) + if text != STATUS_OK: + raise Failure(f"GET_PALETTE after RESET_PALETTE returned {text!r}") + arrived, _, _ = listener.wait_palette(requested, default_palette) + if arrived - done > PALETTE_PROMPT_SECONDS: + raise Failure(f"the reset palette came {arrived - done:.2f} s after the reset") + + with check(f"{scenario}: a later ordinary start is not opted in"): + arming.stop("video") + if not arming.start("video"): + raise Failure(f"video:start failed: {arming.failures.get('video')}") + listener.expect_no_palette() finally: try: - try: - session.poke(PALETTE_BURST_GO, 2) - except Failure: - pass + if fixture_loaded and not fixture_released: + # The fixture waits for GO=1 and must not start pushing + # commands into the queue the restore below uses. + session.reset() + uci.release() with check(f"{scenario}: restore the original runtime palette"): reply, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_SET_PALETTE]) + original) if text != STATUS_OK or reply: - raise Failure(f"{scenario}: restore returned data {reply!r}, status {text!r}") + raise Failure(f"restore returned data {reply!r}, status {text!r}") actual, text = uci.transact(bytes([TARGET_CONTROL, CTRL_CMD_GET_PALETTE])) if text != STATUS_OK or actual != original: - raise Failure(f"{scenario}: restored palette readback was {actual!r}") + raise Failure(f"restored palette readback was {actual!r}") finally: - try: - if stream_started: - with check(f"{scenario}: stop the VIC stream"): - status, body = session.request("PUT", "/v1/streams/video:stop") - if status != 200: - raise Failure(f"video stream stop returned HTTP {status}: {body[:200]!r}") - finally: - sock.close() + arming.stop_all() + listener.stop() + sock.close() return True @@ -1403,7 +1525,7 @@ def run(name: str, fn, *fn_args) -> None: run("transport", run_transport, uci) run("control-target", run_control_target, uci) - run("palette", run_palette, session, uci) + run("palette", run_palette, uci) run("issue-740-matrix", run_issue_740_matrix, session, ftp, uci) run("save-reu-offset-past-end", run_save_reu_offset_past_end, session, uci) run("load-reu-disabled", run_reu_disabled, session, uci, CTRL_CMD_LOAD_REU, "load-reu-disabled") @@ -1412,6 +1534,7 @@ def run(name: str, fn, *fn_args) -> None: run("get-drvinfo", run_get_drvinfo, session, uci) run("softiec-x00-name", run_softiec_x00_name, ftp, uci) run("softiec-setting-modes", run_softiec_setting_modes, session, uci) + run("palette-stream", run_palette_stream, session, uci) run("interface-usable-after", run_interface_usable_after, uci) except Failure as exc: diff --git a/tests/e2e/io/command_interface/vic_palette_burst.asm b/tests/e2e/io/command_interface/vic_palette_burst.asm index c229c559a..f96bd063a 100644 --- a/tests/e2e/io/command_interface/vic_palette_burst.asm +++ b/tests/e2e/io/command_interface/vic_palette_burst.asm @@ -12,7 +12,7 @@ ST_STAT = $40 STATUS = $C000 ; $A4 ready, $A5 running, $5A complete COUNT = $C001 ; completed commands, little endian -GO = $C003 ; host writes nonzero to release the burst +GO = $C003 ; host writes 1 to start the burst, 2 to exit FRAMES = 60 CHANGES_PER_FRAME = 4 @@ -35,9 +35,11 @@ start sta GO lda #$A4 sta STATUS +; Only 1 starts: the 2 that lets a finished burst exit must not start one. wait_go lda GO - beq wait_go + cmp #$01 + bne wait_go lda #$A5 sta STATUS lda #FRAMES diff --git a/tests/e2e/lib/streams.py b/tests/e2e/lib/streams.py index 8b32ffac6..af813cd4c 100644 --- a/tests/e2e/lib/streams.py +++ b/tests/e2e/lib/streams.py @@ -222,14 +222,15 @@ def address(self, stream: str) -> str: def start(self, stream: str, already_arriving: bool = False, timeout: float | None = None, - retries: int | None = None) -> bool: + retries: int | None = None, **params: object) -> bool: """Ask the device to send `stream`, unless it already is. `already_arriving` is the caller saying it has seen packets from its own device at the standard address. That is the one thing about a stream that is not free to ask for twice, so a caller that finds it running issues no request at all and, by not having started it, leaves - it running afterwards. + it running afterwards. `params` go on the start request as they are, + such as `palette=1`. """ if already_arriving or stream in self.started: return False @@ -238,7 +239,7 @@ def start(self, stream: str, already_arriving: bool = False, # keep draining sockets bounds this call; see # rest.RestClient.request. self.api.streams.start(stream, ip=self.address(stream), - timeout=timeout, retries=retries) + timeout=timeout, retries=retries, **params) except Failure as exc: self.failures[stream] = str(exc) self.publish("start-failed", stream) @@ -346,6 +347,26 @@ def receive(sockets: Sequence[socket.socket], addresses: set[str], PAYLOAD_SIZE = LINES_PER_PACKET * BYTES_PER_LINE PACKET_SIZE = HEADER_SIZE + PAYLOAD_SIZE +# The runtime palette packet (#850), sent on the video port only to a stream +# started with palette=1: a video-style header on line 239, then the 16 RGB +# colors. Its 60 bytes cannot be mistaken for a 780-byte video packet. +PALETTE_PACKET_SIZE = HEADER_SIZE + 16 * 3 +PALETTE_LINE = 239 +_PALETTE_FORMAT = bytes([0x80, 0x01, 1, 4, 1, 0]) + + +def palette_packet(data: bytes) -> tuple[int, bytes] | None: + """`(generation, 48 RGB bytes)` for a palette packet, None for anything else. + + A datagram of that size on line 239 with other format bytes raises: it is + neither video nor a palette a client could use. + """ + if len(data) != PALETTE_PACKET_SIZE or int.from_bytes(data[4:6], "little") != PALETTE_LINE: + return None + if data[6:HEADER_SIZE] != _PALETTE_FORMAT: + raise Failure(f"palette packet has invalid format bytes {data[6:HEADER_SIZE].hex()}") + return int.from_bytes(data[:2], "little"), data[HEADER_SIZE:] + # The two frame heights the hardware actually produces: PAL is 272 lines, # NTSC is 240. The last packet's declared height is clamped to this range # (see _clamp_height) so a corrupt "line" field cannot grow the frame buffer From 5dd971c9f13a62e8f2efbd30269a1692f8c5c893 Mon Sep 17 00:00:00 2001 From: Barry Walker Date: Thu, 1 Oct 2026 15:14:34 -0400 Subject: [PATCH 13/13] test(streams): fail a missing palette stream, cover socket reuse and low multicast The palette-stream scenario used the rejected palette value as its capability probe, so firmware that lost the feature reported a skip and the run stayed green. The machines that lack it are now declared in the fix table (vic-palette-stream: only the C64 Ultimate's release firmware), and everywhere else a refused palette parameter fails the scenario. Two checks cover what the scenario left out. Twenty opted-in starts and stops must each still deliver a palette packet; lwIP has eight UDP sockets, so a palette socket that is not closed runs the pool dry well within that. And a stream to 230.0.1.64 must arrive as multicast, which is the 224-231 range this PR moved from unicast. --- .../io/command_interface/uci_targets_test.py | 79 ++++++++++++++++--- tests/lib/machine.py | 11 +++ 2 files changed, 81 insertions(+), 9 deletions(-) diff --git a/tests/e2e/io/command_interface/uci_targets_test.py b/tests/e2e/io/command_interface/uci_targets_test.py index 735f47698..b1d08dbf0 100755 --- a/tests/e2e/io/command_interface/uci_targets_test.py +++ b/tests/e2e/io/command_interface/uci_targets_test.py @@ -65,6 +65,7 @@ from api import UltimateApi # noqa: E402 import ftp as ftp_lib import rest as rest_lib +import machine as machine_lib import streams as stream_lib import targets from report import ( @@ -277,6 +278,16 @@ def __init__(self, host: str, password: str | None, timeout: float) -> None: self.password = password self.timeout = timeout + def machine(self) -> machine_lib.Machine: + """Which machine this is, asked once of the device's /v1/info.""" + def fetch() -> tuple[str, str]: + status, body = self.request("GET", "/v1/info", repeatable=True) + if status != 200: + raise Failure(f"GET /v1/info returned HTTP {status}: {body[:200]!r}") + info = json.loads(body) + return info.get("product", ""), info.get("firmware_version", "") + return machine_lib.identify(self.host, fetch) + def request(self, method: str, path: str, params: dict[str, object] | None = None, repeatable: bool = False, host: str | None = None, data: bytes | None = None) -> tuple[int, bytes]: @@ -843,6 +854,12 @@ def expect_no_palette(self, seconds: float = PALETTE_QUIET_SECONDS) -> None: f"did not ask for them") +# More opted-in start/stop cycles than lwIP has UDP sockets (MEMP_NUM_UDP_PCB 8). +PALETTE_SOCKET_CYCLES = 20 +# A multicast group in 224-231, which #871 moved from unicast to multicast. +PALETTE_LOW_MULTICAST_GROUP = "230.0.1.64" + + def expected_burst_color(index: int) -> bytes: """Color 6 after the burst fixture's `index`th change, counting from 0.""" return bytes(((13 * index) & 0xFF, (85 + 29 * index) & 0xFF, (170 + 47 * index) & 0xFF)) @@ -865,16 +882,21 @@ def run_palette_stream(session: RestSession, uci: Uci) -> bool: raise Failure(f"{scenario}: GET_PALETTE returned {len(original)} bytes, status {text!r}") video_address = f"{session.target.video_group}:{session.target.video_port}" - # The rejected value doubles as the capability probe: firmware without - # palette streams refuses the unknown parameter in its own words. - status, body = session.request("PUT", "/v1/streams/video:start", - params={"ip": video_address, "palette": 2}) - if status == 400 and b"Palette must be 0 or 1" not in body: - check_start(f"{scenario}: the firmware offers palette streams") - check_skip(f"video:start does not take a palette parameter: {body[:120]!r}") + machine = session.machine() + if not machine.has_data_streams: + check_start(f"{scenario}: the machine serves the VIC stream") + check_skip(f"{machine.described} has no VIC stream") return True - if status != 400: - raise Failure(f"{scenario}: video:start with palette=2 returned HTTP {status}: {body[:200]!r}") + # Declared, not probed: a probe would skip exactly when the feature broke. + offered = f"{scenario}: the firmware offers palette streams" + if machine.skip_without_fix(machine_lib.VIC_PALETTE_STREAM, offered): + return True + with check(offered): + status, body = session.request("PUT", "/v1/streams/video:start", + params={"ip": video_address, "palette": 2}) + if status != 400 or b"Palette must be 0 or 1" not in body: + raise Failure(f"video:start with palette=2 returned HTTP {status}: {body[:200]!r}; " + f"expected 400 'Palette must be 0 or 1'") api = UltimateApi(session.target, session.password) arming = stream_lib.Arming(api, session.target) @@ -1077,6 +1099,45 @@ def run_palette_stream(session: RestSession, uci: Uci) -> bool: if not arming.start("video"): raise Failure(f"video:start failed: {arming.failures.get('video')}") listener.expect_no_palette() + + with check(f"{scenario}: palette packets still arrive after " + f"{PALETTE_SOCKET_CYCLES} opted-in starts and stops"): + # Each opted-in start opens the palette socket and each stop closes + # it. lwIP has 8 UDP sockets in all, so a socket that is not given + # back runs the pool dry well within these cycles. + for cycle in range(1, PALETTE_SOCKET_CYCLES + 1): + arming.stop("video") + time.sleep(0.2) + requested = time.monotonic() + if not arming.start("video", palette=1): + raise Failure(f"cycle {cycle}: video:start with palette=1 failed: " + f"{arming.failures.get('video')}") + try: + listener.wait_palette(requested) + except Failure as exc: + raise Failure(f"cycle {cycle}: {exc}") from exc + arming.stop("video") + + low_group = PALETTE_LOW_MULTICAST_GROUP + with check(f"{scenario}: a stream to {low_group}, below 232.0.0.0, is sent as multicast"): + # #871 widened the multicast test from 232-239 to all of 224.0.0.0/4. + # Treated as unicast, a group in 224-231 has no ARP answer. + port = session.target.video_port + low_sock = stream_lib.stream_socket(low_group, port) + try: + status, body = session.request("PUT", "/v1/streams/video:start", + params={"ip": f"{low_group}:{port}"}) + if status != 200: + raise Failure(f"video:start to {low_group} returned HTTP {status}: " + f"{body[:200]!r}") + tuple(stream_lib.receive([low_sock], addresses, 0.1)) + arrived = sum(1 for _, _, mine in + stream_lib.receive([low_sock], addresses, 1.0) if mine) + if arrived < 100: + raise Failure(f"{arrived} video packets reached {low_group} in 1 s") + finally: + session.request("PUT", "/v1/streams/video:stop") + low_sock.close() finally: try: if fixture_loaded and not fixture_released: diff --git a/tests/lib/machine.py b/tests/lib/machine.py index fdd7efd01..b6d113092 100644 --- a/tests/lib/machine.py +++ b/tests/lib/machine.py @@ -263,6 +263,17 @@ def _fix(name: str, behaviour: str, lacking: tuple[str, ...]) -> str: "hexadecimal, rather than parsing it as $0000 and acting there", (C64U, U2)) +# What the palette-stream scenario in tests/e2e/io/command_interface/uci_targets_test.py +# asserts (#850, #871). The C64 Ultimate's release firmware predates it. A +# machine missing from `lacking` that refuses `palette` on video:start fails +# the scenario rather than skipping it, so a regression cannot pass as "not +# offered here". +VIC_PALETTE_STREAM = _fix( + "vic-palette-stream", + "video:start takes palette=0 or 1, and a stream started with palette=1 " + "carries the runtime palette as packets beside the video", + (C64U,)) + # Every fix at once, for a sweep that asks whether the lagging line has caught # up rather than about one behaviour. ASSUME_ALL = "all"