Repository navigation
webm: pictures up to 8192x4352 - AV1's levels are the bound, and an 8K film peaks at about 240 MB - #169
Merged
Conversation
…one tile webm_tiles is 640x360 for three seconds: six tiles, three pictures, so how a picture is cut and which tile is coded again are pinned before the rule that cuts larger pictures changes. Measured on the code before that change, and the same hash from the command line with --id g --seed 7741. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… one tile gav1d codes one tile correctly and no more, and that held a film's picture to 4096x2304. Since pictures are cut into tiles here, every tile is within one of gav1d's, so the bound left is the format's: the largest picture an AV1 level describes, 8192 by 4352, with either side up to 16384. The cut rule keeps today's cuts and adds two, only where a frame cannot carry the layout: a column wider than MAX_TILE_WIDTH and a row taller than the area tile_info leaves a tile once the frame needs several are cut into the fewest equal parts, the larger last, and a frame more than one superblock tall gets columns narrow enough for rows of two superblocks - without that, a wide low picture's last row of tiles was a pixel tall, which Annex A refuses and every decoder took. A picture of 4096x2304 or less keeps the layout and the bytes it had: the golden films, and twelve sizes made by main and by this. Measured on 8K, 8192x4352, 16384x2176, 4097x65, 4096x2305 and 2048x16384: libaom and dav1d decode every frame and agree, dav1d 1.4.1 too, Chromium plays and seeks. New guard: TestEveryFilmLayoutIsOneTheAV1SpecificationAllows holds every layout up to the declared bound to tile_info and Annex A, derived from the specification rather than the writer. A golden film wider than one tile, and wide cases in the tile and libaom guards. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…B, it took 1.3 GB A film kept its starting picture whole in pixels and in planes, and every painter - one for each helper coding beside the writer - copied the planes whole, so a 7680x4320 film held 133 MB of pixels its painters read a sixth of and fifteen copies of 50 MB. Now the film makes its planes sixty four rows at a time and keeps pixels only along the rows a picture can change, a painter has room for the largest tile that can change, and a tile no picture changes is coded straight from the film's planes, which every goroutine reads and none writes. Measured before and after, interleaved, three rounds each: one 7680x4320 film 1276-1284 MB at the peak, now 187-241 MB. Sixteen of them at once 3733-3914 MB, now 1068-1106 MB. 3840x2160 356-358 MB, now 144 MB. The time is the same, and so are the bytes - the golden films, and nine large films made both ways. video.Picture now puts a picture together tile by tile from where the film takes each tile, so the guard that holds it to the picture painted whole holds the tiles' own road. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (22)
📒 Files selected for processing (17)
💤 Files with no reviewable changes (2)
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/picture.go[warning] 217-217: 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] 217-217: 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] 217-217: 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] 474-474: 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 (15)
📝 WalkthroughWalkthroughWebM picture bounds now allow dimensions up to 16,384 pixels per side and a maximum area of 8,192×4,352 pixels. Tile layouts are fitted to frame constraints, and film tiles are rendered and encoded from tile-local sources. ChangesWebM Multi-Tile Picture Support
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant CodedSize
participant gridFor
participant painter
participant encodeTile
CodedSize->>gridFor: geometry and timeline FPS
CodedSize->>painter: keyer with selected grid
loop Each tile
CodedSize->>painter: source for tile rectangle
painter-->>CodedSize: tile planes and position
CodedSize->>encodeTile: source and position
end
Suggested labels: Merge Risk: ⚪ Minimal · up to WebM output now supports pictures up to 8192x4352 using multiple AV1 tiles and uses less memory. Pictures within the old bound keep their bytes. No concrete defect was found, and the change appears ready to merge once CI passes. 🚥 Pre-merge checks | ✅ 14✅ Passed checks (14 passed)
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 |
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
1 participant
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.
What
A film's picture was held to 4096x2304 because gav1d codes one AV1 tile correctly and no more. Since #168 the picture is cut into tiles here and every tile is within one of gav1d's, so the bound left is the format's: the largest picture an AV1 level describes, 8192x4352 (35 651 584 pixels), with either side up to 16384. 7680x4320 and 8192x4320 films can be made. A larger picture is refused as a setting before anything is written.
The cut rule keeps every cut it had and adds two, only where a frame cannot carry the layout:
tile_infoleaves a tile once the frame needs several, are cut into the fewest equal parts, the larger last;The film's memory went with it. A film kept its starting picture whole as pixels and as planes, and every painter, one for each helper, copied the planes whole. Now the film makes its planes sixty four rows at a time, keeps pixels only along the rows a picture can change, each painter holds the largest tile that can change, and a tile nothing changes is coded straight from the film's planes.
Measured
mainand by this branch, byte for byte.The time is the same, about 2 s for an 8K film of ten seconds. Nine large films made before and after the painter change are identical.
Changed on purpose
tfg formats webmgives for them (English catalogue and Polish translation).1 - 16384, on 22 pages.Guards
TestEveryFilmLayoutIsOneTheAV1SpecificationAllowschecks 451 177 layouts up to the declared bound againsttile_infoand Annex A in 0.4 s. The conditions are derived in the guard from the specification, not from the writer. It asserts that it reached the wide, area-cut and narrowed layouts. Proven by three hand mutations, one of them removing the narrowing (2156 breaks).webm_tiles(640x360, six tiles) andwebm_wide(4240x1000, uneven column and row cuts).video.Picturenow puts a picture together tile by tile, the way the film does, so the guard that holds it to the picture painted whole covers the tiles' own road.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit