JPEG XL

The offsets are very practical for <#824000991891554375> though :)

yea, lossy internally is 16-bit, but is there a way to feed the library 10-bit pixel data as a buffer and have it upsample it to 16 before lossy encoding, rather than having me do the upsampling myself

since it already can downsample

I suppose that would require implementing planar input though, since most of those formats are things like gbrp10le

Planar input is something we should probably add, we do planar internally anyway

10-bit with maxval 1023 is something we already have for ppm, though that's outside the libjxl api atm

It's easy enough to implement (just use a different scaling factor in the conversion to/from float), I think we mostly just need to define an appropriate way to extend JxlPixelFormat

Promoting jxlinfo among installed tools was definitely a good idea: https://github.com/libjxl/libjxl/pull/1346 But for Windows users, this doesn't make a big difference. The reason is that the tool (downloaded from Luca's site) needs brotlienc.dll, brotlidec.dll and brotlicommon.dll. It's possible to find and download the DLLs and to run the tool successfully. But it's not very convenient. Furthermore, users must deal with x86/x64. Would it be possible to simplify this somehow for Windows users? (Is anyone here having the same experience actually?)

biggest problem with jxlinfo is it doesn't provide enough info

I think if people are using CLI tools they should have the knowledge of if their OS is 64bit or not (And it'll tell them if they try running a 64bit binary on a 32bit OS)

fwiw there's no 32-bit win11 and win10 32-bit is not sold to consumers iirc

I'd be very surprised if any `jxlinfo` users have a 32-bit OS

jxlinfo could use some info like, for example, it could tell you if the frames are VarDCT or Modular

or if they are lossless or lossy

even `jxlinfo -v foo.jxl` doesn't tell you this

It tells you it is `(possibly) lossless` which just means "modular"

Yes, they should have the knowledge of their OS. But when they find a DLL and they find out that it's x86 while they need x64, the thing becomes too complicated IMHO.

the problem here is that we aren't statically linking the dependencies

or providing them

this is Windows, so we can't just say "use system libbrotlidec.dll"

it's not Linux where that works nicely

I ended up grabbing the dlls from vcpkg, which for whatever reason does default to 32bit binaries

it shouldn't be the users' responsibility to have to go search "brotli dll install" on google

But I had already encountered that for another app, so it was a pitfall I was already aware of

if vcpkg doesn't have lib32 and lib64 distinctions then I wouldn't trust it since wow64 compat is still a problem nowadays

But yes, it should be statically linked into jxl.dll

or provided right next to it

windows checks the same directory, unlike posix

It does, 64bit binaries have a "x64-windows" suffix, you need to call it like `vcpkg.exe install brotli:x64-windows` or else you'll get 32bit binaries

hm, that's backwards

it should use :windows-32 or something and default to 64-bit

Yeah, I have no idea why this is the default they picked, at the very least it should take it from the host OS

Specify it if you're cross compiling, otherwise match the host

The same issue (missing Brotli DLLs) is with cjxl_ng.exe and djxl_ng.exe. This increases the need to deal with it. 😜

sigh, windows is annoying

maybe we should only make static binaries for windows

use MSYS2/mingw-w64

you can install these packages easily

https://packages.msys2.org/package/mingw-w64-x86_64-libjxl?repo=mingw64

i have also a small package installer, which builds from scratch everything needed for a project and can be modified easily to build a lob andits dependencies

<@794205442175402004> dll are good (tm) on Windows (dixit a gcc dev, Pedro Alves)

it's somewhat annoying that we just abort when allocation fails. I understand that we don't want to rely on c++ exceptions, but how much effort would it be to catch (at least some) allocation fails and return a nice error instead of aborting?

or at least print something informative to stderr before aborting

abort in a lib is not nice

exactly

also cjxl just silently crashing on too large input is not nice

so write in C : http://www.rasterman.com/files/c_song.txt ('Let it be' melody)

make C wrappers which catch all

as in make C api functions be written in C++, catch all C++ exceptions & return reasonable error codes

I think we want to avoid compiling with exceptions for perf reasons...

if the goal is not to use C++ features like exceptions, why isn't the library just written in C?

We like to have the convenience of C++ but only to the extent that it doesn't come at a speed cost.

i think that exception case is slow but when no exception gets thrown it shouldn't be that slow

remember reading about it, that C++'s exception handling is actually very optimized for no-exception case

was this measured?

in case of libjxl

because i think it would be slow literally only when it'd otherwise panic

If so, it must have been a long time ago because we're using that compile flag for years

it's kind of popular myth that exceptions make C++ code slow

C++'s exceptions are slow, yes, but disabling them shouldn't bring much gain in perf

how does one benchmark libjxl anyway

could try it to satisfy my curiousity

<https://github.com/libjxl/libjxl/blob/main/doc/benchmarking.md> hmm foundit i g

Main way to test speed is `djxl foo.jxl --num_reps=50`

defaults (without exceptions): ``` % tools/djxl --num_reps=100 ~/Documents/wpedit22.jxl JPEG XL decoder v0.7.0 7f5a2cd [AVX2,SSE4,SSSE3,Scalar] Read 2473321 compressed bytes. No output file specified. Decoding will be performed, but the result will be discarded. Decoded to pixels. 1463 x 2560, geomean: 70.50 MP/s [51.72, 76.87], 100 reps, 12 threads. Allocations: 30605 (max bytes in use: 1.389899E+08) ``` with exceptions: ``` % tools/djxl --num_reps=100 ~/Documents/wpedit22.jxl JPEG XL decoder v0.7.0 7f5a2cd [AVX2,SSE4,SSSE3,Scalar] Read 2473321 compressed bytes. No output file specified. Decoding will be performed, but the result will be discarded. Decoded to pixels. 1463 x 2560, geomean: 76.65 MP/s [61.13, 80.03], 100 reps, 12 threads. Allocations: 30605 (max bytes in use: 1.389899E+08) ```

so it's actually faster with exceptions lol

quite consistently so, running them in whatever order

this is for modular, i guess i can test lossy

with vardct i cant see much of a difference, its roughly the same

as in speed difference decreased a bit but enabling exceptions still didnt make it slower

would need to run them one after another multiple times and averange over results

but from current ones there doesnt seem to be significant difference

Interesting...

it makes sense if you think how it's implemented tbh

exceptions are slow as hell because no checks happen after function returns or such, they unwind stuff

which means that some code possibly in stdlib what would normally have to check stuff, doesn't have to in no-error case when exceptions are enabled

script which runs with and without bins in loop to make stuff fair (it does run them before loop too)

wait i messed this up a bit i think lol

yeh meaning is inverted lol

but like i cant really see much of a difference either way

this one is not messed up

seems like it depends much more on what else happens in my computer than exceptions

Try with --num_threads=0 and with as little other stuff using cpu as possible

ughh second one is hard i have a lot of things running & i dont wanna kill them

maybe if i make negative niceness it'd be okay

Anyway, I don't really see why enabling exceptions would be much slower for us, given that we don't use anything that can throw except for allocations, and those shouldn't be happening often enough to make a difference, I hope

So if we can just enable exceptions and catch them at the api boundary, that would be good imo

yeah.

re-testing modular with 0 threads & -5 niceness r n, it's consistently slower without exceptions still lol

<@179701849576833024> wdyt?

`-q 100 -e 9` modular decoding:

enabling exceptions makes it consistently faster

`-e 9` (vardct):

Enabling exceptions making things faster is incredibly fishy

so kinda variable & in some cases little bit slower with exceptions here. interesting

but it's faster on exceptions with modular

and quite a bit more measurably

im too lazy to draw graph with this

Anyway, the thing with exceptions is not that they are slow (well, perhaps that too), but that they increase code size significantly, possibly hurt optimization (but probably not here) and make the program a lot harder to think about in some cases

``` 5159368 libjxl-orig/build/tools/djxl 5312976 libjxl/build/tools/djxl ``` there's a bit of increase indeed

also ``` 4213472 libjxl-orig/build/libjxl.so 4370840 libjxl/build/libjxl.so ```

that they are slow is kinda irrelevant if codebase don't use them

could say the same about last one

actually looks like overall it's a little bit faster with exceptions

would need to average over results

to confirm

I do agree that libjxl's codebase shouldn't use exceptions

but it seems that removing `-fno-exceptions` and other related flag will bring only good things

well except for slight but kinda significant size increase

but i honestly don't see any other way solving aborting on malloc

other than not using STL containers at all and never doing new(), only malloc

id actually argue that speed increase alone is worth it, if proved by more serious benchmarking on machine with SMT disabled, clock speed locked & running nothing else

how is the size increase after stripping?

``` 3640752 libjxl-orig/build/libjxl.so 3743296 libjxl/build/libjxl.so ``` still increase

If size increase is the only real drawback, then we could keep no-exceptions for libjxl_dec (which is for size-critical applications) but remove it for libjxl, maybe?

those sizes seem a bit surprising

ah, right, the encoder is there too

Still a bit large, is this a release build?

tbh, we can probably get 95% of the way there by properly failing instead of aborting for image buffers

``` 4547336 libjxl-orig/build/tools/djxl 4645640 libjxl/build/tools/djxl ```

it's by far the worst offender for OOMs

Yes, image buffers are the only big allocs so the number of spots that would need to add a check could be limited

In any case it would be nice if the library doesn't bring down the whole application in case of OOM, but returns error so the application can handle it gracefully

tbh -fno-exceptions also has quite some problems with ODR and the STL which I'm not sure how to deal with

(well, and in general using any other C++ lib)

Imagine working on a large image in gimp and then when you try to save, it makes gimp crash

> In any case it would be nice if the library doesn't bring down the whole application in case of OOM, but returns error so the application can handle it gracefully *cires in Linux*

you can disable overcommit if you close your eyes and pretend that fork doesn't exist

Technically speaking, regardless of your settings or the presence of fork(), Linux will always launch the OOM killer if the kernel needs more memory than is free.

Disabling overcommit just makes it less likely to happen.

having proper OOM handling will open path to have user-supplied allocation callback working for all of allocations

which would be handy for limiting libjxl's memory usage

i think 0.2% difference is totally plausible from minor code alignment changes. linux kernel has a DEBUG_FORCE_FUNCTION_ALIGN_64B option which does what it says on the tin. clicking around on the initial submission (32B) https://lore.kernel.org/lkml/1595475001-90945-1-git-send-email-feng.tang@intel.com/, changes of 2.5%, 5%, even 30% for one microbenchmark were seen. also https://users.encs.concordia.ca/~shang/soen691/2016winter/papers/producing-wrong-data-asplos09.pdf, https://people.cs.umass.edu/~emery/pubs/stabilizer-asplos13.pdf

but it's change for the better though. someone else cares to benchmark on something else to see if it gets worse?

mhhh, that'd be (I assume) because of changing what the branch predictor cache would do, I think, which would indeed explain this as IIRC modular is a lot branchier

i think there are a few factors, branch predictor cache is probably one, but also e.g. intel processors are (allegedly) very sensitive to jump target alignment, especially for tight loops. e.g. the first paper mentions intel loop stream detector, i think if your loop crosses some boundaries then it may get much slower. gcc does some alignment by default but iirc not full alignment (too much and you bloat the cache instead)

my CPU is AMD

I don't reject that it may be because of difference between how alignment works, but imo it could also be because of how some failures in stdlib get handled (explicit checks versus leaving it for exceptions)

im kinda hesistating to accept that hypothesis until consistent slowdown is shown on some other system/arch

don't wanna trust "in theory it could be..." arguments because that's how `-fno-exceptions` usually gets into codebases in the first place

amd manual claims that loop alignment isn't necessary on ryzen, but i think gcc still does it

i doubt libstdc++ does any "explicit checks" when exceptions are disabled. it doesn't use `__EXCEPTIONS`, and for `__cpp_exceptions` it seems to just call `__builtin_abort` instead of `throw`. if anything, it does more checks when `__cpp_exceptions` is defined, not less.

`throw` doesn't "do checks", that's the thing. it unwinds the stack, that's why it's slow. but the path when exceptions aren't raised is uncluttered

my point is that it specifically doesn't do `#ifdef __cpp_exceptions catch () #else if (rv < 0)`, it does `#ifdef __cpp_exceptions throw #else __builtin_abort()`

id need to dig into stdc++ because r n i dont really have enough data to argue either way tbh

I would like to note that how things are laid out in memory can have huge consequences on performance. Even having a different length username on Linux can have a noticeable performance impact. Most compiler switches which affect performance also affect layout, so simple comparisons don't actually tell you anything unless the recorded difference is large.

i just stubbornly wanna know if this makes perf worse for some machines but nobody other than me wants to test >_<

but like it probably doesn't matter either way lol

as the difference is quite tiny

It's not even a per machine thing. The true difference between having or not having exceptions on your machine may be opposite to what your benchmark showed, because the number of factors that may have changed between the two versions is huge.

I can try to test it on my computer later though.

This talk describes the problem I'm talking about: https://www.youtube.com/watch?v=r-TLSBdHe1A

Perhaps we could try the tool they developed for benchmarking.

that guy is one of the co-authors on the second paper i linked

and he is talking about the "Stabilizer" software from the paper

oops, that's what I get for not paying attention!

Tree leaves again: from observing generated trees, it frequently happens that leaf nodes have identical predictors to their siblings - especially as we have always offset == 0, multiplier == 1 We can't just merge these pairs of leaves as each leaf has its own distribution However after distribution clustering, some such sibling distributions may have been clustered together, meaning they share a context. Hence both can be merged into their parent (i.e. removing the parent's property decision). If correct: while this idea doesn't help with compression size (save for the tiny benefit of trimming some branches), the intended benefit is at the decompression end, where having a smaller tree may improve decompression speed Does this sound plausible?

Yes

Would also save a few bits in signaling the MA tree, not very significant but still

Could also apply this optimization decode side btw

Decoder already has some optimizations to avoid branching in MA traversal, but it may not cover some cases like this

is there a way to get better error messages out of `JxlEncoderAddImageFrame`?

it's failing and I don't know why

update, it appears it's expecting 3 channels, received only 1

however, I set `JxlBasicInfo` with 1 color channel

as well as the `JxlPixelFormat`

so I'm not sure what's up there

Could be a bug on our end too

Ah, what color encoding did you set?

Maybe you didn't set a grayscale color encoding?

eek, frequentist statistics (18:55)

I didn't set a color encoding

I figured that setting 1 channel would imply grayscale

however, `uses_original_profile = false` in this case, so I figured setting a color encoding was unnecessary

I'm wondering if `uses_original_proifle = false` implies xyb internally, which gets confused if there's one channel

might need an extra branch so it only assumes xyb if `num_channels = 3`

I guess we should make the implicit colorspace be grayscale when basicinfo has only 1 color channel

It uses xyb for lossy grayscale too, just x and b are zero then

that seems more sensible than assuming xyb and immediately failing tbh

I did a quick test on the decode side as you suggested; this eliminates between 1-4% of the leaves, depending on the image. There's a tiny but detectable speed increase, 2.5% at best.

not bad!

I mean, it's not a huge optimization of course, but every small bit helps

so I think we should do this both encode and decode side, it's probably not a lot of code to do recursive bottom-up merging of identical siblings

feel free to make a PR, I'll gladly review it

I had originally tried doing it on the encode side but it got messy (e.g. running BuildAndEncodeHistograms twice)

I'm still focusing on trying to understand the spec so won't be making PRs in the near future (though the code is one tiny fn if you want me to just paste it)

Is there a good way to submit feedback on the spec (the part1.pdf you posted a while ago) other than discord?

not really

I mean

the official way is that you contact your national standardization body so they can submit a defect report to ISO

which we then receive by the next JPEG meeting if we're lucky

and then we have to respond to the defect report and propose an Errandum, Corrigendum or Amendment to the spec

which can then eventually get consolidated in a next edition

the unofficial way is that you just tell me, I fix it on the spec repo if needed, and either it gets fixed in the current 2nd edition (it's in CD stage now, so still some stages before it's done) or it will be on the stack of pending fixes for an eventual 3rd edition

this is the actual CD text by the way, which was approved last week. it should be made public at some point by ISO but they can be slow with that - they're only fast when it is to remove stuff and put it behind a paywall, making things public is slower...

a PR just for the decode side is also nice, you're right that doing it encode side might be a bit messier since I guess the clustering gets done after writing the MA tree atm...

thanks for the pdf

My main criticism of the spec is that it needs much more of an adversarial perspective. It tells you what jxl files look like, but it's less clear how to decode an arbitary bitstream as a jxl file (or not).

For instance the modular_16bit_buffers field: what does it mean for 16-bit integers to be large enough to store and apply inverse transforms to <particular data>? Different implementations may cause the intermediates to grow by different amounts. Is a confirming decoder expected to detect whether or not the field is 'lying'?

these kinds of specs only say how to decode a valid file. What to do with an invalid file is outside the scope of the spec, and is considered a 'quality of implementation' issue — as far as the spec is concerned, the decoder can crash and explode on bad input.

but you may be right that it's not always clear enough what a valid file is

But if the spec doesn't tell you the difference between valid file and a not valid file, how do you tell the difference between creative use of the file format and a non-conforming file?

whenever the spec says something like "this value is smaller than 10", it means that if it is not smaller than 10, it's not a valid bitstream and the decoder behavior becomes undefined

modular_16bit talks about the numbers you get in image buffers (both before and after transforms). We assume that implementations can make sure to do arithmetic (e.g. to compute a predictor) with wide enough integers when needed, but the buffers themselves should fit in int16_t if modular_16bit is true

and no, a decoder is not expected to detect when the field is lying, it can just produce a glitched image where the pixels look very different (due to overflows) from what you would get when using infinite-precision numbers as in the spec

because when the field is lying, the bitstream is not valid, so the decoder can do whatever it wants

you can make a decoder that detects such cases, refuses to decode the image, and gives a nice error message about it

you can make a decoder that just displays a glitched image

you can make a decoder that makes your computer explode

as far as the spec is concerned

as far as the libjxl implementation is concerned, we of course don't want security bugs, but we don't really care what happens on bad input as long as it's not a security problem: both refusing to decode and decoding a glitched image are OK — and in cases like integer overflow, checking this explicitly would be too much of a slowdown (it would be at least one extra branch per pixel, probably more in case of transforms) so we'll tend to do the latter

refusing to decode is preferred when the condition can be checked outside hot loops, like header-level stuff.

files like this are not valid at all: https://discord.com/channels/794206087879852103/824000991891554375/927220411383554121

https://jxl-art.surma.technology/?zcode=c8osSUktKMlQMDbk4tJVCFcwNQADLgA

something like that should according to the spec just decode to something that gets brighter and brighter as you southeast, but it's OK that our decoder doesn't do that (due to int32_t overflow) because the spec says that values have to be representable in int32_t, so this is not a valid file

(it's also not valid because the level 10 box is missing and it has a bitdepth that is too high for level 5, so it's not properly marked)

strictly speaking files like that are not jxl files, they are just malformed codestreams that happen to decode because the decoder happens not to check, which is an OK strategy as far as the spec is concerned

of course nobody should rely on that file decoding to those pixels

To be clear then: a conforming decoder is not required to detect or reject an invalid jxl file.

For modular_16bit_buffers: would it be better to say instead "modular_16bit_buffers indicates whether or not all the decoded samples in modular image sub-bitstreams fit in signed 16-bit integers" - then the spec doesn't have to make assumptions of how unknown implementations do their arithmetic and integer carries?

Correct. A conforming decoder is required to correctly decode every valid jxl file. What it does with invalid files is undefined.

No, because the decoded samples (i.e. after undoing all transforms) could fit in int16_t, but int16_t buffers could still not be enough even disregarding arithmetic. For example, the decoded samples could all be nicely in the -2^15 to 2^15-1 range, but if the RCT or Squeeze transform was used, the values _before_ inverse transforms could be outside that range. These transforms can only be undone after all channels are decoded, so there is no way in that case that an implementation can avoid having to buffer values that are outside the int16 range.

so it's about the values that come straight out of entropy coding, the values you get after undoing the first transform, the values you get after undoing the second transform, etc. until the final output values. They all have to fit in int16_t.

Now it is true that implementations might be able to avoid some of these intermediate values, e.g. you could in principle do a 100,000-color palette (so the palette indices are outside int16_t range) without actually storing the palette indices, immediately undoing the palette transform as you decode. But we don't want to assume that implementations will do such optimizations (it's very tricky to do that in general anyway), so this would still violate the modular_16bit_buffers criterion since at least a naive implementation (as well as the current libjxl) would have to store values that exceed the int16_t range.

(note: libjxl currently never actually uses int16_t buffers, it always uses int32_t buffers, so it doesn't really care about modular_16bit_buffers. But in principle we may want to have a memory-optimized implementation in the future, or a hardware implementation, that does use 16-bit buffers)

How arithmetic is done, there we do assume that it is done with sufficient precision. E.g. for the AvgW+N predictor, we do assume that the (W+N)/2 computation is done without having an overflow in the W+N (so these computations are done in int64_t in current libjxl). Modular_16bit_buffers is only about the buffer precision, the precision of arithmetic is assumed to be infinite in the spec: everything is defined using mathematical numbers, i.e. "real" numbers (which have a strange name because no real implementation can actually really use those)

Ok - so "When modular_16bit_buffers is true, it indicates that when inverse transforms are applied to the decoded samples in modular image sub-bitstreams, the inputs and outputs of each successive transform can all be represented by signed 16-bit integers." which deliberately doesn't assign meaning to modular_16bit_buffers=false.

This was much lower down my list but since you mention 'infinite precision'... I assume that 0.7071067811865476 in the AFV matrix is meant to be sqrt(2)/2 ? To a few more decimal places, sqrt(2)/2 is 0.7071067811865475244. Whichever way you do the rounding, you can get 0.7071067811865475 and you can get 0.7071067811865476, but you can't get 0.7071067811865474 (which occurs in the AFV matrix, negated). My guess would be that this is a rounding error on the part of the arithmetic package that generated the constant from cos(pi/4) as opposed to something germane to how AFV works. The real point here is: Why not define the AFV basis by what it really is (i.e. with infinite precision) as you have done with the more widely-known DCT? This saves implementors from having to reverse-engineer it to take advantage of any structure present. If the JXL spec isn't the place to define how AFV works then adding a reference to the algorithm would be ideal. Similar comments for the magic constants elsewhere in the spec where appropriate; there are quite a few of them.

I think most of such magic constants are just taken from the code and given in the spec with enough decimals to be the same binary32 float number

But yes, I agree that giving them mathematically would have been nicer

We did that in some places, like in splines iirc

But in most places it's just magic numbers with a seemingly arbitrary number of digits

I think in some cases there just are no mathematical numbers, i.e. the constants were the result of some numerical optimization using somewhat arbitrary objective functions (like butteraugli scores on some corpus)

For AFV, I don't really know the mathematical derivation. It's a dct8x8 with 3 corner pixels ignored for the dct and coded separately, if I understand correctly. <@179701849576833024> / <@532010383041363969> is there a mathematically nicer way to formulate the AFV transforms than with a big bag of magic constants?

Yes but I don't remember what way it is 😂

Wdyt about trying to replace magic constants with the mathematically exact version?

Doesn't really make much difference in practice since we give numbers with quite high precision, but it does look nicer and corresponds more with how we define the normal DCT

I can try and ask Thomas

it could be that they just are "root of 16th degree polynomial near 0.001", which is not incredibly useful as a closed form, though

AFAICT, AFV starts off with something like a 2x2 DCT of 3 pixels and an 4x4 DCT of 13 pixels with redundant coeffs removed - i.e. what <@794205442175402004> said. Though there's a linalg step afterwards that does some strange things and mixes the two sub-DCTs more than seems necessary. See e.g. columns 1, 5 of the forwards AFV - both have 11 identical entries at the end which tells me that they're trying to be a coeff of the 2x2 DCT coeff but have a multiple of vector 0 (the DC) added. However you only need to add DC to one of them to satisfy the 'single 8x8 DC coeff' requirement - right?

Did you plug the 16+4 vectors into a linalg package and ask "Give me a normalised basis for these 68 vectors that contains the DC coeff" ? (edited: fixed failure to count)

the *real* answer is "this was way too long ago for me to remember"

but I'll see if we can reconstruct it

thanks I'd be interested - IMO reducing opacity is good for both the spec and its implementations

It's also good in case someone wants at some point to make a 16x16 AFV for some future codec or something like that.

just to be clear, `JxlEncoderSetColorEncoding` sets the description of the pixel data passed, right?

also, I'm getting that libjxl is returning sRGB no matter what

even though the input is PQ and Rec.2100

is libjxl synthesizing an ICCP?

right

yes

even if you set preferred color profile to Rec.2100 PQ?

didn't you say that libjxl always outputs sRGB data if an ICCP is present?

for Rec.2100 PQ the encoder doesn't need to store an ICCP, it can use the enum

but it *is* outputting an ICCP

are you using lossless?

VarDCT XYB

strange, then it should detect that your input icc profile corresponds to rec2100 PQ and use the enum to represent it without spending bytes on the icc profile

jxlinfo properly reports this

there is no input ICC Profile

I set the enums

on encode

and then on decode I get back sRGB and an ICC Profile

Color space: RGB, D65, sRGB primaries, 709 transfer function, rendering intent: Perceptual

??

that's what the enum says in that file

just to double check that I uploaded the right file `ec57f9e37a41e638f1a69b043b327f95`

md5sum, correct?

ca2dca6039c4709655ece9c2f2eb2f26

oh

oh wait

I already had a test.jxl so it saved it as test (1).jxl, lol

``` $ jxlinfo test.jxl JPEG XL image, 3840x2160, lossy, 16-bit RGB Color space: RGB, D65, Rec.2100 primaries, PQ transfer function, rendering intent: Relative frame: full image size ```

Color space: RGB, D65, Rec.2100 primaries, PQ transfer function, rendering intent: Relative

yup

it appears libjxl decodes this as sRGB and synthesizes an iCCP

yes so that looks good

I'm not sure why

are you setting a preferred colorspace when decoding?

and are you decoding to uint?

I'm decoding to uint16

right, it would actually make more sense if it would just decode to rec2100 PQ when decoding to uint16

when decoding to uint8, it's not crazy to default to sRGB, but when decoding to uint16 there is enough bit depth

and I didn't set a preferred colorspace, I first asked libjxl if it had an ICCP, and it said "yes" so I said "cool" and decoded the pixel data to sRGB and attached that ICCP

`target_data` btw

I think current behavior, if you don't set a preferred colorspace, is to just always decode to sRGB when decoding to uint and to linear sRGB when decoding to float

if the bitstream has no ICCP and has enum values, shouldn't it just decode to that

and set the enum values appropriately

or at least, do that by default

the only reason I'm getting an ICCP is cause I asked if there was one and libjxl returned "yes"

yeah, maybe... I dunno, when decoding to uint8 it might be better to clamp away the wide gamut and bright stuff and return it as sRGB instead of putting a rec2100 pq in uint8 which will be a quite bandy experience

how do you ask if there is an ICCP?

`JxlDecoderGetICCProfileSize`

check if nonzero

JxlDecoderGetICCProfileSize will always return something non-zero, because libjxl will just synthesize something if needed

ah.

so that documentation is pretty misleading

yes it is

the only way it will return zero, is if the image is tagged with the enum to have an `Unknown` colorspace

but there is no encoder atm that will produce such files

and it's probably not something you ever want to do except in very niche cases where interoperability is not an issue

It sounds like, I should first check `JxlDecoderGetColorAsEncodedProfile` *first* and if I like the color_encoding returned (i.e. I can work with it), then I should pass it to `JxlDecoderSetPreferredColorEncoding` and then handle the pixel data returned.

and if I don't like that color encoding, or there's an embedded ICCP, then instead retrieve the ICCP and use sRGB instead.

yes, except it's annoying that you get JxlDecoderGetColorAsEncodedProfile as an icc profile while JxlDecoderSetPreferredColorEncoding needs an enum space as input

JxlDecoderGetColorAsEncodedProfile returns an enum space

or rather it populates a `JxlColorEncoding` struct, moe specifically.

ah right

cause the docs seem to imply that you should always check for ICC Profile first, and then only fall back on EncodedProfile if the ICC Profile fails.

ugh we use color profile, color encoding and color space in such an inconsistent way in those function names

in either case, is this the best plan of action if I support a variety of color spaces?

yeah it's the other way around, there's always an ICC profile but not always an equivalent enum

the only annoying thing is that in ICC case, you can get clamping when decoding to uint if the ICC profile has a wider gamut than sRGB

If I do this: ```c jret = JxlDecoderGetColorAsEncodedProfile(ctx->decoder, NULL, JXL_COLOR_PROFILE_TARGET_ORIGINAL, &jxl_encoding); if (jret == JXL_DEC_SUCCESS) { JxlDecoderSetPreferredColorProfile(ctx->decoder, &jxl_encoding); ``` will the result of `JXL_COLOR_PROFILE_TARGET_DATA` match `TARGET_ORIGINAL`?

or does that still depend on the `jxl_pixfmt`

it should, yes

I'll run it like this

```c jret = JxlDecoderGetColorAsEncodedProfile(ctx->decoder, NULL, JXL_COLOR_PROFILE_TARGET_ORIGINAL, &jxl_encoding); if (jret == JXL_DEC_SUCCESS) jret = JxlDecoderSetPreferredColorProfile(ctx->decoder, &jxl_encoding); if (jret == JXL_DEC_SUCCESS) jret = JxlDecoderGetColorAsEncodedProfile(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_DATA, &jxl_encoding); ```

I said that I thought the spec needed more of an adversarial perspective.

Re: Annex M ("profiles and levels"), here's a example of a jxl file that I _think_ is a valid Level 5:

yes, the code actually has a safety check to limit the maximum spline area (amount of pixels being painted), which is a better limitation than just limiting the number of splines (since a spline can cover just a few pixels but it can also cover basically the entire image)

question is though where to draw the line (no pun intended), setting the limit too low can result in an annoying constraint for future encoders or things like svg2jxl, setting the limit too high results in images that take a disproportional amount of time for their size (since pixels could end up needing to be painted a _lot_ of times)

so for now we didn't put a strict limit in the profile yet, but I think we should

I'm also wondering if "Maximum total number of pixels in consecutive zeroduration frame" lets you squeeze in arbitrarily many reference frames. The default frame duration is 0 (suggesting that you can't) but the spec also says that a reference frame "is not itself part of the decoded sequence of frames", suggesting that you can. To me it's ambiguous. I think Annex M disallows certain uses of modular mode that are creative but still result in fast decodes. For instance I was pondering a while ago using a reference frame with a very large number of channels - but with each channel only the fraction of the size of the output image. This wouldn't be allowed under the current Level 5, even though the total pixel area is roughly the same as a 'normal' image. On the other side, I think it's possible to make an image that is accepted by Level 5 but still has a crazy slow decode time - e.g. by using pathologically annoying MA trees, lots of splines and blended patches. FWIW the way I'd do Annex M would be to choose constants A and B, set a budget of width * height * A + B and assign a rough cost to each step of the decoding process that's used. The point being that upon receiving an image, you can read the header to examine its width and height. A reasonable user will accept a decoding time that's linear in the image size, so you can then decide whether or not to proceed. The advantage of this approach is that it allows almost free use of modular mode as long as it's fast, i.e. any image that decodes quickly should be accepted by Level 5. A possibly heretical suggestion for Level 5: insist that all parent nodes of a weighted property / predictor node are 'channel' or 'group' properties, i.e. the weighted predictor must be in a tree of essentially size 1.

max total number of pixels in consecutive zero duration frames does include frames of any type, including kReferenceOnly and kSkipProgressive. We should probably clarify that. It's about how much effort the decoder has to do.

the way we use weighted predictor in cjxl -e 3 is with a fixed tree that uses the weighted error as context. it wouldn't satisfy that criterion, but it's still a tree that can be evaluated without any branching, it's just a lookup

I like the idea of limiting things based on something like total number of samples to process (so extra channels at 1:8 resolution are not that much of a problem), where "process" means entropy decode, undoing a transform, painting a spline, blending a patch, etc.

it's somewhat tricky to formulate things in a way that is 1) easy to check as early as possible in advance, 2) easy to write down in the spec, 3) doesn't assume too much about how things are implemented, 4) robust to adversarial examples

yes it is

I think some things could be checked as you go along - e.g. for the MA trees you could have a limit on tree size but also a limit on the number of traversals below 'c' and 'g' properties at the root (e.g. an average of 4 per pixel). Then a decoder keeps count as it goes along and MAY abort the decode if the cost gets too high.

e.g. having 64 extra channels at 1:8 resolution is only really one extra channel in terms of samples, so we could allow that... but then if those extra channels end up getting used in a later frame as an alpha channel for patch blending, they do in fact need to get upsampled by the decoder to image resolution, and while an advanced implementation can do that on-demand and in a memory-limited way, a more naive implementation could end up using 64 full-sized buffers...

cjxl fails to encode this gif

only the first frame renders

cjxl v0.7.0 ddbf206 [AVX2,SSE4,SSSE3,Scalar] cjxl -e 9 -E 3 -I 1

What does jxlinfo say?

it says that the frames are there, but they're 0 ms

oh

maybe the gif actually has 0 ms frames too, but you don't notice because most viewers enforce a minimum frame duration on gifs

yep, that's it

hmm, interesting, thanks

https://webmasters.stackexchange.com/questions/26994/why-is-this-gifs-animation-speed-different-in-firefox-vs-ie

technically cjxl just interprets the input literally

perhaps it would make more sense to make it emulate whatever chrome does, or something

https://wunkolo.github.io/post/2020/02/buttery-smooth-10fps/

it's a bit of a mess, it seems

browsers nowadays allow up to 50fps (0.02s frame duration)

BUT if you set the duration to something smaller than that (i.e. 0.01s or 0s), they will not play it at 50 fps but at 10 fps

probably for some kind of legacy reasons

then some players will just play 0.01s at 100 fps and 0s at whatever the fastest is they can do (using max cpu)

I see. I went and made the gif 60 fps, and the converted jxl is animated and frame limited in browsers as well.

If high framerate gifs are widespread, it might make sense to have default behavior explicitly lower the framerate, if only to make playback the same on all platforms. But that in itself could be treated as bugged encoding. Dunno

id say the default should be to emulate browsers

with switch to take stuff literally

i wonder how apng encoders/decoders interpret it

Yes

How can we run a specific test? I've read the building and testing doc, and tried running ./ci.sh test jxl_jni_decoder_wrapper as well as ctest jxl_jni_decoder_wrapper in the build dir, but in both cases all the tests are executed, not only the one I specified

You can run a specific executable in build/lib/tests

Oh boy, this does not seem to good: https://github.com/BtbN/FFmpeg-Builds/commit/22eebe8ea63df58eca455ea18a15c0a6f9fc26f7

I don't think this is supposed to happen at all.

https://github.com/BtbN/FFmpeg-Builds/issues/151

I'll look at the issue report and try to reproduce it.

Probably an msvc bug

It's ffmpeg though, so mingw is being used.

I'll check with git libjxl and git ffmpeg to see if I can reproduce the issue.

mingw is gcc right?

<https://github.com/shinchiro/mpv-winbuild-cmake/blob/master/packages/libjxl.cmake#L38-L39> heh

Only scalar is in any case a bit overkill

SSE3 might be fine

Default is GCC yes. You can technically build mingw with Clang, but in 95% of cases, mingw= GCC.

What worries me is that both lossless and lossy work fine on my end in ffmpeg.

greeeeat

what's your CPU?

Zen 2 3700X.

Always fun to debug stuff that happens only on certain environments/machines

I am currently suspecting that some hwy feature detection misfires in some cases

That would explain things, yes.

i think its related to https://github.com/libjxl/libjxl/issues/1282 ? i downloaded d98c707 from https://artifacts.lucaversari.it/libjxl/libjxl/ ran it, no output, hooked up a debugger and breakpointed on the cpuid instruction and flipped the avx2 bit to 0 and got output it also works on my older machine in windows which doesn't support avx2

<@179701849576833024> Any luck tracking down the derivation of the AFV matrix? I had hoped it would just be 3 rows of a 2x2 DCT and 13 rows of a 4x4 DCT with sensible choices for the missing 1 + 3 coeffs (modulo massaging in a single DC coeff and an orthogonalisation step).

However after looking at it for a bit, I don't think this is the case.

the person I wanted to ask is OOO, remind me again next week? 🙂

sure

The first two rows of the AFV matrix M are

`[0.25 0.25 0.25 0.25 0.25 0.25 0.25 0.25 0.25 0.25 0.25 0.25 0.25 0.25 0.25 0.25 ]` `[0.87690293 0.220651811 -0.101400504 -0.101400504 0.220651811 -0.101400504 -0.101400504 -0.101400504 -0.101400504 -0.101400504 -0.101400504 -0.101400504 -0.101400504 -0.101400504 -0.101400504 -0.101400504 ]`

This matrix is the output of a QR decomposition (right...?) meaning that LM == X for some lower-triangular matrix L and the original matrix X.

The first row of the original X is just the DC - fine. The second row of X must be some linear combination of the first two rows of M. It looks like it wants to be a coeff of the 2x2 DCT, i.e. mostly zeroes. So if we use the first row to zero out all the -0.101... coeffs and then scale to get a 1 somewhere then we get `[3.037715891 1. 0. 0. 1. 0. 0. 0. 0. 0. 0. 0. 0. 0. 0. 0. ]` For a normal 2x2 DCT all the coeffs are 1 and sqrt(2) (modulo scaling), so the ratio between any two coeffs is at worst a multiple of sqrt(2). However the 3.037715891 doesn't look like it fits into this.

AFV: The three corner pixels are a simple separate ad hoc transform. The remaining 13 pixels are a normalized orthogonal transform inspired by the DCT -- however, not 4x4 DCT, nor hadamart.

<@532010383041363969> thanks, I'm glad I hadn't missed something obvious. Would you mind posting the original matrix?

is there a `#include <libjxl/common.h>` to pull in things like `JxlPixelFormat` or `JxlColorEncoding`?

that I figure would be included by the decoder or the encoder

so I have *some* more info: the matrix is obtained by "gluing together" eigenvectors of two 3x3 and 13x13 matrices - which implies that getting a nice formula for most of those coefficients is... extremely unlikely

Great, do you have details of how the 3x3 and 13x13 matrixes are derived? I don't think that neat closed expressions for the coefficients are realistic - normalisation alone poses enough of a barrier to this. The eigenvector construction sounds like a process that could be documented for someone to reproduce if they so wish.

FWIW for the 13x13 I think I can see butterflies across the diagonal and three 4x4 sub-transforms, but it really should be documented properly.

it wasn't done that way 😄

I'm not 100% sure of the details myself, I'll get back to you

thanks

so tldr is: create a 4x4 grid graph, "unlink" a corner (cells (0, 0), (0, 1) and (1, 0)) from the rest, write down the laplacians of the two connected components

eigenvectors of those two things are how the AVF basis was created

hmm

```from numpy import * from numpy.linalg import eig m = matrix([[2, -1, -1], [-1, 1, 0], [-1, 0, 1]]) evals, evecs = eig(m) print(evecs) # eliminate component 2 x = evecs[1] + evecs[2] x /= x[0,1] # compare with [3.037715891, 1, 0] from AFV print(x)```

is that right?

and is your description consistent with what <@532010383041363969> said? i'm willing to believe it is but the connection isn't obvious to me

