JPEG XL

no strong opinions 🙂

I suppose there could be some use cases like thermal or depth channels where maybe a downsampling factor of 16 or 32 is useful, and it's a bit weird that we signal dim_shift as `U32(0, 3, 4, 1 + u(3))` if the value 4 is not valid. So I'm tending towards fixing libjxl here and keeping the spec as it is. We could limit it to 64x cumulative upsampling though, so at most 2 upsampling passes are needed.

looking at the libjxl code, I'm not sure if it is doing something sensible in e.g. the case where main channels are 2x subsampled and alpha is 4x subsampled

isn't it then going to first upsample alpha 4x, then do patches (which will break or do something weird since the main channels are still subsampled), then render splines, and then upsample the main channels 2x?

Doesn't it make more sense to make it upsample everything before doing patches and splines, always? What's the advantage of doing upsampling later?

Everything is possible, but if the changes are small enough and we can say they're only editorial, we can skip FDIS stage. We don't want to make any actual change in this second edition, just want to get rid of unclear things and discrepancies between libjxl and the spec.

I believe there was a reason for it, but I don't remember what was the reason

I'd have assumed to lower memory usage and compute time by running patches on a smaller area and just upsampling the results, although that'd be much more of an issue on encode rather than decode

Here's my current version of the draft

well if there's a reason then we should keep it as it is, but it would be good to document the reason somewhere (a note in the spec and/or a comment in the code)

I'll try to remember at some point 🙂

Something that I was quite confused by was the transposing of DCT-method varblocks

since 2D-DCT is just a 1-D DCT then a transpose then a 1-D DCT then another transpose, I'm guessing blocks that were taller than wide have their HF coeffs transposed for that reason

however, when implementing the spec, you have weights transposed as well, and you have to keep varblocks isolated

if you try to lay the HF coefficients in a 256x256 array you run into "when is this transposed?" problems a lot

I think the whole process could be a bit clearer, is all, but I don't think anything is incorrect per se

Regarding the max cumulative upsampling factor for extra channels: current spec has no limit (only the syntactic limit of 8 << 8 = 2048), current libjxl imposes a limit of 8, I would propose to set the limit to 64 for both. I'll make a ballot comment about it and we can discuss how to respond to it during the coming JPEG meeting.

it's also not clear how the tranposing of varblocks interacts with CfL and the like

at least not to me

And I agree the spec can be clearer about coeff layout, it's also mixing Rows x Columns and Width x Height notation

yeah that transpose is quite confusing, especially with square blocks

I just went transposing while coeff decoding and double-transpose while doing DCT

it's probably not really wrong but it's confusing — I don't really dare to touch that stuff too much myself though, I likely introduce more errors than I fix...

Some thoughts: - libjxl currently always upsamples all extra channels. Do we want to keep it like that? Extra channels for depth or thermal info could be only 1:8 or 1:16, so we could also consider only upsampling channels if it is actually needed to do frame/patch blending. We would then need to adjust the API to deal with variable sized channels though... - wouldn't it make the most sense to just do all upsampling before patches? I guess it's too late for that now though... - should we just disallow patches when main channels are subsampled and extra channels are subsampled more than the main channels? Doing that properly would mean extra channels need to be upsampled two times, which also leads to counterintuitive behavior since doing 2x two times gives different results than doing 4x once...

I think we do disallow it

Currently it looks like libjxl doesn't allow a cumulative upsampling of more than 8 (so it also doesn't allow an extra channel dim_shift of 4 even though this value is signaled as `U32(0, 3, 4, 1 + u(3))`)

libjxl API-wise, how do we want to deal with dim_shift and ec_upsampling? Since dim_shift is part of the image metadata and set by the application when encoding, I think it makes sense that if dim_shift > 0, the actual extra channel sample data that you pass to the encoder is already subsampled. While if ec_upsampling > 1, then you still pass data at "full" resolution and the encoder will do the subsampling for you (unless already_subsampled is true). So conceptually, while for a decoder there is no difference (it just needs to upsample by the cumulative factor), the difference is that dim_shift indicates an intrinsic property of the channel data (it just is at a lower resolution) while ec_upsampling is "just a coding tool". Does that make sense?

So what I propose is that we limit the cumulative upsampling factor to 64, but whether you want to get there by having a `dim_shift=6, ec_upsampling=1` or by having `dim_shift=3, ec_upsampling=8` is up to you.

In any case ec_upsampling is at most 8 (implied by header syntax); for dim_shift the header syntax limit is 8 (so factor 256) but I think we should put a stricter limit in the spec.

For patches and frame blending, since extra channels can be used as alpha channel for some of the blend modes, we do need to bring them to the same dimensions as the color channels — which is annoying if we also want to have the possibility to embed e.g. a 1:16 depth map (`dim_shift=4`) in an image and have a way to get back this data without upsampling. Perhaps we should just disallow patches whenever anything is not 1:1, to keep things simple? Then the order of operations also doesn't matter, and we can add a decode API option to fetch extra channels at their "native" resolution (only ec_upsampling is done but not dim_shift upsampling) instead of upsampled to full resolution.

Opinions please, since I think action is required on this topic. Libjxl is not implementing the full possibilities the current spec allows, and the current spec probably allows too much which makes things too complicated and unclear. I think we need to both make the spec more restrictive and add some missing stuff to libjxl, so they meet somewhere in the middle and match.

Current drafts of all the second editions:

had some issues with the references, see https://github.com/relaton/relaton/issues/118

references fixed:

note: this is not yet the version we will submit as FDIS for parts 1 and 2, it's still a draft of that. Next week is the 101st JPEG meeting where we will discuss this draft, possibly change some more things, and then hopefully approve it

(so if anyone still spots a typo or something else somewhere, please report it, we have a few more days to get it fixed)

Current state of the drafts after discussions. Still needs final approving but likely this will be what we will submit to ISO (well, Word versions of this).

May I ask if "IANA JPEG XL Media Type registration" described in 18181-2 Annex B has been officially sumbitted yet, or is still waiting to be submitted? Just want to know if It can be written that the official registration has been started, but just not yet formally published.

https://discord.com/channels/794206087879852103/794206170445119489/845021868712263690

yep, but from <https://www.rfc-editor.org/rfc/rfc6838.html#section-5.2.1> "Provisional registration": "The only required fields in such registrations are the media type name and contact information (including the standards-related organization name).", so I'm interested if full formal registration submission, with all the fields reached IANA

F.2 says: "If `save_before_ct` is `false`, then it is not that case that both `metadata.xyb_encoded == true` and `metadata.colour_encoding.want_icc == true`." However `save_before_ct` is always `false` if `is_last` is `true` (one can prove this with conditions and default values)

I think the default value of `save_before_ct` should be `is_last or !normal_frame`

nope, this breaks blending for enum color space. I'm just skipping color transform for now if `want_icc == true`

regarding conformance test: `alpha_premultiplied` specifies peak error of 3.815e-6 (2^-18), which seems to be for 16-bit Modular extra channel. But color channels are VarDCT encoded, so it's quite tough bound for those. Is this intended?

The spec says that the distribution alphabet size for ANS distributions is read as `U8() + 3`

which should max out at 257

but what happens if this value is greater than `1 << log_alphabet_size` where `log_alphabet_size` is what was read from the distribution?

libjxl rejects on line 234 of dec_ans.cc but it's not specified

Probably we should add that to the spec then...

Table F.7: condition of `clamp` is `extra and (mode == kBlend or mode == kMulAdd or mode == kMul)` in the spec, but it's `(extra and (mode == kBlend or mode == kMulAdd)) or mode == kMul` in libjxl

is this blendmodes section for patches or for frames?

With regards to this issue (https://github.com/image-rs/image/issues/1979) what is the copyright status of the second edition drafts?

The status is: don't distribute or ISO will hunt you down and kill you. I think.

Oof.

There is some kind of general understanding that within JCT1, it is ok to publish drafts up to CD stage. But what exactly it means is a bit of a gray zone. We did cause trouble when we uploaded the CD of the first edition to arxiv. I think ISO prefers to follow some procedure where _they_ put the CD text online and then when the DIS is registered they remove it again, or something.

I don't consider discord "publishing" since arguably this is a private chat, not a public webpage that gets indexed by search engines and where the whole world can easily find it. But it's definitely gray area stuff, where I am trying to walk a fine line between not making ISO angry and getting community feedback so we can have a good spec. It is annoying that it works like that.

(already discussed, but writing it here) - Clarify when saving as reference occurs. i.e. SaveBeforeCT = true still means after patches, but before or after splines/noise? etc.

if i read through all the spec and wrote my own document with instructions on how to make an encoder and decoder, would that count as breaking the copyright law?

it's okay in my understanding. it's something like, a very detailed explanation of jxl, which is so detailed that it can be used to implement a decoder

are you saying said detailed explanation would be allowed or not?

sorry im just a bit confused

IANAL moment, nor have I read into it, but I think it should be fine?

it should be fine

It's fine. You cannot copy phrases, but you can rewrite it.

ISO’s copyright only applies to their documents, not to how JPEG XL works

You can even make it more clear than the original 😊

Is your project hosted somewhere?

Lmao haven’t started yet I’m just thinking

I’m just curious in how the format works and how it’s so efficient

ok 🙂 it's a nice thought. I started with a similar idea of doing a very clear and intelligible and over documented and unoptimized implementation in python which would be a image of the spec... but i lack time...

I’m think of the same but c++

Same with time as well

Also I’m really only interested in decoding, mainly as encoding sounds more difficult

How do I host a git remote repo on the internet?

I’ve only ever set up git local repos

do you have a github, gitlab, bitbucket account ? or something similar ?

I think I have a GitHub

https://github.com/TheAznCoderPro

Does that let me git commit -m “idk I did something and it broke”

_TheAznCoderPro doesn't have any public repositories yet._ first time huh ? 😄

I only use local repos

on github you click at : create repository, then you follow the instructions which will look like this :

```echo "# tutu" >> README.md git init git add README.md git commit -m "first commit" git remote add origin https://github.com/yota-code/tutu.git git push -u origin master```

if you already have a local repository, only the last two commands are relevant

and the first of those two only needs to be run once

So add the remote and push to it?

yeah

Does it only push commits after I run push?

yes

`git push -u origin master` pushes master to origin and also sets the “upstream” branch of master to origin/master

so you can later just run `git push` and it will automatically push to origin/master

that’s the `-u` (`--set-upstream`) in that command

GitHub says to name the branch main for some reason

it’s the new convention

but the name is ultimately up to you

you can even have a different name in your local repository and on github

`git push -u origin local-name:github-name`

Will it only push the master branch and not all other branches?

with that command, yes

with a bare `git push`, it depends on the git option `push.default`, but the default since git 2.0 is: > • simple - push the current branch with the same name on the remote.

So -u just flags a branch as the master?

I have mine set to: > • upstream - push the current branch back to the branch whose changes are usually integrated into the current branch (which is called @{upstream}). This mode only makes sense if you are pushing to the same repository you would normally pull from (i.e. central workflow).

you can start to work remotely with a single branch and when you start to be familiar with a remote repo, look for some tuto online to continue your journey

not quite, it sets the “upstream” of your branch to the corresponding branch on the remote

`-u` also makes sense with other branches than main or master

And if a merge a pull request on GitHub how do I get that to update in my local git

with `git pull` (optionally with `-r`/`--rebase` which is my preference)

(without `-r`, `git pull` will default to a merge)

setting a branch as the master is done through the github repository settings

git push and git pull are the two command to sync with the remote repo, push you upload your commit, pull you download the commits you missed

lmao

how do i get an access token?

nvm found it

yay i did it

https://github.com/TheAznCoderPro/jpegxldec-cpp/tree/master

congrats

now im trying to remember how to use Makefiles

😅

that's clearly impossible, I re-read docs every time 😛

I love makefiles ! so clean so lean 😄

is my makefile any good?

maybe i should make a new thread?

don’t forget to give it a licence

https://external-content.duckduckgo.com/iu/?u=https%3A%2F%2Ftse3.mm.bing.net%2Fth%3Fid%3DOIP.kAcxAr902QrHvxi1M_96fgHaEj%26pid%3DApi&f=1&ipt=800675c4fde7c8dd1f3f78c98a23ea96fb5bc82b498f232767c5d1ecdd06f262&ipo=images

I’d personally tend to favour tup over make, but I guess that’s subjective to some extent ``` CPP = clang++ -std=c++2a : foreach src/*.cc |> $(CPP) -c -o %o %f |> obj/%B.o {objfiles} : {objfiles} |> $(CPP) -o %o %f |> jpegxldec.exe ```

tup ?

i created a new thread so this one doesn't get too crowded

https://discord.com/channels/794206087879852103/1208362658705838090

H.3: > where the `dist_multiplier` from C.3.3 is set to the largest channel width amongst all channels that are to be decoded, excluding the meta-channels. but libjxl doesn't skip meta channels <https://github.com/libjxl/libjxl/blob/6d041617c258d0dfbbc7676d2c96d84676d8cd6a/lib/jxl/modular/encoding/encoding.cc#L562-L575>

is it meant to be "including" instead of "excluding"?

does it not? https://github.com/libjxl/libjxl/blob/6d041617c258d0dfbbc7676d2c96d84676d8cd6a/lib/jxl/modular/encoding/encoding.cc#L567C1-L571C1

it's channel index *larger than* number of meta channels

it actually includes palette channels, in <https://github.com/tirr-c/jxl-oxide/issues/268>

... right

I guess it should have skipped meta channels to make lz77 special distances effective

but I don't think it's really possible to "fix" libjxl now

yeah that kinda sucks

...or use an extension flag here

well it won't be backwards compatible

We need to figure out how bad it is if we fix libjxl. Because the spec does make more sense than libjxl here.

how long has this been wrong in libjxl?

Forever

Question is how many bitstreams will be affected by fixing it

It only affects lossless, that's one thing. So the harm is limited, you can always decode with an older version and re-encode with a newer to "fix" bitstreams.

It also likely only affects files compressed with non-default settings.

at least it doesn't affect things if you don't do palette *and* lz77

Real lz77 is only done when doing -e 9 -I 0, right?

If you're recompressing a PNG or GIF with 256 colors, then the palette channel has a width of 256 which is also kGroupDim so it also doesn't make a difference in that case.

I think fixing libjxl is the way to go here. Maybe in a way that detects the case where it makes a difference, and if it then fails to decode, it could write a useful message saying something like "file was written by old version with symmetric encoder/decoder bug, try decoding this with 0.10.x".

Or just fix it and put the info in the Changelog

It can affect e10 (now e11) encodes, as well as e9I0 ones (maybe e8I0 too?). I don't think default effort or faster can be affected.

yeah I'm in favor of this getting fixed on libjxl's side if the damage is small enough and it seems like with this being exclusive to lossless and only affecting certain effort levels along with it only being discovered just now it doesn't seem like many bitstreams that are affected although it would probably be good to try and get a decent approximation of how common affected bitstreams are.

How intrusive is would the fix be? if it's not super intrusive would it be possible to make libjxl decode both? it could be nice to have libjxl through an issue, and perhaps even be able to "fix" the image using `cjxl old.jxl new.jxl blah blah`

in the bug report, they say it sometimes happen with -e8 and -e9

not sure whether it's with default settings

I mean, if I had to choose, I'd rather add an extension to change the behaviour - the new bitstreams would not be decodable with old libjxl, but at least the old ones would keep working

(those images are encoded with v0.10.1)

we could even have a "grace period" in which the encoder doesn't use the new behaviour while we wait for ppl to update

doesn't *need* to be an extension flag either, we could use the invalid transform ID to signal an additional bundle of some sort if we're making a non-bw-compatible change anyway

would it be better opening a tracking issue in libjxl repo?

<@179701849576833024> can we make it so that the libjxl decoder: - follows spec (so bug is fixed in libjxl) - if it sees a case that now doesn't decode, due to dist_multiplier being computed differently, it retries the group with the old/buggy behavior, so it still decodes (just slower) and the libjxl encoder: - just stops using dist_multiplier (special lz77 distances) for now, so it doesn't produce anything that will not decode with old libjxl. We can re-enable it later when it is safe to assume most people have updated.

I prefer not treating it as a spec bug / requiring new extensions to get the originally intended behavior, but just as a libjxl bug. Breakage should be relatively small, and with the above it could be mitigated.

mhhh

let me think about it

one potential issue is detecting the "doesn't decode" case

opened an issue with reproduction step https://github.com/libjxl/libjxl/issues/3356

ANS signature checking failing of course would do that, but theoretically it could also be a problem with Huffman coded images

That seems like a rather theoretical-only problem, no? The problem cases will be e8+ which should be ANS, no?

Do we check for group stream size overrun/underrun?

over, yes

under, no

it's not ANS under a few (data-dependant) conditions ... but I suspect those are quite unlikely to hit the dist_multiplier path

If we can mitigate it for the ANS case (and maybe the Huffman case when it causes overrun), then we probably have 99.9% of the cases.

Fixing it should help for compression in the case where lz77 is used in combination with palettes of more than kGroupDim colors, where we would now be using a bad dist_multiplier so the WebP-style special distances for doing "2D lz77" are basically broken.

Though for now let's just not use special distances at all in the encoder

(except maybe with an option behind the expert flag)

Was `-e 9 -d 0 <in> <out>`, no other arguments. I ran it on another directory of ~3500 high resolution PNGs and never hit it. I have only ever hit it more than once on JPEGs that were scaled and compressed losslessly just for the purpose of trying to trigger it again. No real user would ever do that as you would use the JPEG recompression.

That said I hit this way to easily to consider it rare, or maybe I was just unlucky. It reminds me of the zstandard bug that caused data corruption on higher compression levels. I wonder if higher levels in anything are undertested because benchmarks say the diminishing returns are not worth it?

I did hit it on a png currently testing my 280k images, all with `-d 0 -e 9` (and some other options)

there should probably be a quick v0.10.2 release to disable using dist_multiplier so no more invalid bitstreams are created

If this turns out to be libjxl's issue, there must be some easy way to check for the non compliant files, either in jxlinfo or djxl.

In this case, both encoder and decoder have the bug so it cancels out when using libjxl for both. It took an independent decoder implementation to find this bug.

Sure, but there are potentially files that can't be decoded by independent decoders. It should be possible to identify them so they can be reencoded.

for now you can try to decode with jxl-oxide

Are there Windows binaries available?

prebuilt binaries of v0.7.1 is available here: <https://github.com/tirr-c/jxl-oxide/actions/runs/8083760696>

pc-windows-msvc is the Windows one

Thank you

Anyway, a possible solution might be djxl printing out a warning while decoding a non compliant file, may be hidden behind -v flag.

726 affected / 279597 images a lot were jpg as well

Recompressed jpegs? At e8+ then?

all `-d 0 -e 9 -E 3`

Between 2022-02-11 and 2024-02-01

Manga source. It seems they were mostly grouped together by source, like Vinland saga had around 300 of those. Some one offs

But I think the affected ones were from pngs

Ah yes, losslesslly compressed, not transcoded

I will go through my collection of artwork to see if something shows up.

Turns out my old gradient test is effected

Should this be a big problem? Should I decode my images before updating?

it shouldn't be, whatever we do we will do our best to stick with stuff that is bw compatible, one way or the other

So the edge case is now kinda just part of the spec?

We haven't decided yet

from my understanding that's undecided as of right now however I hope this doesn't become apart of the spec

So this only affects palettes images at -e 9?

My opinion is to not change the spec, fix libjxl but do it in a way that still decodes what turned out to be incorrectly encoded images.

It affects low-color images at e8 or e9

not a 1 to 1 comparison but this reminds of a WebP encoding feature that got flipped off until programs and devices that used an older decoder went out of use https://github.com/webmproject/libwebp/commit/34c807491586d047586ef3b14ebaf945c188d1f0

Good thing I only use -e 7 so I shouldn’t be affected?

In particular: if there are groups with more than 256 distinct colors but less than 1024 colors, then the lz77 distance multiplier might be computed incorrectly.

Are the images still properly decodable though?

Correct. At e7 the encoder doesn't produce such files

Yes, they are now, that's why the bug went undetected until we had an independent decoder

Libjxl encoder and decoder do it wrong in the same way so it still works, just doesn't follow the spec

jxl-oxide was updated to decode both spec conformant streams and streams affected by this bug. Personally I hope you decide to fix libjxl, as it is possible other encoders will implement the current spec even if it gets amended later. This may never go away otherwise. It is also cleaner.

Id say fix libjxl and maybe add a note somewhere in the spec as old bw compatible behaviour

But idk how this works

what's proposed here https://discord.com/channels/794206087879852103/1021189485960114198/1213787865654231100 is the way to go I think

I guess as it’s still a new format and most people are using the reference decoder it should be fine

It’s just independent decoders might not know about the bug and might cause problems

Maybe libjxl should also print something during decode so the user knows so the can reencode if they want

Does Apple use libjxl? I heard a rumour Adobe wrote their own.

Like the “known wrong iccp thing”

Adobe uses libjxl, not e8/e9 though so there should be no issue there

Apple uses libjxl but I guess mostly decoder only

Yeah I don’t think apple has any encode functionality

In any case we should make the encoder stop producing things that are not spec compliant

Definitely

I’m guessing that’s gonna be in the next revision

could be done in a quick v0.10.2 release just to disable creating invalid bitstreams first?

Good idea

Also does anyone know which effort affiñity photo uses?

Lol Spanish keyboard

The most likely use case to be hit are people mass-upgrading PNGs.

When I mass upgraded PNG của em , I just used e7d0

The Paint.NET plugin exposes effort directly, and lets you use level 8 or 9.

Although idk why but I always just used png export and then converted to jxl using cjxl

Many Plugins have not updated to 0.10.x yet, so a quick revision will catch people before they do this.

Some did update at 0.10.0 though. If github don't already have such a functionality, to send mails to people which follows the project, they could be invited to register to a mailing list which could warn for important issues like this one. Unless it already exists 😅

interestingly, jxlatte decodes this to an all-white image without errors

I wonder what the spec says it should decode to (not what libjxl produces)

It's a 48bit file so that might be effecting it too

shouldn't matter, jxlatte always decodes to floats and then sends that to a PNG encoder

I am now starting to tend towards the opinion that we should update the spec, not change libjxl. 1) the compression benefit of doing it the spec way seems to be quite small, even if you only look specifically at images that are affected (for most it doesn't make any difference at all). 2) I made a libjxl patch to try the spec way and if that fails, try the libjxl way (similar to what jxl-oxide does). However, I found cases where the same bitstream decodes without error both ways, but with a different pixel output. That means there is no way to change it without breaking existing files in a nasty way (they will decode but incorrectly, which is arguably even worse than not decoding).

Another way of looking at it is that there's far fewer encoders/decoders than there are encoded images. So much less work to fix the decoders than to try and sort through the files or re-encode them all

what is the actual difference in this circumstance?

in behavior

this is about the biggest difference I found by trying random images from the qoi testset: ``` 432707 Battle_for_Mandicor_0.0.5.png 197284 Battle_for_Mandicor_0.0.5.png.e8.bug.jxl 196985 Battle_for_Mandicor_0.0.5.png.e8.fixed.jxl 188207 Battle_for_Mandicor_0.0.5.png.e9.bug.jxl 187598 Battle_for_Mandicor_0.0.5.png.e9.fixed.jxl ```

For the vast majority, there's no difference at all, or if there is, it's like 5-50 bytes, and sometimes in the other direction too...

I think they meant the different pixel output

The question is more: what will be more detrimental to the format. A bug almost no one will ever notice, but if noticed will piss off many. Or a spec which may incorporate bugs of the reference implementation 😅

I trust them to make the best choice. I just played with the format and didn't deleted the originals yet 😁

Ah. In the image I saw, it was not very noticeable (max abs error of 5/255 in the blue channel only), but in principle the difference could be arbitrarily large. Basically if a group ends in an lz77 match, it can have two completely different distances. And a group can end in a match with a significant length, so it can affect many pixels in principle. Though typically only the bottom of a group.

Such as my gradient turning into a valid pure white image

Turning libjxl behavior into spec is the best solution imo. Then basically it's not an issue in practice. We did intend something else, but it doesn't make a huge difference either way, and it's also not like it will make the spec complicated or inelegant.

It was my worry, that it would become complicated

I also found few files that jxl-oxide v0.7.1 couldn't decode. One of them decoded without error but with wrong colors.

That said, having files decoded incorrectly without triggering an error is worrying.

Part of me wondered if there's a way to 'fix' files without completely re-encoding, but at that point it's more effort than just changing spec

Are there any other popular encoders other than LibJXL?

there are other encoders but they are not popular

this is the only other lossless encoder I know of besides libjxl: https://github.com/etemesi254/zune-image/tree/dev/crates/zune-jpegxl

huh

I wonder if it encodes images such that they hit this issue...

that looks like a 1:1 port of fjxl xD so probably not

so it's undetectable if there is only one LZ77 symbol at the end... yeah, fixing the spec would be better, especially there's little benefit fixing libjxl

oh dear

What's the actual bug

I mean like, what does the spec say that is wrong

and what should it say instead

https://discord.com/channels/794206087879852103/1021189485960114198/1213749793176682536 ?

dist_multiplier including meta channel width

technically it's libjxl that is wrong not the spec however it might just be easier to change the spec rather then fix libjxl's behavior since there are encoded JXLs that are affected by this

excluding meta channels (as written in the spec) makes more sense, but there are broken images in the wild and there are cases that it's impossible to say whether it's broken or not

like, two different methods can end up with exact same bitstream and entropy-coded stream state but completely different image

Someone should make a PoC of how bad it could be

In a maliciously crafted bitstream, the difference could be arbitrarily large. I think we don't really have a choice but to turn libjxl into spec. Especially because the compression benefit of doing it the spec way is very small.

it's really fortunate that this is a relatively isolated case, as it only alters frames where there are some meta channels wider than color channels, and that can only happen when meta channels have been introduced in the first place (i.e. at least one palette transform).

Coincidentally the images from my collection that are triggering the palette issue also compress better using VP8L so I will keep these in lossless webp for time being.

How much better?

maybe around this amount? https://discord.com/channels/794206087879852103/1021189485960114198/1214646134044098661 although having the files to mess with or at the least results for https://discord.com/channels/794206087879852103/1021189485960114198/1217899891435503636 would be nice

what do the palette transforms do?

how do they create meta channels?

say that all input channels are W by H pixels, then the palette transform will give a single output channel of W by H, and also a metachannel that represents the palette itself. the metachannel width would be equal to the number of colors in the palette, so it can be arbitrarily enlarged even when W is fixed.

So like a 100x100 image might need a 350x1 palette metafile?

yeah that might be possible.

I will post the example file to the appropriate thread. Anyway, is there any update on the palette transforms issue?

<:This:805404376658739230> I think we're going for turning libjxl behavior into spec. It will be for an eventual third edition to actually fix it, the second edition is already too far in the process. The spec fix is to simply remove "`, excluding the meta-channels`" from the definition of `dist_multiplier`. So the spec will actually become slightly shorter 🙂

“eventual” as in Dutch “eventueel”, or “uiteindelijk”?

Yes. Not initiated yet, so perhaps we'll have to do this as a defect report and erratum for now. Though I think we can just keep track of it and eventually initiate a third edition that fixes it.

(I meant which one of the two 😅)

Oh, uiteindelijk. That's what it means in English, right?

yes, just wanted to make sure since both interpretations would have made sense

what does this mean?

are there two different words in dutch for the english word "eventual"?

no, but there is a word that is very similar to "eventual" but means something very different

same with "actual", as I recall

oh, like those false friends in french

Some not proficient enough European English speakers use "actual" to mean "current", though I don't know if that applies to Dutch.

"actuellement" is french for "currently" so that makes sense

"attendre" means to wait for, not to attend to attend you'd say "assister à" and to assist would be "aider"

"eventueel" probably means something like, something that may be necessary, but perhaps not

whence the alternative interpretation of "eventual third edition"

hm

"Eventueel" in Dutch means something like "optionally", and "actueel" means "current", so it follows the French meaning, not the English ones.

same as Italian then 😛

I guess it was English that changed the meaning of those words 🙂

I’m not quite sure this is correct, is it?

shouldn’t it be `new_alpha + old_alpha * (1 - new_alpha)`, matching the blending of pixel values?

on wikipedia, the two match https://en.wikipedia.org/wiki/Alpha_compositing#Straight_versus_premultiplied

yeah, it should be the same

libjxl seems to do neither? https://github.com/libjxl/libjxl/blob/de2779d484fdb4631e72f64ce210d46488cb6263/lib/jxl/alpha.cc#L26

that's a different spelling of new + old * (1 - new)

oh, good, thanks (I should have checked)

in regular algebra, anyway, I'm not sure how different the result is in 754 math

what is BitsOffset(n,m) mean in a U32?

in the Container format description

right, we need to adjust that. It means m + u(n)

like 4+u(3) means read a u3 and add 4 to it?

yes

Current draft of 18181-2 ED2. The notations are fixed now.

with 9.12 AppMarker, does the data follow the `length` field ?

or is that not how JPEG appmarkers work

are there any updates to the -1 draft?

(as of 2023-11-03)

the one from 2023-11-03 is still the current state of the document

> I think we don't really have a choice but to turn libjxl into spec. Has this happened? Is it safe for me to use high effort levels without the images breaking now?

Yes, in the second edition of the spec the phrase has been adjusted so what libjxl does is correct

This is the current version of the document (that is, the actual document is a Word document because it's almost published now and the ISO editors use Word, but I'm keeping our own source files in Metanorma up to date)

metanorma ! I was looking for this word 😄

I don't get why it is pushed by ISO, but at the end, it still makes a word document 🤔

oh, it's asciidoc with extra steps ... I'm disappointed

it's basically just asciidoc with a few extensions for stuff you tend to need for ISO standards, and it produces output Word documents that the ISO editors don't complain about so that's good

The very first drafts of the jxl spec were edited in Google docs, which turned out to be a rather bad idea because 1) you can't properly do numbering of sections and referencing in it (which is a rather crucial thing in specs), and 2) when a document gets large and you have a couple of people working on it together, it can get pretty slow and unresponsive and occasionally weird glitches happen.

Then we had to use Word because at the DIS stage ISO gives you a Word file and tells you to make any changes to that document with track changes enabled. And it has to be done in Microsoft Word because otherwise internal stuff gets lost (i.e. you cannot import it to Google docs, edit, and export back to Word). Word does have collaborative editing features like Google docs, but it works even worse, at least on a large-ish document and with track changes enabled. It could become very unresponsive at times.

(in particular, if I wanted to create a version with all track changes accepted — since ISO likes to get both a version with and one without track changes — it could take a full day between pressing the "accept all changes" button and Word becoming responsive again)

Then we used LaTeX+git for a while, which works great, but then turning that into a Word document that ISO editors will accept turns out to be harder than we thought. There are many tools that can do some kind of LaTeX to Word, but none of them really produces fully usable results that don't require tons of manual layout fixes etc.

And now we use Metanorma (+git) and it's quite great. Only annoying thing is that we have to use a private git repo. I would very much prefer to have it on a public git repo but it would very likely cause trouble with ISO.

Glad it's not just me wondering why Word has to be such a monopoly. As seemingly the only person in the world without Office since using it at school, it's always a pain trying to open one when offline. When everyone already uses it, it's great. When you're just trying to read something once, re-uploading it to google docs just to read it for 2 minutes turns into a lot of hassle (Especially with Wordpad getting removed as the last non-paid way to open them locally)

Libreoffice doesn't work? For revisions?

No no. They have special funky stuff inside their doc files that works with some proprietary internal software they have, and somehow it can stay synced as you edit the document in Microsoft Word, but any other software will mess it up.

Basically now I learned to avoid making big changes after DIS, because at that point I have to make the changes both in Word and in our actual sources (which are in metanorma).

for casual viewing why not just use LibreOffice or FreeOffice? using the Google suite for such purposes sounds like a horribly awful experience.

It's a shame they don't sync the metanorma over git (or pijul if they prefer)

sending message so i join thread (new account)

We do that, just not allowed to use a public git...

Should I make sure to not have the draft specifications in my public gate repost?

Repo*?

Yes, best not to distribute it if you don't want to get sued by ISO

is putting it in my `.gitignore` good enough?

As long as there are no versions of the spec on the public web, it's fine for ISO.

Wouldn’t this discord count as public web?

It's a gray zone. You need a discord account, it doesn't get indexed by search engines, so I think it qualifies as private distribution, but it's a gray zone I guess, and it's probably a bit risky for me to share drafts here. But the feedback is valuable.

In K.1: > In case `frame_header.upsampling > 1` and/or `max(frame_header.ec_upsampling) > 1`, the corresponding colour and/or extra channels are first upsampled as specified in K.2. I thought it means upsampling is done first before rendering splines/patches/etc. but in libjxl it seems that color channel upsampling is done after rendering splines and patches. <https://github.com/libjxl/libjxl/blob/ba759b4254a574f89cc930043020f99ffb7ca056/lib/jxl/dec_cache.cc#L178-L197>

Also brought this up in 2023 🙂 (https://discord.com/channels/794206087879852103/1021189485960114198/1101253976030199808)

The specification says > The decoder first reads extra_precision as a `u(2)`. Next, the decoder reads a Modular sub-bitstream as described in Annex H, to obtain the quantized LF coefficients `LfQuant`, which consists of three channels with `ceil(height / 8)` rows and `ceil(width / 8)` columns, where the number of rows and columns is optionally right-shifted by one according to `frame_header.jpeg_upsampling`. This is in Section G.2.2, LF Coefficients, and it is incorrect.

consider a 600x600 image, with jpeg_upsampling specifying half-chroma in each direction

`height/8` and `width/8` are both 75

The specification implies that the channel sizes are `Y=75x75`, `Cb=37x37`, `Cr=37x37`

however, the actual sizes are `76x76`, and `38x38`

instead of right-shifting the channel sizes by 1, instead they're ceil-divided by 2

and then the channels that aren't sub-sampled are padded to be actually twice that of the subsampled ones

Right, I remember it was a bit funky to make it match what exactly jpeg does, and it looks like the spec is sloppy here...

Section I.7.2, the DCT stuff still has `\[` and `\]` (LaTeX stuff)

Annex C.2.5: libjxl checks `D[omit_pos] > 0` (https://github.com/libjxl/libjxl/blob/main/lib/jxl/dec_ans.cc#L178C1-L183C1) while spec only says `D[omit_pos] >= 0`. Allowing it to be zero is not useful so the spec should be updated.

Even if it’s not useful does it cause a problem? Otherwise might be easier to leave it alone

I don't think we urgently need to fix this, but I think it's better not to allow bitstreams to be valid that libjxl refuses to decode. It's currently only a hypothetical issue since no encoder produces such bitstreams, but still...

also for those looking in here I want to point out that there is some discussion here as well: https://github.com/libjxl/jxl-rs/pull/23#discussion_r1779978182

It would be useful to clarify what to do in case of multi-page with animation: see https://discord.com/channels/794206087879852103/803574970180829194/1301915731084185630

in section 9.11 (the jbrd box) of 18181-2: > After reading these fields, the decoder decompresses a single Brotli stream as specified in IETF RFC 7932. the stream doesn't need to be byte-aligned right? this makes integrating into existing Brotli decompressor a bit tricky...

no, I've just read libjxl code, it's indeed byte aligned

maybe the spec should mention that, or am I missing something

it might just be assumed that brotli would want to be byte-aligned

agreed that it should be clearer though

although do we really want a v3 of the spec?

We will do a 3rd edition of 18181-2 to add the gain map box, so might as well put in a clarification regarding byte alignment in jbrd

ah yeah gain map box was a thing, that would be great then

UltraHDR transcoding would be nice if another edition of -1 happens...

That wouldn't really require a change to -1, just an implementation to put the gain map part of the ultrahdr jpeg in the gain map box (and a viewer that actually uses the gain map box)

I suppose so

How does the gain map box work?

In section A.8 of 18181-2: > Qk are corresponding quantization factors from the JPEG XL codestream (I.2.4). what is "corresponding quantization factors"? should I find the correct channel using `component_q_idx` in the header and get the quant matrix of that channel?

Yes, I think so.

It could be useful to make this a bit more specific in the spec

is there any guarantee about jxl encoding of losslessly recompressed quant matrix? I assume they are RAW encoded, but how about `denominator`? can decoders just blindly copy integer values of quant matrix?

uhm, I think you do have to apply the denominator but you can assume that what you'll end up with are integers. IIRC libjxl should only use a denominator of 1, but in principle I suppose if the quant table happens to have only even values you could make the denominator 2 and encode the raw quant matrix like that.

I'm getting multiplier of `0.000490189` from the `cafe` image which is recompressed, and raw integer values matches the table in JPEG 1 spec. maybe I should apply denominator and multiply 2048 or 2047 to get actual values? I'm not sure how quantization works in JPEG...

looks like for some reason there is a denominator of `8 * 255`

I don't remember how we arrived at that, but that's the scaling factor we needed to make the JPEG quantization work in the same way between how jxl does it and how jpeg does it.

We should definitely make this more explicit in the spec.

So basically to check that a RAW quant table corresponds to a jpeg quant table, you check that the denominator is equal (or as close as F16 gets to it, I dunno if it can represent it exactly) to `1.0 / (8 * 255)` and then you can just use the integers in the RAW table directly.

We should probably put it in the spec that it will be RAW encoded, that the denominator will have that value and that 'corresponding' means just taking the ints.

or I guess I can just multiply 2040 and round to get quantization values

yes that will work too, the thing is that libjxl will currently not reconstruct a jpeg if those conditions are not met, so it's best to add the conditions to the spec too, otherwise you could use some other type of quant table encoding or another denominator and it would get ambiguous if that's valid or not (spec wouldn't say it isn't, but libjxl doesn't reconstruct a jpeg).

that's fair

Why shouldn’t we allow having denominators?

it's part of jxl spec, we need one to correctly decode image as jxl

oh I read your question the other way around, I'd say arbitrary denominators makes things a bit complex when doing jpeg reconstruction, and libjxl already rejects such image

> some other type of quant table encoding or another denominator Would these other ways of encoding the quant table be beneficial / detrimental to compression density?

The raw quant table encoding takes maybe 100 bytes if even that, so any savings you could get would be in the order of a few bytes.

18181-2, Table 15: what is `last_needed_pass`? it isn't mentioned anywhere else in the spec other than this table. is this related to jxl HF passes?

hm, https://github.com/libjxl/libjxl/blob/46b4883c998498e2fb5bccd9c50c77d502a7ef12/lib/jxl/jpeg/jpeg_data.h#L111

18181-2, Table 14: the length of `counts` is 17, not 16. libjxl loops from 0 to 16 *both inclusive*.

18181-2, Section A.2: maybe it should say "Tc is 1 if `hc.is_ac == true`, and 0 otherwise. Th corresponds to `hc.id`."

it seems that my jbrd parser works correctly after these changes, now I need to do reconstruction...

congrats

good timing to find these spec bugs before we're submitting a third edition of 18181-2. Plan is to submit the third edition in January and immediately at DIS stage (skipping WD/CD stages)

Do you get a discount on revisions if you own a previous version?

No idea, probably not. But I wouldn't recommend anyone to spend money to buy ISO standards anyway.

Is there another place to- ahem- *access* them?

No, not legally if you want the official version. If a recent draft version is fine, you can scroll up in this channel. I won't comment on illegal options.

I am currently using the recent draft version, however it doesn’t mention anything about the new gainmap boxes (when were they added to the spec?)

I will probably share a new draft next week or so. Gain maps aren't added to the spec yet.

18181-2: bundle tables are using older syntax (`Bits` instead of `u`, `Val(a)` and `BitsOffset(n, a)` instead of `a` and `a + u(n)`). maybe my copy of the spec is outdated

18181-2: it might be a problem only in HTML rendering, but app marker prefix of XMP metadata is... a bit misformatted. `http://ns.adobe.com/xap/1.0/` should not be linked (I guess), and trailing `/>` should be removed

this is the current draft I have for the document that will become the proposed DIS of 18181-2 Ed.3

there's already a proposed text for the gain map box in there, but the comments above still need to be addressed

the notation issues (`Bits` instead of `u` etc) were already fixed in the final version of the second edition

18181-2, Section A.5: the spec mentions `block_idx` but it's not defined (`ezr.run_length` is defined though, which isn't used anywhere else)

18181-2, Section A.5: - padding bits when `!has_padding` is one, not zero - emitting EOB symbol or updating EOBRUN should be done when the number of remaining zero coeffs is positive, not non-negative

Thanks for finding these spec bugs! I think this is the first time someone is independently implementing the jpeg bitstream reconstruction stuff so it was to be expected there would be some mistakes/inaccuracies

I see in libjxl there's `dcoff` array that modifies DC coefficient of reconstructed JPEG, which is derived from dequant matrix. <https://github.com/libjxl/libjxl/blob/8a39b30133c880c873ca7e2bd0911f5c8dcda49f/lib/jxl/dec_group.cc#L231-L233> What are the relevant parts of the spec? Is it related to JPEG 1 spec?

could be there is something missing from 18181-2. IIRC this is caused by JPEG effectively using signed DC values where 0 corresponds to gray, which in case of YCbCr we handled by defining our YCbCr transform that way, but in case of RGB JPEGs we have to compensate for it. Or something like that. Has been a while, don't remember exactly...

pseudocode for inverse horizontal squeeze: `if (W1 > W2) output(2 * W2) = input_1(W2);` should be `if (W1 > W2) output(2 * W2, y) = input_1(W2, y);`

18181-2, Table 18: `reset_point` should be converted to some kind of cumulative sum of decoded array. Maybe the same for Table 19

now I see why it's called `run_length` in ExtraZeroRun...

...or maybe not

does app marker for exif and xmp metadata appear only once in jbrd?

libjxl seems to reject jbrd with multiple exif and xmp markers; is this supposed to be required by the spec or just a limitation of libjxl?

not sure why you would have multiple exif/xmp

wouldn't that be invalid

I think the spec implies that if there are multiple such markers, the corresponding boxes should be consumed in the same order in which the boxes appear in the jxl file, but libjxl assumes there is only at most 1 exif and 1 xmp box.

are there jpeg files in the wild that have multiple exif / xmp markers?

not that I'm aware of. I do think those images will be extremely rare even if they exist

maybe I could just handle one exif/xmp marker and leave a TODO 😉

What if there are too many / not enough boxes?

Take the first, jbrd the rest

it would be nice if the jpeg reconstruction spec clarifies that chroma-from-luma may need to be applied before reconstruction, and also specifies the requirements of various parameters (no LF CfL, base correlation factor of 0, `colour_factor = 84`, ...)

18181-1, in the pseudocode for Patches decoding: `/* there is more than 1 alpha channel */` should be `/* there is more than 1 extra channel */`

I'm adding that bit of pseudocode in the next edition of 18181-2 (and renaming `extra_zero_run.run_length` to `.block_idx`), I hope I got it right

draft version of the study text of 18181-2 Ed.3, which will be input for the January JPEG meeting and which might become the DIS

the JPEG meeting starts January 6th so between now and then is a good time to spot any further spec issues so they can get fixed during the JPEG meeting week.

This is partially addressed in the newer spec. It still says "Tc together with Th, encoded as a single byte, is `hc.id`", where I think it should just say "Tc together with Th is encoded as a single byte", or remove the sentence completely.

and I see there's `jhgm` box definition that contains another jxl payload. is it possible that the payload is another jxl container file, not just a core codestream?

Yes, so it can be a reconstructable jpeg too...

18181-1, Annex C.2.3: `ReadUintConfig()` is called `HybridUintConfig()` in C.2.1

18181-1, Table E.1: condition for `tf` needs to have `and not_xyb`

I noticed something

The spec seems to permit the following scenario. consider a 24x48 image with one group

VarDCT

The first Varblock is 16x16. We place it in the upper left.

The second varblock is 16x16 as well. We place it below the first one, as that's the first place it fits.

The third one is 8x8. It fits in the top right so we put it there.

As far as I'm aware, this is legal but libjxl never does this, and there are no encoders that do this.

List them out of raster order, that is.

Is it intended that this is legal?

It should probably not be legal, I think. Probably we should say that every block has to fit in the next raster order position.

as currently specified you just put each block in the first place it fits

I'm pretty sure jxl-oxide can't decode such image

Neither can libjxl I assume

So I think the spec is just formulated too permissively

Is there any benefit to be gained if it were implemented? I'm assuming no since VarDCT doesn't load block by block anyway

there is no benefit, you can still get the same block layout, just have to signal it in the expected order rather than the weird one

> The type is a DctSelect sample and is stored at the coordinates of the top-left 8 × 8 rectangle of the varblock. **This position is the earliest block in raster order that is not already covered by other varblocks.** The positioned varblock is completely contained in the current LF group, does not cross group boundaries, and also does not overlap with already-positioned varblocks. The HfMul sample is stored at the same position and gets the value 1 + mul.

(boldface mine)

seems to me like this means "the earliest block in raster order it can be placed" but it seems like it is intended to mean "the earliest place, period, if it can't be placed there, it's illegal"

it may just be my reading of it

What should the decoder do if it reads an out of order varblock sequence

The decoder can do anything it wants on an illegal bitstream

the specification only specifies how legal bitstreams are decoded

it could abort with error, make a best attempt, silently ignore the problem, etc.

Send a nuclear warhead to your location?

it wouldn't break spec compliance but that's probably not a very user friendly solution

April fools djxl patch

It's a bit ambiguous, it does say put it at the earliest position in raster order and it says it does not cross group boundaries, but it's not completely clear if your example is legal or not. The intention was that the sentence in bold defines the position and the next sentence makes your example illegal, but you could also read them together and interpret the sentence after the bold one as part of the definition of how to position.

Very small nit: In `K.3.1 Patches decoding`, there is a parenthesis mismatch in the code example: ```c++ if ((mode > 3 and /* there is more than 1 alpha channel */) { ``` I guess one of the first two parentheses should be removed.

In `H.6.4 Palette`, particularly in this snippet from the palette inverse transform code: ```cpp if (index & 1 == 0) value = -value; ``` Given that in C++, `==` has precedence over `&`, this boils down to `if (index) value = -value;`. Which seems wrong compared to how it's done in `libjxl`: ```cpp static constexpr int kMultiplier[] = {-1, 1}; pixel_type result = kDeltaPalette[((index + 1) >> 1)][c] * kMultiplier[index & 1]; ```

pretty sure it's supposed to be `(index & 1) == 0`

Yes it is

In `H.6.2.2 Horizontal inverse squeeze step`, the `y` parameters seem to be lacking for both channels: ```cpp if (W1 > W2) output(2 * W2) = input_1(W2); ``` -> ```cpp if (W1 > W2) output(2 * W2, y) = input_1(W2, y); ```

Right, I thought this was already fixed but maybe it was something similar elsewhere

I think this was about the same change: https://discord.com/channels/794206087879852103/1021189485960114198/1310253069317046304

https://github.com/Tom94/tev/pull/282#issuecomment-2845104793 > Images with premultiplied alpha seem to have alpha multiplied *after* transferring into non-linear colors. What?!? I'm not sure if this is just a bug in the test images, but if this is a genuine thing in the jxl spec, that's terrible and defeats many reasons why one would use premultiplied alpha in the first place. Argh. Besides this rant, it implies we need to unmultiply alpha before doing all the color conversions. But! And here comes the kicker! When using jxl's built-in CMS, it _doesn't do the unmultiplication first_. we might want to look into this at some point

Alpha blending is done in the image color space to match how layers typically work in layered image formats (xcf, psd, etc). You can always make an image in linear space if you want the alpha blending to be in linear space — which does conceptually make the most sense but just isn't how alpha blending is usually done.

If libjxl doesn't properly handle color management in the pre-multiplied case, that's something else and that just needs to be fixed then.

The distinction between sections and entropy streams should be clarified a bit. E.g. lz77 runs can cross streams but not sections...

They can?

I thought Lz77 is tied to an entropy stream

and the windows don't carry over

if they can that sounds a bit like a bug

and probably undesired

Yeah no it turns out they cannot, I got confused. Still it should be clarified a bit better in the spec what the scope of the entropy decode state is.

Does the spec state that backreferencee aren't permitted outside the window?

i.e. distance>0 on the first symbol, disrance>9 on the 10th symbol, etc.

if it doesn't it should

IIRC, the window is supposed to be zero-initialized so if you have this you get zeroes. Not sure if the spec agrees with libjxl here.

ffmpeg's parser currently rejects such streams, it's good to know if they are valid

<@179701849576833024> do you have opinions on this? IIRC current spec and libjxl behavior is to zero-init the lz77 window. I think there is no speedup from avoiding that initialization, since then you need to check every time you have a backreference if it's within the available window, which adds a branch.

I suppose everything has modulo semantics too, i.e. you could have a length or distance longer than the window size. For distance that is not useful but for length it is.

I am not sure you remember correctly 🙂