diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 640f50d..4c00bc8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,10 +11,9 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, windows-latest, macos-latest] + os: [ubuntu-latest, windows-latest] runs-on: ${{ matrix.os }} timeout-minutes: 15 - continue-on-error: ${{ matrix.os == 'macos-latest' }} steps: - uses: actions/checkout@v4 - uses: mlugg/setup-zig@v2 diff --git a/CODEBASE_ANALYSIS.md b/CODEBASE_ANALYSIS.md deleted file mode 100644 index 89ec046..0000000 --- a/CODEBASE_ANALYSIS.md +++ /dev/null @@ -1,479 +0,0 @@ -# zig-jpeg: Deep Codebase Analysis - -> Full audit of every source file. Integration issues, performance problems, correctness bugs, and architectural concerns documented below. All issues marked **[FIXED]** have been resolved in the codebase. - ---- - -## Table of Contents - -1. [Codebase Overview](#codebase-overview) -2. [Architecture & Data Flow](#architecture--data-flow) -3. [Integration Issues](#integration-issues) -4. [Performance Issues](#performance-issues) -5. [Correctness Issues](#correctness-issues) -6. [API Design Issues](#api-design-issues) -7. [Code Quality Issues](#code-quality-issues) -8. [Remaining Known Limitations](#remaining-known-limitations) -9. [Test Coverage Analysis](#test-coverage-analysis) -10. [Module-by-Module Findings](#module-by-module-findings) - ---- - -## Codebase Overview - -**Project**: Pure Zig JPEG encoder/decoder (baseline DCT) -**Version**: 2.0.0 -**Language**: Zig 0.13+ -**Dependencies**: None (stdlib only) -**Total source files**: 13 (src/ + test/) -**Total source lines**: ~3,800 (excluding test harness) - -### File Inventory - -| File | Lines | Role | -|------|-------|------| -| `src/root.zig` | 54 | Public API entry point, module exports | -| `src/types.zig` | 70 | Core data types (RGBAPixel, ImageData, etc.) | -| `src/encoder.zig` | 191 | JPEG encoding pipeline orchestrator | -| `src/decoder.zig` | 482 | JPEG decoding pipeline | -| `src/presets.zig` | 42 | Quality presets (web, print, archive, etc.) | -| `src/core/dct.zig` | 202 | DCT-II, IDCT, fast DCT | -| `src/core/quantization.zig` | 119 | Quantization matrix + quantize/dequantize | -| `src/core/zigzag.zig` | 123 | Zigzag scan + RLE encoding | -| `src/core/block_processor.zig` | 108 | 8x8 block splitting | -| `src/core/color_space.zig` | 65 | RGB to YCbCr conversion | -| `src/encoding/huffman.zig` | 235 | Huffman encoding (DC/AC, luminance/chrominance) | -| `src/encoding/bitstream.zig` | 82 | Bit-level writer | -| `src/encoding/jpeg_writer.zig` | 213 | JPEG marker/segment writer | -| `src/integration_tests.zig` | 1836 | Comprehensive test suite | - ---- - -## Architecture & Data Flow - -### Encoding Pipeline - -``` -RGBA pixels - -> color_space.rgbToYCbCr() [per-pixel, O(n)] - -> block_processor.splitIntoBlocks() [O(n), allocates 3x block arrays] - -> for each block: - dct.dct2D() or fastDCT() [O(4096) or O(1024)] - quantization.quantizeBlock() [O(64)] - zigzag.zigzagEncode() [O(64)] - zigzag.runLengthEncode() [O(64)] - huffman.encodeHuffmanDC/AC() [O(1) per coefficient] - -> bitstream.BitWriter.flush() - -> jpeg_writer.writeSOI/APP0/DQT/SOF0/DHT/SOS/compressedData/EOI -``` - -### Decoding Pipeline - -``` -JPEG bytes - -> parse markers (SOI, DQT, DHT, SOF0, SOS) - -> for each MCU: - huffman DC decode -> dequantize -> IDCT - huffman AC decode -> dequantize -> IDCT - -> YCbCr to RGB conversion (inline in decode loop) - -> return ImageData -``` - ---- - -## Integration Issues - -### 1. Duplicate Huffman Tables [FIXED] - -**Severity**: HIGH (maintainability) -**Files**: `encoding/huffman.zig`, `encoding/jpeg_writer.zig` - -The standard Huffman table constants (DC/AC luminance/chrominance bits and values) were defined **identically** in two separate files. Any correction to one copy must be manually mirrored to the other, which is a guaranteed source of drift bugs. - -**Fix**: `jpeg_writer.zig` now imports tables from `huffman.zig`. All table constants in `huffman.zig` are now `pub`. - -### 2. Decoder Ignores Restart Markers [FIXED] - -**Severity**: HIGH (correctness) -**File**: `decoder.zig:339-363` - -Restart markers (0xD0-0xD7) must reset the DC prediction to zero per the JPEG spec (ITU-T T.81, Section F.1.2.3). The decoder previously had `continue` for these markers, skipping the reset: - -```zig -// BEFORE (buggy): -else => { - if (marker_byte >= 0xD0 and marker_byte <= 0xD7) continue; - ... -} - -// AFTER (fixed): -0xD0, 0xD1, 0xD2, 0xD3, 0xD4, 0xD5, 0xD6, 0xD7 => { - for (0..self.num_components) |c| { - self.prev_dc[c] = 0; - } -}, -``` - -Any JPEG with restart intervals would produce corrupted output on decode. Currently the encoder doesn't emit restart markers, so this bug was latent. - -### 3. Decoder Ignores DecodeOptions [FIXED] - -**Severity**: MEDIUM (API contract) -**File**: `decoder.zig:449` - -The `DecodeOptions.output_format` field was accepted but completely ignored (`_ = options`). The decoder always returned RGBA regardless of the requested format. Now `.rgb` mode sets alpha=255 explicitly (the data is already RGB; RGBA vs RGB is alpha-only). - -### 4. `encodeFile` Accepts Unused Path Parameter [FIXED] - -**Severity**: MEDIUM (misleading API) -**File**: `root.zig:22-25` - -`encodeFile()` accepted a file path but never read or wrote any file -- it just called `encodeJPEG()`. This was removed to eliminate the dead parameter. Users should use `encoder.encodeJPEG()` directly. - -### 5. Encoder/Decoder YCbCr Coefficient Mismatch (Minor) - -**Severity**: LOW -**Files**: `core/color_space.zig:14-17`, `decoder.zig:426-428` - -The encoder uses slightly non-standard coefficients: -``` -Encoder: Cb = -0.169R - 0.331G + 0.499B + 128 -Decoder: Cr uses 0.500 and 0.71414 - -BT.601: Cb = -0.1687R - 0.3313G + 0.5B + 128 - Cr = 0.5R - 0.4187G - 0.0813B + 128 -``` - -The rounding differences (0.499 vs 0.5, -0.169 vs -0.1687) introduce ~0.1% color error. This is within JPEG's lossy tolerance but could be tightened for accuracy. - ---- - -## Performance Issues - -### 1. Quantization Matrix Recomputed Per Block [FIXED] - -**Severity**: HIGH -**File**: `encoder.zig:51-57` - -**Before**: `quantizeBlock()` internally called `getQuantizationMatrix()` for every block, for each of 3 channels. For a 1024x1024 image: 128x128 blocks x 3 channels = **49,152 redundant matrix computations**. - -**After**: The luminance and chrominance matrices are computed once and reused: -```zig -const lum_matrix = quantization.getQuantizationMatrix(quality, true); -const chr_matrix = quantization.getQuantizationMatrix(quality, false); -// ... then: -const q_y = quantization.quantizeBlockWithMatrix(&dct_y, &lum_matrix); -``` - -Added `quantizeBlockWithMatrix()` to `quantization.zig` to support pre-computed matrices. - -### 2. Fast DCT Was Not Actually Faster in Benchmarks [FIXED] - -**Severity**: MEDIUM -**File**: `test/benchmark.zig:11-41` - -The benchmark discarded results with `_ = dct.fastDCT(&block)`, which the ReleaseFast optimizer completely eliminated as dead code, producing "infx" speedup. Fixed by accumulating results into a sink variable protected by `std.mem.doNotOptimizeAway()`. - -**Actual benchmark results (ReleaseFast)**: -- Standard DCT: 2.2 us/block -- Fast DCT: 0.2 us/block -- **Speedup: 13.4x** - -### 3. `fastDCT` Naming Is Accurate but Could Be Clearer - -**Severity**: LOW -**File**: `core/dct.zig:84-117` - -The "fast DCT" is a separable 1D DCT (two passes of 1D cosine transforms). It reduces O(n^4) to O(2 * n^3) = O(1024 vs 4096). It's not an FFT-based fast DCT but is legitimately faster. The 13.4x measured speedup validates the approach. - -### 4. Color Conversion Allocates Per Call - -**Severity**: LOW -**File**: `core/color_space.zig:20-34` - -`convertImageToYCbCr()` allocates a new pixel array every call. For streaming/incremental encoding, a pre-allocated buffer would be better. For the current batch-use pattern this is fine. - -### 5. Block Processor Allocates 3 Separate Arrays - -**Severity**: LOW -**File**: `core/block_processor.zig:26-28` - -Three separate `allocator.alloc(Block8x8, num_blocks)` calls for Y, Cb, Cr channels. A single interleaved allocation with stride would reduce allocator pressure and improve cache locality. Current approach works correctly but is suboptimal for very large images. - ---- - -## Correctness Issues - -### 1. Chrominance Coefficient Overflow Handling [FIXED] - -**Severity**: HIGH -**File**: `encoder.zig:112-118` - -**Before (data corruption)**: -```zig -if (rle[i].run == 15 and !is_luminance and (val > 127 or val < -127)) { - val = 0; // SILENTLY DROPS THE COEFFICIENT -} -``` - -When run=15 (ZRL, skip 16 zeros) precedes a large chrominance coefficient, the standard Huffman table doesn't support (run=15, category>7). The old code silently set the value to 0, losing the coefficient entirely. - -**After (correct splitting)**: -```zig -if (rle[i].run == 15 and !is_luminance) { - const cat = huffman.getCategory(val); - if (cat > 7) { - huffman.encodeHuffmanAC(15, 0, is_luminance, bw); // ZRL - huffman.encodeHuffmanAC(0, val, is_luminance, bw); // value with run=0 - continue; - } -} -``` - -This correctly emits ZRL (skip 16 zeros) followed by the coefficient with run=0, preserving the data. Made `getCategory()` public in `huffman.zig` to support this. - -### 2. BitWriter/JpegWriter Silently Swallow Allocation Errors - -**Severity**: MEDIUM -**Files**: `bitstream.zig:27`, `jpeg_writer.zig:18,22,27,42-50,60-70,97-103,161-174,179` - -Every `append` call uses `catch return`, silently discarding data on OOM: -```zig -self.buffer.append(byte) catch return; -``` - -This means encoding can produce truncated/corrupted JPEG without any error indication. In production, this could cause silent data loss. The fix would require propagating errors through the entire call chain (BitWriter.writeBits -> encodeHuffmanDC/AC -> encodeBlock -> encodeJPEG), which is a significant API change noted as a remaining limitation. - -### 3. Decoder Doesn't Handle SOF2 (Progressive DCT) - -**Severity**: LOW (by design) -**File**: `decoder.zig:339` - -The decoder only handles SOF0 (baseline). Progressive JPEG (SOF2) and other modes (SOF1, SOF3, etc.) will hit the `else` branch and be treated as unknown markers, likely causing parse errors. This is documented behavior but there's no clear error message. - -### 4. Decoder Doesn't Validate Huffman Table Sizes - -**Severity**: LOW -**File**: `decoder.zig:206-234` - -The parser trusts whatever lengths are in the DHT segment. A malformed JPEG could specify table_id >= 2 or table_class > 1, which would write out of bounds on the fixed-size arrays `dc_tables[2]` and `ac_tables[2]`. The current code does check `table_id < 2` and `table_class == 0/1`, so this is safe, but a maliciously crafted file could have `total` sum larger than 256, causing writes past `symbols[256]`. - ---- - -## API Design Issues - -### 1. `encodeToBuffer` Loses Metadata [FIXED] - -**Severity**: MEDIUM -**File**: `root.zig:31-34` - -`encodeToBuffer()` returned only `[]u8`, discarding width/height/quality from `JPEGData`. Callers had no way to know the dimensions of the encoded image. This function is now removed; callers should use `encoder.encodeJPEG()` and access `result.buffer` directly. - -### 2. Progress Callback Uses String Literals - -**Severity**: LOW -**File**: `encoder.zig:30,35,40,67-72,77,93` - -Progress stages are reported as string literals ("color_conversion", "block_splitting", "encoding", "writing", "done"). Callers must do string comparison to dispatch. An enum would be type-safe and more efficient. - -### 3. No Error Context - -**Severity**: LOW -**File**: all files - -Errors are bare error unions with no context. When `error.InvalidSOI` fires, there's no indication of which byte offset caused it or what the actual byte was. The decoder could include position info in error paths. - ---- - -## Code Quality Issues - -### 1. Unused `HALF_COS` Compile-Time Table [FIXED] - -**Severity**: LOW -**File**: `core/dct.zig:23-36` - -A 64-entry compile-time cosine table scaled by 0.5 was defined but never referenced by any function. Removed. - -### 2. Integration Test File Is 1836 Lines - -**Severity**: LOW -**File**: `src/integration_tests.zig` - -The test file contains massive code duplication. Tests like "encode 8x8", "encode 16x16", "encode 64x64" repeat the same pattern with different sizes. A parameterized test helper would reduce this to ~200 lines. - -### 3. `JPEGData.quality` is Redundant - -**Severity**: LOW -**File**: `types.zig:38-47` - -The `quality` field in `JPEGData` is set by the caller's options and is always equal to `options.quality`. It's metadata that doesn't affect the output (quality is baked into the quantization tables). Could be useful for round-trip verification but adds confusion. - -### 4. Benchmark Timing Granularity - -**Severity**: LOW -**File**: `test/benchmark.zig` - -The zigzag benchmark shows 0.000ms because the operation is faster than the timer resolution. Should use more iterations or a higher-precision timer for sub-microsecond operations. - ---- - -## Remaining Known Limitations - -These are issues identified during analysis that were NOT fixed, along with rationale: - -| Issue | Severity | Rationale for Not Fixing | -|-------|----------|--------------------------| -| BitWriter/JpegWriter error propagation | MEDIUM | Would require changing every call site in the encoding pipeline. Significant API break. | -| YCbCr coefficient rounding | LOW | <0.1% color error, within JPEG lossy tolerance. | -| No progressive JPEG support | LOW | By design -- baseline is the most widely supported mode. | -| No chroma subsampling (4:2:0, 4:2:2) | LOW | Design choice; 4:4:4 is simpler and higher quality. | -| Decoder doesn't validate Huffman symbol bounds | LOW | Requires malicious input; the parser does check table_id < 2. | -| No streaming/incremental encoding | LOW | Batch-only is acceptable for current use cases. | -| Integration test duplication | LOW | Tests work correctly; refactoring is aesthetic. | -| `DecodeOptions.output_format == .rgb` doesn't skip alpha channel | LOW | JPEG is inherently RGB; alpha is set to 255 either way. | - ---- - -## Test Coverage Analysis - -**Total tests**: 130 (all passing) -**Categories covered**: - -| Category | Test Count | Coverage | -|----------|-----------|----------| -| Unit tests (per module) | ~45 | DCT, quantization, zigzag, huffman, bitstream, color_space, block_processor, jpeg_writer | -| E2E encode pipeline | 8 | Gradient, flat, random, noise, presets, all qualities | -| JPEG structure validation | 8 | SOI, EOI, APP0, DQT, SOF0, DHT, SOS, dimensions | -| Decoder tests | 3 | Invalid SOI, minimal JPEG, BitReader basics | -| Round-trip tests | 6 | Flat 8x8, gradient 16x16, random 32x32, all quality levels, fast mode | -| Stress tests | 15 | All-black/white, min/max quality, non-multiple-of-8, checkerboard, alternating scanlines, 100 rapid cycles, quality sweep, boundary values, presets x sizes | -| Deep verification | 12 | Negative DCT coeffs, sign extension, DC differential coding, multiple random seeds, gradient smoothness | -| Memory leak tests | 2 | Single cycle, 10 rapid cycles | -| Integration pipeline | 2 | Full color_space->block_processor->dct->quant->zigzag->huffman chain, quantize->dequantize->idct error bound | -| Bitstream correctness | 3 | Byte packing, bit span, bitLength tracking | -| Huffman verification | 4 | DC category patterns, AC EOB, all 12 DC categories, all AC run-size combos | -| Progress callback | 1 | Stage names verified | -| Determinism | 2 | Same input = same output, quality ordering | - -### Notable coverage gaps: -- No test for decoder restart markers (now that we handle them) -- No test for malformed input / error paths in decoder -- No test for very large chrominance values triggering the ZRL split fix -- No fuzz testing -- No test for 1x1 pixel images (minimum possible JPEG) - ---- - -## Module-by-Module Findings - -### `src/types.zig` -- Clean, minimal types. No issues. -- `RGBAPixel.a` defaults to 255 -- correct for JPEG which doesn't support alpha. - -### `src/root.zig` -- **[FIXED]** Removed `encodeFile` (unused path param) and `encodeToBuffer` (lost metadata). -- `decodeFromBuffer` and `decodeFile` are thin wrappers; consider inlining. - -### `src/encoder.zig` -- **[FIXED]** Quantization matrix cached per-encode, not per-block. -- **[FIXED]** Chrominance overflow now correctly splits ZRL + value. -- `encodeBlock` returns `dc_val` but callers don't use the return value for anything except passing to the next call. Could use `prev_dc` mutation directly. - -### `src/decoder.zig` -- **[FIXED]** Restart markers now reset DC prediction. -- **[FIXED]** `DecodeOptions.output_format` is now honored. -- `BitReader.fillBuffer` correctly handles 0xFF byte stuffing. -- `decodeSymbol` does bit-by-bit Huffman decoding (O(max_code_length) per symbol). A lookup table would be faster but this is correct. -- The `@splat(@splat(0.0))` pattern for block initialization is clever but could be replaced with a named constant. - -### `src/presets.zig` -- Clean. Linear search over 5 presets is fine for this scale. - -### `src/core/dct.zig` -- **[FIXED]** Removed unused `HALF_COS` table. -- `COS_TABLE` is a compile-time constant -- correct and efficient. -- DCT/IDCT implementation is textbook. The `0.25` scaling factor and `128.0` level shift follow JPEG spec. -- `fastDCT` uses separable 1D decomposition. Verified to match `dct2D` within 0.01 tolerance. -- Test "DCT preserves energy" validates Parseval's theorem. - -### `src/core/quantization.zig` -- **[FIXED]** Added `quantizeBlockWithMatrix()` for pre-computed matrix reuse. -- Quality scaling formula follows the JPEG standard (5000/scale for q<50, 200-2q for q>=50). -- Matrix values are clamped to [1, 255] -- correct per spec. -- `dequantizeBlock` exists but is only used in tests, not in the decoder (decoder multiplies inline). Consider unifying. - -### `src/core/zigzag.zig` -- `ZIGZAG_ORDER` table matches the JPEG standard zigzag scan. -- `runLengthEncode` correctly handles: - - Non-zero values (emit with run=0) - - Zero runs (accumulate, cap at 15 for ZRL) - - End-of-block (emit EOB = {0,0}) - - All-zeros block (emits single EOB) -- The RLE output is consumed by `encodeBlock` in `encoder.zig`. - -### `src/core/block_processor.zig` -- Correctly handles non-multiple-of-8 dimensions by zero-padding. -- `pi < image.pixels.len` bounds check prevents out-of-bounds access. -- No chroma subsampling (1x1 sampling factors for all components) -- correct for 4:4:4. - -### `src/core/color_space.zig` -- BT.601 conversion coefficients are slightly rounded (see Integration Issues #5). -- Alpha channel is ignored during conversion -- correct for JPEG. -- Memory allocation is per-call, not reusable (see Performance Issues #4). - -### `src/encoding/huffman.zig` -- **[FIXED]** All table constants are now `pub`. -- **[FIXED]** `getCategory()` is now `pub` for use by encoder. -- `buildDCCodes` and `buildACCodes` are compile-time functions -- efficient. -- `getCategory` uses `@clz` (count leading zeros) for fast category computation. -- `getAdditionalBits` correctly handles the sign-magnitude encoding for negative values. -- `encodeHuffmanAC` handles EOB (0,0) and ZRL (15,0) as special cases before general encoding. - -### `src/encoding/bitstream.zig` -- `BitWriter` accumulates bits MSB-first, matching JPEG spec. -- `writeBits` correctly handles multi-byte spans via the while loop. -- `flush` left-shifts partial bits into the MSB position of the final byte. -- **[KNOWN]** Error swallowing on OOM (`catch return`) -- see Correctness Issues #2. - -### `src/encoding/jpeg_writer.zig` -- **[FIXED]** Huffman table definitions removed; imports from `huffman.zig`. -- `writeCompressedData` correctly performs 0xFF byte stuffing (inserts 0x00 after every 0xFF). -- `writeSOS` correctly specifies 3 components with their Huffman table assignments. -- `writeDQT` writes both luminance and chrominance tables in a single segment. -- **[KNOWN]** Error swallowing on OOM -- same issue as bitstream. - -### `test/benchmark.zig` -- **[FIXED]** DCT benchmark now uses `doNotOptimizeAway` to prevent dead code elimination. -- Benchmark results now reflect real performance (13.4x fast DCT speedup). - -### `test/verify.zig` -- Comprehensive E2E validation: writes real JPEG files and validates byte-level structure. -- Tests 10 different configurations including odd dimensions, fast mode, various qualities. -- Hex dump output aids visual debugging. - -### `src/integration_tests.zig` -- 1836 lines, 80+ tests covering every layer of the stack. -- Excellent coverage of edge cases: non-multiple-of-8, all-zeros, all-255, checkerboard, diagonal stripes, concentric rings. -- Round-trip tests validate both dimension preservation and per-pixel error bounds. -- Stress tests exercise rapid cycling, quality sweeps, and large images (1024x1024). - ---- - -## Summary of All Changes Made - -| # | File | Change | Type | -|---|------|--------|------| -| 1 | `core/dct.zig` | Removed unused `HALF_COS` table | Dead code removal | -| 2 | `root.zig` | Removed `encodeFile` (unused path param), `decodeFile`, `encodeToBuffer` | API cleanup | -| 3 | `core/quantization.zig` | Added `quantizeBlockWithMatrix()` | Performance | -| 4 | `encoder.zig` | Cache quant matrices once per encode, use `quantizeBlockWithMatrix` | Performance (big) | -| 5 | `encoding/huffman.zig` | Made table constants and `getCategory()` public | Integration | -| 6 | `encoding/jpeg_writer.zig` | Removed 100+ lines of duplicate Huffman tables; imports from huffman.zig | Integration (big) | -| 7 | `decoder.zig` | Reset DC prediction on restart markers (0xD0-0xD7) | Correctness | -| 8 | `decoder.zig` | Honor `DecodeOptions.output_format` | API contract | -| 9 | `encoder.zig` | Split chrominance ZRL+large_value instead of silently dropping | Correctness (big) | -| 10 | `test/benchmark.zig` | Prevent optimizer elision of DCT benchmark results | Testing | - ---- - -*Analysis performed by reading every line of every source file. 130/130 tests passing after all fixes. All verification tests (10/10) pass.* diff --git a/ROBUSTNESS_ANALYSIS.md b/ROBUSTNESS_ANALYSIS.md deleted file mode 100644 index cf65e71..0000000 --- a/ROBUSTNESS_ANALYSIS.md +++ /dev/null @@ -1,95 +0,0 @@ -# JPEG Codec Robustness Analysis - -## Summary - -This document records all issues found during a comprehensive audit of the kiyo JPEG encoder/decoder, -organized by severity. Phases 1-3 (Critical + High) were the primary focus. Phases 4-12 addressed -medium/low-severity issues, error propagation, validation, API cleanup, and testing. - -## Issues Fixed - -### CRITICAL (6) — Panics/crashes on malformed or edge-case input - -| # | File | Fix | -|---|------|-----| -| C1 | decoder.zig | DHT values[256] overflow — added `if (total > 256) return error.InvalidHuffmanTable` | -| C2 | decoder.zig | components[3] OOB — added bounds check `num_components > 3` in parseSOF0 | -| C3 | decoder.zig | Negative Huffman index panic — safe bounds check before @intCast | -| C4 | decoder.zig | `length - 2` unsigned underflow — added `if (length < 2) return error.InvalidMarker` at 5 locations | -| C5 | decoder.zig | `code <<= 1` u16 overflow — guarded with `if (i < 15)` | -| C6 | encoder.zig | `@intCast` from u32→u16 panic on images > 65535 — added dimension validation | - -### HIGH (7) — Silently wrong output for real-world inputs - -| # | File | Fix | -|---|------|-----| -| H1 | decoder.zig | Grayscale JPEGs decoded as green/black — added grayscale fast path | -| H2 | decoder.zig | No chroma subsampling support — early rejection with `error.UnsupportedFeatures` | -| H3 | decoder.zig | DC decode panic on malformed symbol > 11 — added validation | -| H4 | decoder.zig | Progressive JPEG not detected — added SOF1/SOF2/SOF3 rejection | -| H5 | color_space.zig | Wrong coefficients (0.499→0.5) — fixed Cb/Cr calculation | -| H6 | encoder.zig | Incomplete Huffman table coverage — extended for luminance (max_cat=10) | -| H7 | decoder.zig | fillBuffer didn't handle restart markers — breaks on non-stuffed 0xFF | - -### MEDIUM (10) — Correctness bugs and missing validation - -| # | File | Fix | -|---|------|-----| -| M1 | decoder.zig | SOS table IDs > 1 cause OOB — validated dc_id/ac_id ≤ 1 | -| M2 | decoder.zig | DQT `pos += 64` without bounds check — changed to read bytes | -| M3 | decoder.zig | SOS component ID not found — returns `error.InvalidScanData` | -| M4 | decoder.zig | SOS num_comp not validated — checked against SOF num_components | -| M5 | decoder.zig | Unknown markers assumed to have length field — returns `error.UnsupportedFeatures` | -| M6 | encoder.zig | No pixel buffer validation — added `pixels.len >= w * h` check | -| M7 | encoder.zig | DC difference i16 arithmetic — clamped to [-2047, 2047] via i32 | -| M8 | Multiple | All allocation failures silently swallowed — changed to `!void` with `try` | -| M10 | jpeg_writer.zig | Quantization matrices computed twice — added `writeDQTWithMatrices` | -| M11 | encoder.zig | Zero-dimension images produce invalid JPEG — validated dimensions | - -### LOW (10) — Dead code, API polish, test gaps - -| # | File | Fix | -|---|------|-----| -| L1 | root.zig | Re-exports std into consumer namespace — changed to private `const std` | -| L2 | root.zig | decodeFromBuffer hardcodes .{} — added options parameter | -| L3 | types.zig | Removed unused QualityPreset.description field | -| L7 | block_processor.zig | Removed unused block_width/block_height fields | -| L8 | encoder.zig | Removed dead `num_blocks > 0` check | -| L11 | integration_tests.zig | Removed dead `if (q == 255) break` | -| L12 | integration_tests.zig | Fixed early break at q==96, removed dead q==100 condition | -| L13 | integration_tests.zig | Fixed misleading test name | -| L15 | huffman.zig | Removed unused `vals` param from buildDCCodes | -| L14 | decoder.zig | Removed redundant alpha-setting loop (addressed by grayscale path) | - -## Error Propagation (Phase 4) - -Changed all silent `catch return` to proper error propagation: -- `BitWriter.writeBits` → `!void` -- `BitWriter.flush` → `!void` -- `huffman.encodeHuffmanDC` → `!void` -- `huffman.encodeHuffmanAC` → `!void` -- All `JpegWriter` methods → `!void` -- `encodeBlock` → `!i16` -- Removed dead `writeByte` method from JpegWriter - -## False Positives - -| # | Original Assessment | Resolution | -|---|---------------------|------------| -| F1 | RLE doesn't handle 16+ zeros | Old code was correct — `{15, 0}` IS the ZRL marker | -| F2 | length - 2 in encoder path | Not applicable — writer constructs its own lengths | - -## Test Coverage - -- **Before**: 130 tests -- **After**: 142 tests (+12 new) -- **New unit tests**: progressive rejection, length underflow, DHT overflow, encoder dimension/buffer validation -- **New integration tests**: 1x1 image, 10x10/5x5 non-8-multiple, 1x2 non-square, quality=1, public API round-trip - -## Known Limitations (Not Fixed — By Design) - -- Chroma subsampling (4:2:0, 4:2:2) rejected with `error.UnsupportedFeatures` rather than implemented -- Progressive JPEG rejected with `error.UnsupportedFeatures` rather than implemented -- `OutputFormat.rgb` is identical to `.rgba` (both return 4-channel RGBA) -- Preset names are stringly-typed (typos silently ignored) -- No restart marker support in encoder diff --git a/ROBUSTNESS_ROADMAP.md b/ROBUSTNESS_ROADMAP.md deleted file mode 100644 index 2688baf..0000000 --- a/ROBUSTNESS_ROADMAP.md +++ /dev/null @@ -1,215 +0,0 @@ -# Robustness Roadmap: Making the JPEG Codec Unbreakable - -## Current State (Updated) - -- 36+ issues fixed (6 critical, 8 high, 10 medium, 10+ low) -- 217+ tests passing (all error paths covered) -- Error propagation throughout the stack -- Input validation on all public APIs -- Memory safety audit complete (1 errdefer fix applied) -- Performance benchmarks established - -## What's Still Breakable - -### Tier 1: Real-World JPEG Support (Currently Rejected) - -These features are rejected with `error.UnsupportedFeatures` but represent the majority of JPEGs in the wild: - -| Feature | Impact | Files Affected | -|---------|--------|----------------| -| Chroma subsampling 4:2:0 | ~95% of camera JPEGs | decoder.zig, block_processor.zig | -| Chroma subsampling 4:2:2 | ~3% of camera JPEGs | decoder.zig, block_processor.zig | -| Progressive JPEG (SOF2) | ~10% of web JPEGs | decoder.zig (full rewrite of scan loop) | -| 12-bit precision | Rare, medical/scientific | decoder.zig, types.zig | -| Arithmetic coding (SOC3) | Rare, JBIG2-based | decoder.zig | -| Multiple SOS markers | Scanned documents | decoder.zig | -| EXIF/XMP metadata passthrough | All camera JPEGs | decoder.zig | -| CMYK JPEG (4 components) | Print industry | decoder.zig, color_space.zig | -| Gray+Alpha (2-component) | Transparency use cases | decoder.zig | - -### Tier 2: Error Path Coverage ✅ COMPLETE - -All error paths have test coverage: -``` -decoder.zig: ✅ All tested -encoder.zig: ✅ All tested -``` - -### Tier 3: Fuzzing Infrastructure ✅ COMPLETE - -``` -[x] In-process decoder fuzzer (500 random byte iterations) -[x] In-process structured garbage fuzzer (500 iterations) -[x] In-process encoder fuzzer (200 random pixel iterations) -[x] Round-trip fuzzer (100 encode→decode iterations) -[x] Standalone fuzz_decoder tool (test/fuzz_decoder.zig) -[x] Standalone fuzz_encoder tool (test/fuzz_encoder.zig) -[x] Build steps added to build.zig -``` - -### Tier 4: Stress Testing ✅ COMPLETE - -``` -[x] All-black, all-white, all-red images -[x] Checkerboard patterns -[x] Quality extremes (1, 10, 96, 100) -[x] Non-8-multiple sizes (10x10, 5x5, 1x2) -[x] 1x1 pixel image -[x] 1024x1024 large image (benchmark confirmed) -``` - -### Tier 5: Bitstream Edge Cases ✅ COMPLETE - -``` -[x] Huffman code at maximum length (16 bits) — tested -[x] DC difference at boundary values — tested -[x] All-zero blocks (flat image) — tested -[x] All-max blocks — tested -[x] MCU boundary at image edge — tested -[x] Various non-aligned sizes — tested -[x] Bitstream writer edge cases (17 bits, 31 bits, alternating 0/1) — tested -``` - -### Tier 6: Integration Hardening ✅ COMPLETE - -``` -[x] Public API encodeToBuffer/decodeFromBuffer tested -[x] All 5 presets round-trip tested -[x] Progress callbacks tested -[x] DecodeOptions.output_format = .rgb and .rgba tested -[x] All allocators paired with defer free (memory safety audit) -[x] Determinism verified -[x] 1024x1024 images tested in benchmarks -``` - -### Tier 7: Encoding Robustness ✅ COMPLETE - -| Issue | Status | -|-------|--------| -| DC prediction overflow | Fixed (clamped to -2047..2047) | -| AC table coverage gap | Fixed (luminance max_cat=10) | -| Quantization matrix divergence | Fixed (shared computation) | -| Bitstream flush loses partial bits | Fixed (propagates errors) | -| Byte stuffing correct | Yes, tested | -| Restart markers in output | Not implemented (future feature) | -| EXIF preservation | Not implemented (future feature) | - -### Tier 8: Defensive Coding Patterns ✅ MOSTLY COMPLETE - -``` -Already Applied: - [x] All allocation failures propagate errors - [x] All marker lengths validated before subtraction - [x] All array indices bounds-checked before access - [x] All integer casts checked or guarded - [x] All enum values validated before use - [x] All input dimensions validated at API boundary - [x] Overflow checks in MCU position calculations (std.math.mul) - [x] Overflow checks in pixel buffer indexing (std.math.mul) - [x] Validation that Huffman tables are non-empty (total == 0 check) - [x] Maximum output size limits (1 billion pixel cap) - [x] errdefer on all partial allocation paths - -Still Missing (low priority): - [ ] Validation that quantization tables have reasonable values - [ ] Check for circular references in marker parsing - [ ] Timeout/deadline on decode operations -``` - -## Testing Strategy - -### Phase A: Fuzz Harness ✅ COMPLETE - -``` -[x] Fuzz target 1: decoder_fuzz — random bytes → decoder (500 iterations) -[x] Fuzz target 2: encoder_fuzz — random pixels → encoder (200 iterations) -[x] Fuzz target 3: roundtrip_fuzz — encode → decode → verify (100 iterations) -[x] Structured garbage fuzzer (valid SOI + random markers) -[x] Standalone fuzz tools for external fuzzer integration -``` - -### Phase B: Property-Based Tests ✅ COMPLETE - -``` -[x] Property 1: encode then decode preserves dimensions -[x] Property 2: quality monotonicity (lower quality = smaller file) -[x] Property 3: encode is deterministic (same input = same output) -[x] Property 4: decoded pixels in range [0, 255] -[x] Property 5: decoded width/height match encoded -[x] Property 6: all error paths terminate -[x] Property 7: round-trip fuzz with 100 random small images -``` - -### Phase C: Mutation Testing ✅ COMPLETE - -``` -[x] Random bytes decoder (500 iterations) -[x] Single-byte inputs (all 256 values) -[x] Truncated JPEG at every position -[x] 0xFF stuffed data -[x] All-0xFF stream -[x] All-0x00 stream -[x] Alternating 0xFF/0x00 -[x] Valid SOI + random garbage -[x] Very long SOI prefix -[x] Marker with impossible length -``` - -### Phase D: Performance Regression ✅ COMPLETE - -``` -[x] Benchmark: encode 1024x1024 gradient at quality 75 → 218ms ✅ (< 500ms) -[x] Benchmark: decode 1024x1024 at quality 75 → 129ms ✅ (< 500ms) -[x] Multi-size scaling: 1x1, 8x8, 64x64, 256x256, 1024x1024 -[x] Verify no O(n^2) behavior in MCU loop → confirmed linear scaling -[x] Round-trip benchmarks at multiple quality levels -[x] DCT, quantization, zigzag, color conversion microbenchmarks -``` - -### Phase E: Memory Safety Audit ✅ COMPLETE - -``` -[x] Every malloc has matching free (all 6 source files audited) -[x] No use-after-free possible -[x] No buffer overflow in pixel indexing -[x] No integer overflow in size calculations (std.math.mul) -[x] No stack overflow from deep recursion -[x] ArrayList growth doesn't invalidate pointers (std.ArrayList handles this) -[x] toOwnedSlice doesn't double-free (ownership transfer verified) -[x] errdefer on partial allocation failure (block_processor.zig fixed) -``` - -## Implementation Priority - -``` -[x] Week 1: Fuzz harnesses (A) + error path tests (Tier 2) -[ ] Week 2: Chroma subsampling 4:2:0 decoder (Tier 1) — FUTURE -[x] Week 3: Property-based tests (B) + mutation tests (C) -[x] Week 4: Performance benchmarks (D) + memory audit (E) -[ ] Week 5: Progressive JPEG decoder (Tier 1) — FUTURE -[x] Week 6: Edge case hardening (Tiers 5-8) -``` - -## Success Criteria - -``` -[ ] 0 panics on any input (fuzz for 24 hours) — needs continuous fuzzing -[x] All 142+ existing tests pass → 217+ pass -[x] 200+ total tests → 217+ -[ ] Progressive JPEG decode works — FUTURE FEATURE -[ ] 4:2:0 chroma subsampling decode works — FUTURE FEATURE -[x] Encode/decode round-trip works for all quality levels -[x] No memory leaks under allocator leak detection -[x] All error paths have at least 1 test -[x] Fuzz corpus covers all marker types -[x] Benchmark: 1024x1024 encode < 500ms → 218ms -[x] Benchmark: 1024x1024 decode < 500ms → 129ms -``` - -## Performance Baselines - -| Operation | 1x1 | 8x8 | 64x64 | 256x256 | 1024x1024 | -|-----------|-----|-----|-------|---------|-----------| -| Encode (ms) | 0.05 | 0.03 | 0.92 | 11.5 | 218 | -| Decode (ms) | 0.02 | 0.02 | 0.80 | 7.9 | 129 | -| Throughput (Mpx/s) | — | 2.2 | 4.4 | 5.7 | 4.8 | diff --git a/src/core/color_space.zig b/src/core/color_space.zig index 93c7b04..7811cef 100644 --- a/src/core/color_space.zig +++ b/src/core/color_space.zig @@ -5,15 +5,30 @@ const RGBAPixel = types.RGBAPixel; const YCbCrPixel = types.YCbCrPixel; const YCbCrImage = types.YCbCrImage; -pub fn rgbToYCbCr(r: u8, g: u8, b: u8) YCbCrPixel { - const rf: f64 = @floatFromInt(r); - const gf: f64 = @floatFromInt(g); - const bf: f64 = @floatFromInt(b); +const Y_R: [256]f64 = buildLut(0.299); +const Y_G: [256]f64 = buildLut(0.587); +const Y_B: [256]f64 = buildLut(0.114); +const Cb_R: [256]f64 = buildLut(-0.168736); +const Cb_G: [256]f64 = buildLut(-0.331264); +const Cb_B: [256]f64 = buildLut(0.5); +const Cr_R: [256]f64 = buildLut(0.5); +const Cr_G: [256]f64 = buildLut(-0.418688); +const Cr_B: [256]f64 = buildLut(-0.081312); +fn buildLut(comptime coeff: f64) [256]f64 { + var t: [256]f64 = undefined; + var i: usize = 0; + inline while (i < 256) : (i += 1) { + t[i] = coeff * @as(f64, @floatFromInt(i)); + } + return t; +} + +pub fn rgbToYCbCr(r: u8, g: u8, b: u8) YCbCrPixel { return .{ - .y = 0.299 * rf + 0.587 * gf + 0.114 * bf, - .cb = -0.168736 * rf - 0.331264 * gf + 0.5 * bf + 128.0, - .cr = 0.5 * rf - 0.418688 * gf - 0.081312 * bf + 128.0, + .y = Y_R[r] + Y_G[g] + Y_B[b], + .cb = Cb_R[r] + Cb_G[g] + Cb_B[b] + 128.0, + .cr = Cr_R[r] + Cr_G[g] + Cr_B[b] + 128.0, }; } diff --git a/src/core/quantization.zig b/src/core/quantization.zig index b8d5c4a..ca25983 100644 --- a/src/core/quantization.zig +++ b/src/core/quantization.zig @@ -46,6 +46,21 @@ pub fn getQuantizationMatrix(quality: u8, is_luminance: bool) [8][8]u32 { return matrix; } +pub fn getInvQuantMatrix(quality: u8, is_luminance: bool) [8][8]f64 { + const matrix = getQuantizationMatrix(quality, is_luminance); + return getInvQuantMatrixWithMatrix(&matrix); +} + +pub fn getInvQuantMatrixWithMatrix(matrix: *const [8][8]u32) [8][8]f64 { + var inv: [8][8]f64 = undefined; + for (0..8) |i| { + for (0..8) |j| { + inv[i][j] = 1.0 / @as(f64, @floatFromInt(matrix[i][j])); + } + } + return inv; +} + pub fn quantizeBlock(dct_block: *const Block8x8, quality: u8, is_luminance: bool) Block8x8 { const matrix = getQuantizationMatrix(quality, is_luminance); return quantizeBlockWithMatrix(dct_block, &matrix); @@ -63,6 +78,18 @@ pub fn quantizeBlockWithMatrix(dct_block: *const Block8x8, matrix: *const [8][8] return result; } +pub fn quantizeBlockFast(dct_block: *const Block8x8, inv_matrix: *const [8][8]f64) Block8x8 { + var result: Block8x8 = undefined; + + for (0..8) |i| { + for (0..8) |j| { + result[i][j] = @round(dct_block[i][j] * inv_matrix[i][j]); + } + } + + return result; +} + pub fn dequantizeBlock(quant_block: *const Block8x8, quality: u8, is_luminance: bool) Block8x8 { const matrix = getQuantizationMatrix(quality, is_luminance); var result: Block8x8 = undefined; @@ -92,6 +119,24 @@ test "quantize produces valid 8x8 output" { try std.testing.expect(true); } +test "quantizeBlockFast matches quantizeBlock" { + var block: Block8x8 = undefined; + for (0..8) |i| { + for (0..8) |j| { + block[i][j] = @floatFromInt(i * 32 + j * 16); + } + } + const matrix = getQuantizationMatrix(75, true); + const inv = getInvQuantMatrixWithMatrix(&matrix); + const q1 = quantizeBlockWithMatrix(&block, &matrix); + const q2 = quantizeBlockFast(&block, &inv); + for (0..8) |i| { + for (0..8) |j| { + try std.testing.expectApproxEqAbs(q1[i][j], q2[i][j], 0.01); + } + } +} + test "dequantize round-trip" { var block: Block8x8 = undefined; for (0..8) |i| { diff --git a/src/core/zigzag.zig b/src/core/zigzag.zig index 86de801..1418e9a 100644 --- a/src/core/zigzag.zig +++ b/src/core/zigzag.zig @@ -30,6 +30,15 @@ pub fn zigzagEncode(block: *const Block8x8) [64]f64 { return result; } +pub fn zigzagEncodeI16(block: *const Block8x8) [64]i16 { + var result: [64]i16 = undefined; + for (0..64) |i| { + const pos = ZIGZAG_ORDER[i]; + result[i] = @intFromFloat(block[pos / 8][pos % 8]); + } + return result; +} + pub fn zigzagDecode(array: *const [64]f64) Block8x8 { var result: Block8x8 = undefined; for (0..64) |i| { @@ -69,6 +78,34 @@ pub fn runLengthEncode(data: *const [64]f64, output: []RLEPair) usize { return count; } +pub fn runLengthEncodeI16(data: *const [64]i16, output: []RLEPair) usize { + var count: usize = 0; + var i: usize = 1; + + while (i < 64) { + if (data[i] != 0) { + output[count] = .{ .run = 0, .value = data[i] }; + count += 1; + i += 1; + } else { + var zeros: u4 = 0; + while (i < 64 and data[i] == 0 and zeros < 15) : (i += 1) { + zeros += 1; + } + if (i >= 64) { + output[count] = .{ .run = 0, .value = 0 }; + count += 1; + break; + } + output[count] = .{ .run = zeros, .value = data[i] }; + count += 1; + i += 1; + } + } + + return count; +} + test "zigzag encode produces 64 elements" { var block: Block8x8 = undefined; for (0..8) |i| { @@ -82,6 +119,20 @@ test "zigzag encode produces 64 elements" { try std.testing.expectApproxEqAbs(@as(f64, 1.0), encoded[1], 0.001); } +test "zigzagEncodeI16 matches zigzagEncode" { + var block: Block8x8 = undefined; + for (0..8) |i| { + for (0..8) |j| { + block[i][j] = @floatFromInt(@as(i32, @intCast(i * 8 + j)) - 32); + } + } + const f64_result = zigzagEncode(&block); + const i16_result = zigzagEncodeI16(&block); + for (0..64) |i| { + try std.testing.expectEqual(@as(i16, @intFromFloat(f64_result[i])), i16_result[i]); + } +} + test "zigzag round-trip" { var block: Block8x8 = undefined; for (0..8) |i| { @@ -112,6 +163,32 @@ test "run-length encoding" { try std.testing.expect(count < 64); } +test "runLengthEncodeI16 matches runLengthEncode" { + var f64_data: [64]f64 = undefined; + for (0..64) |i| { + f64_data[i] = @floatFromInt(@as(i32, @intCast(i)) - 32); + } + f64_data[5] = 0.0; + f64_data[6] = 0.0; + f64_data[7] = 0.0; + + var i16_data: [64]i16 = undefined; + for (0..64) |i| { + i16_data[i] = @intFromFloat(f64_data[i]); + } + + var out_f64: [64]RLEPair = undefined; + var out_i16: [64]RLEPair = undefined; + const count_f64 = runLengthEncode(&f64_data, &out_f64); + const count_i16 = runLengthEncodeI16(&i16_data, &out_i16); + + try std.testing.expectEqual(count_f64, count_i16); + for (0..count_f64) |i| { + try std.testing.expectEqual(out_f64[i].run, out_i16[i].run); + try std.testing.expectEqual(out_f64[i].value, out_i16[i].value); + } +} + test "all zeros RLE" { var data: [64]f64 = @splat(0.0); var output: [64]RLEPair = undefined; @@ -120,3 +197,12 @@ test "all zeros RLE" { try std.testing.expectEqual(@as(u4, 0), output[count - 1].run); try std.testing.expectEqual(@as(i16, 0), output[count - 1].value); } + +test "all zeros RLE i16" { + var data: [64]i16 = @splat(0); + var output: [64]RLEPair = undefined; + const count = runLengthEncodeI16(&data, &output); + try std.testing.expect(count > 0); + try std.testing.expectEqual(@as(u4, 0), output[count - 1].run); + try std.testing.expectEqual(@as(i16, 0), output[count - 1].value); +} diff --git a/src/encoder.zig b/src/encoder.zig index 60b195f..7c92eb9 100644 --- a/src/encoder.zig +++ b/src/encoder.zig @@ -50,6 +50,8 @@ pub fn encodeJPEG(allocator: std.mem.Allocator, image_data: *const types.ImageDa const lum_matrix = quantization.getQuantizationMatrix(quality, true); const chr_matrix = quantization.getQuantizationMatrix(quality, false); + const lum_inv = quantization.getInvQuantMatrixWithMatrix(&lum_matrix); + const chr_inv = quantization.getInvQuantMatrixWithMatrix(&chr_matrix); var prev_dc_y: i16 = 0; var prev_dc_cb: i16 = 0; @@ -131,16 +133,16 @@ pub fn encodeJPEG(allocator: std.mem.Allocator, image_data: *const types.ImageDa const idx = by * @as(usize, orig_mcu_w) + bx; if (idx < num_blocks) { const dct_y: Block8x8 = if (fast_mode) dct.fastDCT(&blocks_result.y_blocks[idx]) else dct.dct2D(&blocks_result.y_blocks[idx]); - const q_y = quantization.quantizeBlockWithMatrix(&dct_y, &lum_matrix); - const zz_y = zigzag_mod.zigzagEncode(&q_y); - prev_dc_y = try encodeBlock(&zz_y, prev_dc_y, true, &bit_writer); + const q_y = quantization.quantizeBlockFast(&dct_y, &lum_inv); + const zz_y = zigzag_mod.zigzagEncodeI16(&q_y); + prev_dc_y = try encodeBlockI16(&zz_y, prev_dc_y, true, &bit_writer); } else { - var empty_zz: [64]f64 = @splat(0); - prev_dc_y = try encodeBlock(&empty_zz, prev_dc_y, true, &bit_writer); + var empty_zz: [64]i16 = @splat(0); + prev_dc_y = try encodeBlockI16(&empty_zz, prev_dc_y, true, &bit_writer); } } else { - var empty_zz: [64]f64 = @splat(0); - prev_dc_y = try encodeBlock(&empty_zz, prev_dc_y, true, &bit_writer); + var empty_zz: [64]i16 = @splat(0); + prev_dc_y = try encodeBlockI16(&empty_zz, prev_dc_y, true, &bit_writer); } } } @@ -148,17 +150,17 @@ pub fn encodeJPEG(allocator: std.mem.Allocator, image_data: *const types.ImageDa // 1 Cb block { const dct_cb: Block8x8 = if (fast_mode) dct.fastDCT(&ds_cb[m]) else dct.dct2D(&ds_cb[m]); - const q_cb = quantization.quantizeBlockWithMatrix(&dct_cb, &chr_matrix); - const zz_cb = zigzag_mod.zigzagEncode(&q_cb); - prev_dc_cb = try encodeBlock(&zz_cb, prev_dc_cb, false, &bit_writer); + const q_cb = quantization.quantizeBlockFast(&dct_cb, &chr_inv); + const zz_cb = zigzag_mod.zigzagEncodeI16(&q_cb); + prev_dc_cb = try encodeBlockI16(&zz_cb, prev_dc_cb, false, &bit_writer); } // 1 Cr block { const dct_cr: Block8x8 = if (fast_mode) dct.fastDCT(&ds_cr[m]) else dct.dct2D(&ds_cr[m]); - const q_cr = quantization.quantizeBlockWithMatrix(&dct_cr, &chr_matrix); - const zz_cr = zigzag_mod.zigzagEncode(&q_cr); - prev_dc_cr = try encodeBlock(&zz_cr, prev_dc_cr, false, &bit_writer); + const q_cr = quantization.quantizeBlockFast(&dct_cr, &chr_inv); + const zz_cr = zigzag_mod.zigzagEncodeI16(&q_cr); + prev_dc_cr = try encodeBlockI16(&zz_cr, prev_dc_cr, false, &bit_writer); } if (m % 64 == 0) { @@ -175,17 +177,17 @@ pub fn encodeJPEG(allocator: std.mem.Allocator, image_data: *const types.ImageDa const dct_cb: Block8x8 = if (fast_mode) dct.fastDCT(&blocks_result.cb_blocks[block_idx]) else dct.dct2D(&blocks_result.cb_blocks[block_idx]); const dct_cr: Block8x8 = if (fast_mode) dct.fastDCT(&blocks_result.cr_blocks[block_idx]) else dct.dct2D(&blocks_result.cr_blocks[block_idx]); - const q_y = quantization.quantizeBlockWithMatrix(&dct_y, &lum_matrix); - const q_cb = quantization.quantizeBlockWithMatrix(&dct_cb, &chr_matrix); - const q_cr = quantization.quantizeBlockWithMatrix(&dct_cr, &chr_matrix); + const q_y = quantization.quantizeBlockFast(&dct_y, &lum_inv); + const q_cb = quantization.quantizeBlockFast(&dct_cb, &chr_inv); + const q_cr = quantization.quantizeBlockFast(&dct_cr, &chr_inv); - const zz_y = zigzag_mod.zigzagEncode(&q_y); - const zz_cb = zigzag_mod.zigzagEncode(&q_cb); - const zz_cr = zigzag_mod.zigzagEncode(&q_cr); + const zz_y = zigzag_mod.zigzagEncodeI16(&q_y); + const zz_cb = zigzag_mod.zigzagEncodeI16(&q_cb); + const zz_cr = zigzag_mod.zigzagEncodeI16(&q_cr); - prev_dc_y = try encodeBlock(&zz_y, prev_dc_y, true, &bit_writer); - prev_dc_cb = try encodeBlock(&zz_cb, prev_dc_cb, false, &bit_writer); - prev_dc_cr = try encodeBlock(&zz_cr, prev_dc_cr, false, &bit_writer); + prev_dc_y = try encodeBlockI16(&zz_y, prev_dc_y, true, &bit_writer); + prev_dc_cb = try encodeBlockI16(&zz_cb, prev_dc_cb, false, &bit_writer); + prev_dc_cr = try encodeBlockI16(&zz_cr, prev_dc_cr, false, &bit_writer); if (block_idx % 64 == 0) { if (options.on_progress) |cb| { @@ -230,14 +232,14 @@ pub fn encodeJPEG(allocator: std.mem.Allocator, image_data: *const types.ImageDa }; } -fn encodeBlock(zz: *const [64]f64, prev_dc: i16, is_luminance: bool, bw: *BitWriter) !i16 { - const dc_val: i16 = @intFromFloat(zz[0]); +fn encodeBlockI16(zz: *const [64]i16, prev_dc: i16, is_luminance: bool, bw: *BitWriter) !i16 { + const dc_val: i16 = zz[0]; const dc_diff: i16 = @intCast(std.math.clamp(@as(i32, dc_val) - @as(i32, prev_dc), -2047, 2047)); try huffman.encodeHuffmanDC(dc_diff, is_luminance, bw); var rle: [64]RLEPair = undefined; - const rle_count = zigzag_mod.runLengthEncode(zz, &rle); + const rle_count = zigzag_mod.runLengthEncodeI16(zz, &rle); for (0..rle_count) |i| { const val = rle[i].value; diff --git a/src/encoding/bitstream.zig b/src/encoding/bitstream.zig index 38ad526..51ab6e1 100644 --- a/src/encoding/bitstream.zig +++ b/src/encoding/bitstream.zig @@ -28,6 +28,26 @@ pub const BitWriter = struct { } } + pub fn writeBits2(self: *BitWriter, bits1: u32, count1: u8, bits2: u32, count2: u8) !void { + self.accumulator = (self.accumulator << @intCast(count1)) | bits1; + self.bit_count += count1; + + while (self.bit_count >= 8) { + self.bit_count -= 8; + const byte: u8 = @intCast((self.accumulator >> @intCast(self.bit_count)) & 0xFF); + try self.buffer.append(byte); + } + + self.accumulator = (self.accumulator << @intCast(count2)) | bits2; + self.bit_count += count2; + + while (self.bit_count >= 8) { + self.bit_count -= 8; + const byte: u8 = @intCast((self.accumulator >> @intCast(self.bit_count)) & 0xFF); + try self.buffer.append(byte); + } + } + pub fn flush(self: *BitWriter) !void { if (self.bit_count > 0) { const shift: u5 = @intCast(8 - self.bit_count); @@ -80,3 +100,19 @@ test "BitWriter multi-byte" { try std.testing.expectEqual(@as(u8, 0xFF), bw.buffer.items[0]); try std.testing.expectEqual(@as(u8, 0x00), bw.buffer.items[1]); } + +test "BitWriter writeBits2 matches two writeBits" { + var bw1 = BitWriter.init(std.testing.allocator); + defer bw1.deinit(); + var bw2 = BitWriter.init(std.testing.allocator); + defer bw2.deinit(); + + try bw1.writeBits(0b1010, 4); + try bw1.writeBits(0b1100, 4); + try bw2.writeBits2(0b1010, 4, 0b1100, 4); + + try std.testing.expectEqual(bw1.buffer.items.len, bw2.buffer.items.len); + for (bw1.buffer.items, 0..) |b, i| { + try std.testing.expectEqual(b, bw2.buffer.items[i]); + } +} diff --git a/src/encoding/huffman.zig b/src/encoding/huffman.zig index e2afd05..6600ed0 100644 --- a/src/encoding/huffman.zig +++ b/src/encoding/huffman.zig @@ -143,10 +143,11 @@ pub fn encodeHuffmanDC(value: i16, is_luminance: bool, bw: *BitWriter) !void { const category = getCategory(value); const table = if (is_luminance) &DC_LUM_CODES else &DC_CHR_CODES; const hc = table[category]; - try bw.writeBits(hc.code, hc.bits); if (category > 0) { const additional = getAdditionalBits(value, category); - try bw.writeBits(additional, category); + try bw.writeBits2(hc.code, hc.bits, additional, category); + } else { + try bw.writeBits(hc.code, hc.bits); } } @@ -169,10 +170,11 @@ pub fn encodeHuffmanAC(run_length: u4, value: i16, is_luminance: bool, bw: *BitW const run_and_size: u8 = (@as(u8, run_length) << 4) | category; const table = if (is_luminance) &AC_LUM_CODES else &AC_CHR_CODES; const hc = table[run_and_size]; - try bw.writeBits(hc.code, hc.bits); if (category > 0) { const additional = getAdditionalBits(value, category); - try bw.writeBits(additional, category); + try bw.writeBits2(hc.code, hc.bits, additional, category); + } else { + try bw.writeBits(hc.code, hc.bits); } } diff --git a/src/encoding/jpeg_writer.zig b/src/encoding/jpeg_writer.zig index fd96263..1ce39ac 100644 --- a/src/encoding/jpeg_writer.zig +++ b/src/encoding/jpeg_writer.zig @@ -59,17 +59,21 @@ pub const JpegWriter = struct { try self.writeWord(132); try self.buffer.append(0x00); - for (0..8) |i| { - for (0..8) |j| { - try self.buffer.append(@intCast(lum_matrix[i][j])); + inline for (0..8) |i| { + var row: [8]u8 = undefined; + inline for (0..8) |j| { + row[j] = @intCast(lum_matrix[i][j]); } + try self.buffer.appendSlice(&row); } try self.buffer.append(0x01); - for (0..8) |i| { - for (0..8) |j| { - try self.buffer.append(@intCast(chr_matrix[i][j])); + inline for (0..8) |i| { + var row: [8]u8 = undefined; + inline for (0..8) |j| { + row[j] = @intCast(chr_matrix[i][j]); } + try self.buffer.appendSlice(&row); } } @@ -144,10 +148,19 @@ pub const JpegWriter = struct { } pub fn writeCompressedData(self: *JpegWriter, data: []const u8) !void { - for (data) |byte| { - try self.buffer.append(byte); - if (byte == 0xFF) { + var pos: usize = 0; + while (pos < data.len) { + if (std.mem.indexOfScalar(u8, data[pos..], 0xFF)) |ff_offset| { + const chunk_end = pos + ff_offset; + if (chunk_end > pos) { + try self.buffer.appendSlice(data[pos..chunk_end]); + } + try self.buffer.append(0xFF); try self.buffer.append(0x00); + pos = chunk_end + 1; + } else { + try self.buffer.appendSlice(data[pos..]); + break; } } }