well, I guess actually it should be the eigs of the 16x16 matrix (although how disambiguated, I don't know)

From my point of view it was necessary for the 13 other values to be orthonormal and different from dct so that we have an alternative transform rather than more of the same -- usually I would have used optimization to find such values, whatever produces best results on photographs, but here we happened to have a more theoretical approach handy, so to save time we just used those values

i can repro it except for the top corner

was that ratio a 'whatever produces the best results' number?

honestly? no clue

I don't remember

fortunately it only affects 3 vectors, so not too bad (and one of them is the all-equal basis vector, which is simple enough :P)

hello

to add a new framework supporting libjxl, should I just provide a PR for README.md ?

hmm, software_support.md actually

Yes

done

<@794205442175402004> i guess you don't review on sunday ?

Oops

thank you

The top corner was something I and Luca haggled/hacked/kludged (and I tweaked the quantization coefficients afterwards) -- we wanted to have some independence for defining the 3 values. IIRC, we had very little concern of theoretical principles around the three pixels. The idea was that another object is peeking into the transform and often only covering the corner pixels, but sometimes extending to one or two of its 4-connected neighbours

because of that idea (IIRC) we had the corner pixel as a separate coefficient

(I'm writing this from my fantasy on how it should have been -- didn't check the code/spec) 🙂

Sorry to interrupt you Jyrki, but it just me or are pipelines failing in everyone else's forks, too?

checking

what is the command line that doesn't work?

or something on github is broken?

Failing pipelines are: `arm-linux-gnueabihf`, `aarch64-linux-gnu` (both normal and lowprecision), `vcpkg / x64-windows-static`, `vcpkg / x86-windows-static`.

for me linting with './ci.sh lint' stopped working -- but I manage to paint myself in a corner regularly with git/github

Eugene will look at the failing pipelines

Going to look into ASAP

I'm pretty sure the top corner is still obtained that way, looking at the code, except rotated differently

how we did that I don't recall

<@179701849576833024> when you say 'code' I'm not sure whether you mean libjxl or the original code that generated AFV... but in either case I couldn't manage to get that coeff out of the eigenvectors of the graph (0,1) <-> (0,0) <-> (1,0)

no no it shouldn't be eigs of that graph

but rather eigs of the whole thing put together

with the removed edges

... maybe it doesn't actually change anything and I misremember where we got it from

Btw I was right about the butterflies across the diagonal and the 'sub-transforms' - it's part of the structure of the matrix. The char poly factors over Z into polys of degrees 4, 4, 3, 1, 1; so I was seeing the sub-matrices corresponding to the first three. One immediate consequence is that closed form expressions for the AFV coeffs aren't too bad e.g. `4*t^3 - 32*t^2 + 80*t - 60` (modulo normalisation) where t is a root of `x^4 - 14*x^3 + 66*x^2 - 124*x + 78.` Having a 'hidden algorithm' in the spec is decidedly non-good IMO. I contend that it's possible to define AFV concisely and precisely; the only decimal you need is the unknown coeff from the top corner transform, viz.: 3.0377158905213184. A benefit of doing it this way is that once readers know that the matrix came from spectral graph theory, they may be inclined to write a much cheaper implementation of AFV that exploits the structure of the matrix, instead of doing a schoolbook matrix multiply like libjxl.

I think so - the graph components are disjoint so the laplacian has the two 'sub-laplacians' along a block diagonal. So you should end up with essentially a union of eigenvectors rather than anything more involved

oh, interesting that the poly factors, I didn't expect that

right, that makes sense

what did you get for those 3 coeffs eigenvectors and what's actually in libjxl?

iirc there's a [1, 1, 1], a [0, -1, 1], and then I don't remember (modulo normalization)

right - the other one is (2, -1, -1)

*should be

and what is it actually? 😄

it's hard to say because - as you said at the start of this - I think you've done some splicing

specifically, the first row (the DC) isn't an eigenvector of either

well, it is

of both 😛

i mean it's not 'raw' - you have to add / butterfly them together

eh, they still form an eigenbasis... a bit annoying that the eigenspaces have dimension >1

this is the matrix before normalisation; look at rows 0 and 5. row 5 is the eigenvector for eiganval 0 in the 13x13 and row 0 is the sum of both 'DC's

in fact if it weren't for row 0, you wouldn't need to normalise at all (evecs of symmetric matrices)

(that's just an artefact of the algorithm for finding them 😛)

my bad, row 1 i mean

that row 1 is weird though

it doesn't *seem* like an eigenvector

that's the decoder-side matrix, right?

AFAIU that kinda looks like a "let's make the corner pixel less precise" which I can't imagine why we would do

putting it another way, since the graph matrix is symmetric but we still have to normalise the above matrix, row 1 *isn't* an eigenvector... right?

best I can tell

let me check the jxl enc and dec sources too

wait, how did you get that matrix? the one in the spec isn't all-0 on row 1

it's pre-normalisation

(I mean in the 13 part)

normalization usually doesn't get you 0s though 😛

i added the 'sensible' multiple of row 0 to get a vector that only uses the corner pixels

the matrix I posted is the same as AFV, modulo normalisation

ah, I see

but then it makes sense that you wouldn't get an eigenvec anymore, because 0 and 1 are not eigs for the same eigenvalue of the 3-part

then again, the eigvecs for eigval 0 are 0 and 5, clearly, so what the heck is the original row 1 anyway?

I think that's only valid if we're happy that row 1 is an evec to begin with - which I don't believe we are

let me do some more checking

yeah, no, that thing is not an eigenvector, is not particularly close to being an eigenvector, and the direction in which it's not an eigenvector doesn't make a lot of sense to me

right

... just for completeness, is it actually orthogonal?

no, the lack of orthogonality proves it isn't an eigenvector

sorry i wasn't clear earlier

right, thought so

because the spec says it is

so the spec lies

ah no sorry wait wait

they *are* orthogonal in the spec (AFV as given really is an orthonormal matrix, modulo questionable floating point rounding)

i'm talking about the presumed row 1 of the matrix i posted before normalisation

ah, ok

so my best guess now is that row 1 is obtained by orthonormalization of the rest of the matrix

does that make sense?

no

well, it is, but I don't think that explains it

why not?

I believe we have to choose between 1) The AFV matrix has always consisted of orthogonal vectors; that is, the effect of normalisation was only to scale each row. This makes row 1 a weird choice of basis vector with uncertain provenance, or 2) the AFV matrix did not start out orthogonal, in which case row 1 probably started off as a multiple of [3.037716, 1, 0, 0, 1, ...]. This does at least relate to only the corner pixels, but it's not an eigenvector.

My money is on 2), especially given <@532010383041363969> 's comments about "The top corner was something I and Luca haggled/hacked/kludged"

that doesn't explain how row 5 looks like it does though

it's too nice for that 3.smthing to have been randomly decided

well, now that 3.x does make sense to me at least

my money is now on "we computed the eigvecs, then tweaked them until row 1 looked a bit more (0, 0)-heavy, and then stuck with that"

What's wrong with row 5? You mean `[-0.410538 0.623549 -0.064351 -0.064351 0.623549 -0.064351 -0.064351 -0.064351 -0.064351 -0.064351 -0.064351 -0.064351 -0.064351 -0.064351 -0.064351 -0.064351]` right? I claim this was `[ 0. 0. -0.27735 -0.27735 0. -0.27735 -0.27735 -0.27735 -0.27735 -0.27735 -0.27735 -0.27735 -0.27735 -0.27735 -0.27735 -0.27735 ]` before normalisation which is just the evec for eval 0 in the 13x13

sorry, row 4