Parse a1lx and expose item extents for layered AVIF images - #456
jakearchibald wants to merge 2 commits into
Conversation
A layered (progressive) AVIF image item holds up to four AV1 frames concatenated in its payload, each a better rendering of the same picture. `a1lx` gives the byte sizes of the first three, and is the only signal saying where one layer ends and the next begins. It was previously recorded as an unsupported feature and skipped without reading its payload, so a caller had no way to find a layer boundary and could only decode the item whole. Parse it, and give callers what they need to slice the payload themselves: - `ItemProperty::LayeredImageIndexing` carries the layer sizes, and `Feature::A1lx` becomes supported. - `AvifItem` retains its `iloc` extents and construction method, reached through `primary_item_extents`, `alpha_item_extents` and the `*_is_file_construction` predicates. A caller decoding an item incrementally needs to know which of its bytes have arrived, which the existing item-data copy cannot tell it -- and for a multi-extent item that copy would have captured the whole payload anyway. - `primary_item_a1lx`, `alpha_item_a1lx` and `primary_item_lsel` report the properties that decide whether an item is progressively renderable. An alpha auxiliary item can be layered too, with its own `a1lx`. A malformed `a1lx` -- truncated, overlong, or with non-zero reserved bits -- is deliberately not fatal. It is recorded as present with no layer sizes, which reads as "not a layered image". Such a file decoded fine when the property was skipped, and failing it now would be a regression. The C API gains the matching `Mp4parseAvifInfo` fields, plus `Mp4parseItemExtents` as a borrowed slice of `ItemExtent`, valid for the lifetime of the parser. Because `a1lx` is no longer an unsupported feature, the files whose only unsupported property it was now parse with an empty `unsupported_features` set, so they come off `AVIF_UNSUPPORTED_IMAGES`. `animals_00_multilayer_grid_a1lx` and `quebec_3layer_op2` stay, since `grid` and `a1op` are still unsupported.
| pub struct ItemExtent { | ||
| pub offset: u64, | ||
| pub len: u64, | ||
| pub to_end: bool, |
There was a problem hiding this comment.
Why put the repr-C-friendly form of this in the pure-Rust part of things? The advantage to the non-repr-C-friendly Extent enum is that it makes the representation of illegal states impossible as well as the interpretation of the data clear from the code rather than comments. Is there an essential reason why the transformation can't be constrained to the part of the code concerned with FFI?
There was a problem hiding this comment.
Fair. I've moved it. Again, I tested it with the Firefox patch.
There was a problem hiding this comment.
I don't have time to review all of this closely enough to give a general approval, but with this revision, I don't see anything objectionable from a cursory perusal. Thanks for taking the time to add this cool feature.
There was a problem hiding this comment.
Hi Jon hope you're well! And thanks for giving it a look. Myself or @kinetiknz will look more closely.
There was a problem hiding this comment.
Hi Paul! I miss y'all. Feel free to ping me sometime; it would be lovely to catch up.
This is part of https://bugzilla.mozilla.org/show_bug.cgi?id=2072679 - supporting progressive AVIF rendering in Firefox. This change is needed to expose the progressive layers.
Forgive me, for this was largely vibe coded, although I've manually tested it integrated with Firefox as part of https://phabricator.services.mozilla.com/D326564.
Here's some slop explaining what's going on here:
A layered (progressive) AVIF image item holds up to four AV1 frames concatenated in its payload, each a better rendering of the same picture.
a1lxgives the byte sizes of the first three, and is the only signal saying where one layer ends and the next begins. It was previously recorded as an unsupported feature and skipped without reading its payload, so a caller had no way to find a layer boundary and could only decode the item whole.Parse it, and give callers what they need to slice the payload themselves:
ItemProperty::LayeredImageIndexingcarries the layer sizes, andFeature::A1lxbecomes supported.AvifItemretains itsilocextents and construction method, reached throughprimary_item_extents,alpha_item_extentsand the*_is_file_constructionpredicates. A caller decoding an item incrementally needs to know which of its bytes have arrived, which the existing item-data copy cannot tell it -- and for a multi-extent item that copy would have captured the whole payload anyway.primary_item_a1lx,alpha_item_a1lxandprimary_item_lselreport the properties that decide whether an item is progressively renderable. An alpha auxiliary item can be layered too, with its owna1lx.A malformed
a1lx-- truncated, overlong, or with non-zero reserved bits -- is deliberately not fatal. It is recorded as present with no layer sizes, which reads as "not a layered image". Such a file decoded fine when the property was skipped, and failing it now would be a regression.The C API gains the matching
Mp4parseAvifInfofields, plusMp4parseItemExtentsas a borrowed slice ofItemExtent, valid for the lifetime of the parser.Because
a1lxis no longer an unsupported feature, the files whose only unsupported property it was now parse with an emptyunsupported_featuresset, so they come offAVIF_UNSUPPORTED_IMAGES.animals_00_multilayer_grid_a1lxandquebec_3layer_op2stay, sincegridanda1opare still unsupported.