Repository navigation
webm: pictures cut into AV1 tiles - a tile nothing changes is coded once a film - #168
Conversation
…coded once a film A film's pictures differ only in the clock and the square, so from 256x144 up the picture is cut into AV1 tiles on superblock lines: a row under the clock's band, rows around the square's, a column for each superblock the clock's characters reach and one for the rest. Each tile is coded by gav1d as a picture of its own, read where it lies in the picture's planes, and joined under a frame header this package writes - tile_info with the tiles' sizes, the tile group with their lengths. A tile is looked up by what its pixels depend on (the clock's characters in it, the square's columns in it), so the tiles nothing changes are coded once a film, the square's ten places once each, and a change codes only the clock's tile. A thirty-minute 1920x1080 film of 2 GB: 67.9 s before, 2.8 and 3.1 s now, every one of its 54 000 frames decoded by libaom. Films of one tile keep their bytes - the three golden films are unchanged. - A new guard holds every tile in a film to gav1d's own coding of the pixels there, and the decoded frame to its tiles decoded alone, read back with a tile_info reader of its own. Four film guards now assert that they asked about a film of tiles. - The ladder's ceilings re-measured for the rungs that have tiles. - The work a change reports is the pixels of the tiles it codes. - change_interval no longer says a change costs a whole picture in time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…22 objects each The default film, 640x360 in six tiles, makes 27 calls to gav1d, about 594 objects before anything of this package. Measured at 886 to 891 objects at one, four and sixteen threads, and 1177 with one object allocated per frame, the defect the ceiling is for. The owner's decision. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (19)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (20)
🧰 Additional context used📚 Code guidelines (1)📓 Path-based instructions (13)Applies to text shown to the user (labels, buttons, tooltips, placeholders, dialogs, errors, status messages, empty states, translations).⚙️ CodeRabbit configuration file Files:
Verify tests check real behavior and would fail if the implementation were broken.⚙️ CodeRabbit configuration file Files:
These are end-user desktop applications.⚙️ CodeRabbit configuration file Files:
Performance is a known weak spot of these projects.⚙️ CodeRabbit configuration file Files:
Applies only to code that builds or styles a GUI.⚙️ CodeRabbit configuration file Files:
User-facing changelog.⚙️ CodeRabbit configuration file Files:
Domain: test file generator (Go; `tfg` CLI and `tfg-gui` Fyne window over one engine).⚙️ CodeRabbit configuration file Files:
SECURITY, HIGH PRIORITY.⚙️ CodeRabbit configuration file Files:
These apps are QA/developer tools.⚙️ CodeRabbit configuration file Files:
Go code.⚙️ CodeRabbit configuration file Files:
Check that documentation matches the actual code in this PR: commands, flags, config keys, file paths, build steps and examples must exist.⚙️ CodeRabbit configuration file Files:
All code in this repository is written by an AI coding agent (Claude Code).⚙️ CodeRabbit configuration file Files:
Source excerpt: **Words a user reads are English, with a flat hyphen and no semicolons.**📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
🪛 ast-grep (0.45.3)internal/format/video/bits.go[warning] 44-44: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values. (integer-overflow-narrowing-conversion-go) [warning] 47-47: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values. (integer-overflow-narrowing-conversion-go) [warning] 48-48: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values. (integer-overflow-narrowing-conversion-go) internal/format/video/grid.go[warning] 200-200: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values. (integer-overflow-narrowing-conversion-go) internal/guard/filmtiles_test.go[error] 389-389: Zip-Slip: joining the extraction directory with an attacker-controlled archive entry name (e.g. zip.File.Name / tar.Header.Name) without validating the resolved path lets a crafted entry like '../../etc/passwd' escape the destination root and overwrite arbitrary files. Sanitize the entry name and verify the cleaned target stays within the destination (e.g. reject names containing '..', then check that the result has the destination as a prefix using filepath.Clean + strings.HasPrefix or filepath.Rel). (zip-slip-filepath-join-archive-entry-go) internal/format/video/picture.go[warning] 343-343: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values. (integer-overflow-narrowing-conversion-go) 🔇 Additional comments (19)
📝 WalkthroughWalkthroughThe WebM video pipeline now encodes AV1 tiles, reuses coded tiles across picture changes, and assembles tiled frame samples. It adds tile-aware planning, work accounting, and parallel encoding, with tests that inspect tile data and decoded output. ChangesAV1 tile-based WebM encoding
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant WebM as WebM writer
participant Pictures
participant Crew
participant Encoder as encodeTile
participant Sampler as frameShape.sample
WebM->>Pictures: Request picture change
Pictures->>Crew: Offer tile jobs
Crew->>Encoder: Encode selected tile rectangle
Pictures->>Sampler: Assemble coded tiles into frame sample
Pictures-->>WebM: Return key and copy samples
Suggested labels: Merge Risk: ⚪ Minimal · up to WebM films with multiple tiles now encode AV1 tiles independently and reuse unchanged tiles, which greatly speeds up long films. Films with one tile keep their bytes. No concrete defect was found, and new guards check tile contents and decoded pixels, so the change appears ready to merge. 🚥 Pre-merge checks | ✅ 12 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (12 passed)
Full details: Safe File ParsingExplanation The new test parser can panic on malformed WebM/AV1 data. In Resolution Make Full details: Scope, Duplication And DocsExplanation The PR adds multi-tile frames, but the WebM size-limit explanation remains inaccurate. Resolution Update
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…the ejected preset follow Planning counts a frame's tile_info as this package now writes it, one bit for one tile at 1x1, rather than the three gav1d could write, so the bound on the smallest film at the default settings came ten bytes closer to the film: 3294 B to 3284 B. The film itself is the same - 3284 B is written and decodes in libaom frame for frame, 3283 B is refused with the new minimum. The ejected empty-and-minimal recipe asks for that size, so its bytes move again before any release carried the film, under the decision the row records. The site's format and preset pages and the preset screens' totals are regenerated from the program. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, not a panic A panic in a guard ends every guard of the package, not the one that read a broken film. The bit reader says it ran out instead, and the reader of tile_info and the tile group refuses a header that ends inside itself, a frame that ends before its tile group or inside a tile's size, and an empty frame OBU. filmOne is filmWithSeed without the seed rather than a copy of it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
What
A film's pictures differ only in the clock and the square. From 256x144 up the picture is now cut into AV1 tiles on superblock lines, each tile coded by gav1d as a picture of its own and joined under a frame header this package writes (
tile_infowith the tiles' sizes, the tile group with their lengths). A tile is looked up by what its pixels depend on - the clock's characters in it and the square's columns in it - so the tiles nothing changes are coded once a film, the square's ten places once each, and a change codes only the clock's tile.Measured
change_count).Changed on purpose
AllocCeilingfor webm 512 to 1024: one gav1d call a tile at 21-22 objects a call. Measured 886-891 objects at 1, 4 and 16 threads, 1177 with one object allocated per frame. The 4 MiB to 64 MiB growth check is untouched and green.change_intervalno longer says a change costs a whole picture in time (English catalogue and Polish translation).Guards
TestEveryTileOfAFilmIsGav1dsCodingOfItsPixelsAndDecodesAsItDoesAlone- every tile in a film is the tail of gav1d's own coding of those pixels, and the decoded frame equals each tile decoded alone. It readstile_infowith a reader of its own and needs no ffmpeg. Proven by three hand mutations (key without the clock, a wrong tile height, a wrong stride).Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit