Encoder - #31
Merged
Merged
Conversation
- Introduced Hnm0Encoder project for encoding PNG frames into HNM format. - Implemented command line parsing, frame discovery, and quantization logic. - Added validation for encoded streams and palette records. - Created end-to-end tests for encoder CLI and headless player functionality. - Included sample frames and output files for testing purposes. - Updated solution file to include new projects and configurations.
There was a problem hiding this comment.
Pull request overview
Adds a new tools/Hnm0Encoder CLI to encode HNM0 streams from PNG frames, plus a headless mode for HnmPlayer and end-to-end tests to validate encoder output via the existing player parsers.
Changes:
- Added
Hnm0Encodertool: frame discovery, loading/normalization, quantization, palette record writing, chunk writing, and a stream validator. - Added headless playback/screenshot capture to
HnmPlayerand an end-to-end test project that runs both CLIs. - Updated solution + CI workflow to build and run tests on PRs.
Reviewed changes
Copilot reviewed 23 out of 28 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/Hnm0Encoder/QuantizedFrame.cs | Adds a quantized-frame DTO (palette + indices). |
| tools/Hnm0Encoder/Program.cs | Implements the encoder CLI entrypoint and preflight output. |
| tools/Hnm0Encoder/PaletteRecordWriter.cs | Writes VGA DAC palette records into an HNM payload. |
| tools/Hnm0Encoder/HnmStreamValidator.cs | Validates chunk framing/palette/frame headers for encoder output. |
| tools/Hnm0Encoder/Hnm0Writer.cs | Builds the palette chunk + per-frame chunks and writes the output file. |
| tools/Hnm0Encoder/Hnm0Encoder.csproj | New encoder project with ImageSharp dependency. |
| tools/Hnm0Encoder/FrameQuantizer.cs | Quantizes RGBA frames to indexed 256-color data. |
| tools/Hnm0Encoder/FramePreflight.cs | Preflight pass for palette/color stats across input frames. |
| tools/Hnm0Encoder/FrameLoader.cs | Loads PNGs and normalizes them to 320×200. |
| tools/Hnm0Encoder/FrameDiscovery.cs | Discovers/sorts PNG frames and selects a window by index/count. |
| tools/Hnm0Encoder/EncoderOptions.cs | Options model for the encoder CLI. |
| tools/Hnm0Encoder/CommandLineParser.cs | Parses encoder CLI arguments and validates paths/values. |
| tools/Hnm0Encoder/CanonicalFrame.cs | Adds a canonical-frame DTO for normalized pixel data. |
| tests/HnmEndToEndTests/HnmEndToEndTests.csproj | New xUnit test project for end-to-end validation. |
| tests/HnmEndToEndTests/HnmEncoderE2eTests.cs | Runs encoder + player headless and validates decoded output vs source frames. |
| logo.sln | Registers the new tool and test projects in the solution. |
| HnmPlayer/Program.cs | Adds CLI parsing and selects headless vs UI app builder. |
| HnmPlayer/PlayerLaunchContext.cs | Introduces a launch-options context + CLI parser for headless mode. |
| HnmPlayer/HnmPlayer.csproj | Adds Avalonia.Headless and compiles new headless runner files. |
| HnmPlayer/HeadlessPlayerRunner.cs | Implements headless stepping + screenshot writing. |
| HnmPlayer/HeadlessCaptureWindow.cs | Headless window that runs capture on open and then exits. |
| HnmPlayer/App.cs | Routes startup to either headless capture window or the normal UI window. |
| .github/workflows/dotnet.yml | Runs restore/build/test on PRs across OS matrix. |
Comment on lines
+21
to
+27
| while (position < palette.Length) | ||
| { | ||
| int remaining = palette.Length - position; | ||
| int count = remaining > byte.MaxValue ? byte.MaxValue : remaining; | ||
| buffer.WriteByte((byte)position); | ||
| buffer.WriteByte((byte)count); | ||
|
|
Comment on lines
+41
to
+44
| if (palette.Length == 0 || palette.Length > 256) | ||
| { | ||
| throw new InvalidDataException($"Quantized palette size {palette.Length} is outside the supported 0..256 range."); | ||
| } |
Comment on lines
+15
to
17
| [SupportedOSPlatform("windows")] | ||
| public override void OnFrameworkInitializationCompleted() | ||
| { |
Comment on lines
1
to
+4
| using Avalonia; | ||
| using Avalonia.Controls.ApplicationLifetimes; | ||
| using Avalonia.Themes.Fluent; | ||
| using System.Runtime.Versioning; |
Comment on lines
+17
to
+27
| CanonicalFrame firstFrame = FrameLoader.LoadCanonicalFrame(framePaths[0]); | ||
| QuantizedFrame firstQuantizedFrame = FrameQuantizer.Quantize(firstFrame); | ||
| chunks.Add(BuildInitialPaletteChunk(firstQuantizedFrame.Palette)); | ||
| chunks.Add(BuildFrameChunk(firstQuantizedFrame)); | ||
|
|
||
| for (int i = 1; i < framePaths.Count; i++) | ||
| { | ||
| CanonicalFrame frame = FrameLoader.LoadCanonicalFrame(framePaths[i]); | ||
| QuantizedFrame quantized = FrameQuantizer.Quantize(frame); | ||
| chunks.Add(BuildFrameChunk(quantized)); | ||
| } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 31 changed files in this pull request and generated no new comments.
Suppressed comments (7)
tools/Hnm0Encoder/FrameQuantizer.cs:44
- The exception message says the supported palette range is
0..256, but the code treats0as invalid (palette.Length == 0). This makes the error misleading when it triggers.
if (palette.Length == 0 || palette.Length > 256)
{
throw new InvalidDataException($"Quantized palette size {palette.Length} is outside the supported 0..256 range.");
}
HnmPlayer/App.cs:16
OnFrameworkInitializationCompletedis not Windows-only in Avalonia; marking the override with[SupportedOSPlatform("windows")]is inaccurate and can suppress/shift platform compatibility diagnostics rather than reflecting actual behavior. Remove the attribute unless there is truly Windows-specific code inside this method.
[SupportedOSPlatform("windows")]
public override void OnFrameworkInitializationCompleted()
tools/Hnm0Encoder/HnmStreamValidator.cs:85
ValidatePaletteChunkreturns immediately when it encounters the 0xFF terminator, without verifying that the terminator is the final byte. This allows malformed palette chunks with trailing bytes after the terminator to pass validation.
if (index == 0xFF)
{
if (!sawRecord)
{
throw new InvalidDataException("Palette chunk terminator appeared before any palette records.");
tools/Hnm0Encoder/FrameDiscovery.cs:121
CompareNaturalconsiders names likeframe1.pngandframe01.pngequal (all parts compare equal, and split-array lengths are the same).Array.Sortis not stable, so equal elements can be reordered non-deterministically, changing frame ordering between runs.
}
return ax.Length.CompareTo(bx.Length);
Directory.Build.props:4
- This globally suppresses NuGet vulnerability advisories (NU1902/NU1903) for the entire repo, which can hide real security issues in dependencies. Prefer fixing/upgrading the vulnerable packages, or scoping suppressions to the specific project/package with a clear justification.
<PropertyGroup>
<NoWarn>$(NoWarn);NU1902;NU1903</NoWarn>
tools/Hnm0Encoder/CommandLineParser.cs:69
--fpsis required and stored inEncoderOptions, but it is only printed in the summary and does not affect the encoded output. This is user-visible (changing--fpsproduces identical files) and likely indicates missing encoding of timing metadata or an unnecessary CLI argument.
if (fps is null)
{
throw new InvalidOperationException("Missing required argument --fps.");
}
tests/HnmEndToEndTests/HnmEndToEndTests.csproj:17
- The
HnmEndToEndTestsproject currently contains no test source files (no*.csundertests/HnmEndToEndTests) and no project references, so the CIdotnet teststep will run but provide no coverage for the new encoder/headless capture functionality.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net10.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<IsPackable>false</IsPackable>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="18.8.1" />
<PackageReference Include="xunit" Version="2.9.3" />
<PackageReference Include="xunit.runner.visualstudio" Version="3.1.5">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
</ItemGroup>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a HNM0 encoder (from a set of images in PNG format)