jxl-frame: EC upsampling check #34
Conversation
crates/jxl-image/src/lib.rs
Outdated
| let dim_shift = read_bits!(bitstream, U32(0, 3, 4, 1 + u(3)))?; | ||
| if (1usize << dim_shift) > 8 { |
There was a problem hiding this comment.
As discussed in jxl discord chat, there's something wrong in the check in libjxl; this makes decoded U32 value of 4 (10 in bitstream) instantly invalid. Instead we could defer the check to the frame header decoding.
There was a problem hiding this comment.
Agreed, it seems to be an early check for dim_shift when ec_upsampling is not yet known.
EugeneVert
changed the title
dim_shift check
crates/jxl-frame/src/lib.rs
Outdated
| "EC upsampling < color upsampling, which is invalid").into() | ||
| ); | ||
| } | ||
| if ec_up_cumulative > 8 { |
There was a problem hiding this comment.
As far as I got it right, it's the dim_shift that makes the actual encoded block dimension smaller (to group_dim >> dim_shift), so only dim_shift should be checked here. The spec says that 1 << dim_shift does not exceed group_dim, so it makes sense; otherwise the actual dimension would be smaller than 1.
The required assertion is that (1 << dim_shift) <= group_dim, where group_dim = 1 << (7 + group_size_shift), so we need to check whether ec.dim_shift <= 7 + frame_header.group_size_shift holds.
EugeneVert
force-pushed
the
extra-channel-info-check
branch
2 times, most recently
from
bc66859 to
db8e682
Compare
EugeneVert
force-pushed
the
extra-channel-info-check
branch
from
db8e682 to
9baeb00
Compare
|
I should probably separate the dim_shift and ec_upsampling checks |
I feel like it's ok as is, but feel free to do so if you want to! |
EugeneVert
force-pushed
the
extra-channel-info-check
branch
from
9baeb00 to
8a44c7b
Compare
No description provided.