JPEG XL

Gamut mapping is just clamping when requesting uint, and returning out of range values when requesting float, iiuc

Tone mapping is done according to the tone mapping header iiuc, but <@604964375924834314> probably knows better

there is code for tone mapping in the codebase but at present, it’s not really well-integrated with the rest

djxl can use it but djxl_ng not yet

libjxl doesn’t either

can libjxl output any enum space including `JXL_PRIMARIES_CUSTOM` or is that an exception?

at least in theory it should, yes

(if XYB)

I noticed the libjxl docs say this

> The JPEG XL image has an attached ICC Profile, in that case, the encoded structured data is not available, this function will return an error status. JxlDecoderGetColorAsICCProfile should be called instead.

Is this only the case for `TARGET_ORIGINAL`?

I don't see why this would fail if you have an XYB encoded space and you call `TARGET_DATA`. I would think it would just return the color encoding you requested with `JxlDecoderSetPreferredColorEncoding`

Does this mean that if I can parse the ICC Profile for `TARGET_ORIGINAL`, and determine the primaries and white point from it, then in theory I could request the pixel data in the space described by that ICC Profile, using `JXL_PRIMARIES_CUSTOM` and then attach that original ICC Profile to that pixel data and have it render properly?

I think this might be true, but best if you try ...

In principle yes, although of course not all color profiles are representable via enum :) (also you also would need to handle the TF too)

wouldn't any white point and primaries be representable by the enums?

what else is in the ICCP other than TRC?

to be honest, what I'm trying to do here is request pixel data from the decoder in the `TARGET_ORIGINAL` space, and attach the `TARGET_ORIGINAL` iccp, which is not necessarily possible without a CMS

but if I have a full version of libjxl and not just libjxl_dec then it will have a CMS included

so I don't see why the decoder of the full version should not have this functionality

is that a post-1.0 goal or a pre-1.0 goal?

i.e. the full ersion of libjxl should export `JxlDecoderSetOriginalProfile` which sets the decoded pixel data to the data which will be accurately represented by `TARGET_ORIGINAL`'s ICC profile

or something

there could be a lot of things actually, i.e. LUT-based profiles... many (most?) ICCs are WP + PRI + TRC, but not all

some ICCs don't even *have* primaries or transfer curves

I think if we only had wp/pri/trc ICCs we'd just have implemented parsing in libjxl, but the world doesn't look that way so...

CMYK ICC profiles can be a megabyte big and god knows what they contain but it's certainly not just some primaries and transfer curve

4D LUTs 😉

I guess. After all they try to model stuff like how paper absorbs ink and what chemically happens when inks are mixed, which is a lot more hairy stuff than modeling idealized light

either way, is this something that would happen before or after 1.0?

I hope before

https://github.com/libjxl/libjxl/pull/1366

Implementing this new api should be mostly a matter of extending the final render pipeline stage to call cms on the floats just after interleaving and just before converting to the pixelformat. Shouldn't be terribly hard to do I suppose.

And I assume it makes sense to have a default cms just like in the encoder (except in libjxl_dec where you wouldn't have that, so you'd get error when trying to set output space without setting cms first)

oh, interesting, there's already a PR for this

oh, it's just for the API

If this implementation happens, would you also add SetPreferredOutputICCProfile?

so you can pass it an ICC profile, and the pixel data received will be accurately described by that ICC Profile?

either an ICC Profile for a monitor, or the TARGET_ORIGINAL profile if you're just passing it through?

So only preferred, not desired? Only transforming in case of xyb?

You could do that by inspecting uses_original_profile and only setting desired profile if it's false, I guess.

how would you set the desired profile, though?

the API proposal changes don't have a function to set a preferred/desired ICC Profile, only desired output JxlColorEncoding

No it has both

JxlDecoderSetICCProfile

I didn't see it <:DunktsukiStare:759697530786938901>

oh, it's after all the comments

*nevermind*

I assume it would fail if `uses_original_profile == true`?

or will that always work?

Will always work, if cms is set (or default cms is available, I guess)

DecodeHybridVarLenUint seems to be one of the things in jxl that I have to be really careful about overflow since it's essentially unbounded

Should fit in uint32 in practice I think. At least that's how we implement it iirc

I recall that libjxl did have some sort of bounds in the hybrid int decoder, but I'm not sure if those bounds are for trees or for individual values

(without looking at the actual code) my guess is the latter, as I've seen some bitstream where the LZ77 token starts at a very large number while only a few small values are actually used for literals

libjxl `and`s every token's nbits with 31

that's the only real bounds check there

(which is equivalent to saying that everything that doesn't fit in u32 makes things go... weird)

a valid bitstream but not properly supported by libjxl, so to say?

I'd argue that if it's valid it's because we forgot to put the limit in the profiles and levels section xD

jxl bitstream is very dense so I'm not sure that &31 is a good solution

since it still leaves a possibility for an invalid bitstream to be decoded as valid

We do say that int32 has to be enough for the buffers, maybe we should add that uint32 has to be enough for the PackSigned residuals

a very unlikely possibility, but yes

given the spec doesn't say what happens to invalid bitstreams I wouldn't mind too much xD

haha

but yeah, my journey to the jxl spec (and jxsml) gave a feeling that the format is very unforgiving

(also note that at least with ANS you have a builtin checksum, so it's not very *likely* that you'll end up decoding successfully)

ah that is a valid point

I'm yet to reach the point in the spec where ANS is frequently used (I did implement but couldn't test it for that reason)

ICC is the first place to test it

I give a 50% chance at best that all the details of histogram reading etc are actually correct in the spec xD

I've already gone through ICC (didn't do the actual decode, but enough code to fully parse them and move on, which means a full entropy code impl)

oh, very cool 😄

and I think my test image didn't have ANS codes for ICC?

maybe it is only useful for larger ICC profiles

nah, trees are not for *all* ANS streams

just for modular images

i.e. frames with kModular, and DC in VarDCT

ah, my bad, I meant codes not trees (which are not used in ANS itself)

ah, could have been a huffman

yeah

in fact it probably is most of the time

I'm also missing a nested entropy code impl (which should be not too hard at this point, but I lack a test image)

not too hard to convince cjxl to use ANS anyway

nested?

using yet another entropy code for decoding cluster maps

or context maps (I think that was a terminology used by libjxl)

ah yeah

not sure if you can actually trigger that for ICC

mhhh if you find an image with a huge ICC (or attach such an ICC to it with imagemagick), you should be able to

another interesting point: https://github.com/libjxl/libjxl/blob/main/lib/jxl/enc_icc_codec.cc#L400

if you remove that line, and make the ICC big enough, it *probably* will use ANS

is there any particular reasoning for that line?

yeah, Huffman can handle larger alphabets

and for ICC that's often useful

ah that kind of things

and I'm also noticing `LZ77Method::kOptimal` for small enough ICC profiles, heh

yeah

that LZ77 encoder is *slow*

rightly so for optimal parse

I mean, way slower than it needs to be 😛

well at least you only have exactly one chunk of compressed data unlike, say, DEFLATE :p

ah, the format is fast enough

just not enough effort in the enc impl

for the format, did you mean DEFLATE? (was a bit confused for a moment)

I mean JXL's LZ77 bitstream

ah

why dont' we just use zlib?

for that

in jxl?

well, because (a) no context modeling (b) short lengths (c) huffman

no I mean why not use zlib for lz77

since iirc it can do that

it can do it *for deflate*

and even then, it doesn't even come close to `LZ77Method::kOptimal` in terms of compression (which is closer to zopfli, both in speed and in implementation 😛)

ah

could someone add the Documentation label to https://github.com/libjxl/libjxl/pull/1421

done!

If RGBA picture is compressed using VarDCT, is the alpha channel compressed losslessly like in webp, or is VarDCT used for the alpha channel as well? Or are both options possible?

VarDCT can only operate on 3 channels

So alpha is always encoded with modular

It can be lossy or lossless

Slightly lossy alpha is done by default when doing a lossy encode

The artifacts of lossy modular are different from those of lossy VarDCT: ringing shouldn't happen (which could be weird in an alpha channel), though you can get some blur and jaggies.

Is there an option in cjxl to force lossless alpha?

Also, what would be the benefit of using lossy Modular in general instead of VarDCT? Is it good for non photographic content?

Perhaps sometimes it is.

I don't think there's currently an option in cjxl to set alpha quality independently from color quality

Would it be useful?

We don't have it in the encode api either atm. Wouldn't be very hard to do though, could even have distance settings for each extra channel separately.

I think lossy alpha might be undesirable in some cases. It would be useful to add the options. Also it seems that VarDCT and Modular use a different quality scale.

At least -d option didn't have any effect on Modular. I know I can use -q for both modes, but there I don't know what -q value is equivalent to "near lossless" -d 1 and what is the "near lossless" value for Modular (perhaps used for the lossy alpha too?).

In the current git version, -d and -q are just the same thing, and it influences both modular and vardct

-d 1 is -q 90

Alright, 90 seems to be a good mapping value. Now I can do some comparisons.

doesn't -Q (sort of by mistake) do that? or did we remove it?

We removed -Q

ah, then we need an option for alpha quality indeed

Yes, it would also be useful for other extra channels, e.g. kBlack you also probably want to control separately, etc.

How good is modular mode at compressing photographic content lossily?

not as good as vardct, but I can't tell you much more than that

modular is good for some things, but I would not put photographs in that category

Better than flif but not as good as vardct :)

well I suppose any lossy codec is better than flif at being lossy <:kek:857018203640561677>

<@179701849576833024> just to be clear, did you want bulleted lists to have *more* indentation?

and how much did you want?

I ask because I modeled it after JxlDecoderGetColorAsEncodedProfile

the whitespace, that is

is this ideal?

(yes it's a screenshot of code but I don't know how discord will wrap lines)

this seems ideal

thanks 🙂

Flif is not _that_ horrible as a lossy codec: https://flif.info/lossy.html

I mean, ignoring speed

Probably not better than jpeg, but not horribly worse either

Isn't flif lossless

or is this proprocessed, like lossy png

Yes

More effective than lossy png

But same idea

<@179701849576833024> sent an updated PR

meaning I force-pushed

and I approved 🙂

I marked the two outdated conversations as resolved

sure

merged

I thought I had automerge enabled...

it disabled when I force-pushed

yay yet another spec bug

lovely

well, great that you found it 🙂

well I've filed 20 spec issues so far, so finding another one is not a big deal 😉

but some spec bugs are particularly annoying to track down, this time alias table lookup is completely broken

(the spec says `offset = offsets[i] + pos;`, which should be a conditional move just like symbols)

I still have at least one outstanding bug I haven't yet tracked down, hopefully that's the last one before I can actually decode a whole ANS stream

the spec says that?

I mean, I can see it does

yes

but how did I manage to write it that way? xD

sleep and/or caffeine deprivation? lol

spec doesn't have any automated test, so...

more a question of too many changes over time I think 😛

ah, the C++ code does say `s.offset = offsets1_or_0 + pos`

so I can see how that would happen

just... yeah

well there are also just typos, like the alias table _initialization_ code where i and u and o are randomly swapped

🤦‍♂️

and here I thought I copied that from the C++ code

AFAIU it should be `offset = (pos >= cutoffs[i]) ? offsets[i] + pos : pos` or something like that

I think the original standard had them in prose (what I'm working with now is not one but the current community draft, as had been shared recently)

yep

so the conversion may have corrupted that

I remember spending a lot of time writing it in prose, and ISO basically saying "nah, code"

then I copypasted from C++

;_;

epic

but apparently wrong

i mean kinda yeah code can be less weird at some details but lol

Yurume is single-handedly finding more spec bugs than all the national bodies of ISO combined 😅

Honestly, are the national bodies really interested in finding bugs? 🙂

I'm kinda surprised too, didn't other ISO members proofread the document and give feedback? maybe they didn't take a look at libjxl though...

They read it but they didn't try to execute it, that kind of makes a big difference

Just for clarification, VarDCT always uses 3 channels for color images, but how does it handles grayscale images? Does it use a single channel?

it uses 3 channels, two of which are 0

(it took some pain to ensure they are *actually* 0)

A channel that is all-zeroes compresses to some small constant size regardless of dimensions, in both vardct and in modular

So we didn't really have to bother with not encoding zero chroma, it would make very little difference

(it's different in all other codecs I know, which have some nonzero fixed cost per sample so having a special case for luma-only does make sense for them)

In modular mode, grayscale is encoded with just a single channel, but that's because modular is flexible in the channelcount anyway

tbh it would make sense for us too, for memory usage 😉

Well we could special-case a chroma histogram that is a singleton and not actually allocate it in that case

Okay. I suppose the same thing happens when transcoding grayscale JPEG to VarDCT, such JPEG has only the luma channel. In modular, I guess, indexed color could be used for grayscale as well. After all, 8 bit grayscale uses small amount of "colors".

yes, probably if you pass it a grayscale image as RGB, the palette transform will make a palette and encode it like that

and otherwise YCoCg will turn it into one grayscale channel and two all-zero channels

The application can pretty easily detect a grayscale image by comparing the RGB values. After all, some image formats don't support grayscale directly and store it as indexed color.

is it true that all-gray RGB images are always R=X, G=X, B=X though?

Usually I see grayscale treated as a YUV luma plane, with U and V both zeros

but depending on the 3x3 matrix, that won't necessarily translate to Cb and Cr, will it?

shouldn't every variant of yuv make u=v=0 iff r=g=b?

I don't know

that was my question

I _think_ so, but no idea if there is some exotic yuv variant that doesn't do that. It would be weird though - then yuv400 would not be grayscale in that yuv variant...

but for all the sane ones it is that way, I see

what's gray anyway

Gray is the luma channel, contrast and brightness only.

I mean what is gray *light*

or... whatever

Gray is an intensity of light. It doesn't need to be visible light, it could be infrared as well. Or it doesn't have to be a picture at all. A grayscale image can represent any arbitrary rectangular array of values.

a grayscale image can be used for just any single-channel data (could be an alpha channel or whatever), but I'd say what makes a visual image 'grayscale' is that all the pixels have the same chromaticity as the WhitePoint

one way to think about grayscale is that the color gamut is extremely restricted to a triangle in xy that is just a dot, i.e. the R, G and B primaries all coincide and are equal to the WhitePoint itself

I guess you _could_ have a grayscale image with a WhitePoint that is actually not white at all, e.g. not even close to D65 but somewhere randomly in the xy plane. I suppose if you convert such a grayscale image to sRGB, you should get pixel values that are very much not R=G=B, but colored (but of course monochromatic)

I wonder what happens if you try something like that in libjxl. Do we do whitepoint conversion?

I believe so yes

Could be fun to make a nominally grayscale image that looks purple, could be a good test of cms being implemented properly

I'm not sure, but I think with advanced LUT trickery you could also make a 1c image that looks 2 different colors or something

(basically use the CMS to do indexed color, but continuous)

I think it would be a neat trick to add sepia tone to a plain black and white picture.

Yes with full icc you can probably do very funky stuff, I was wondering what can be done within the enum though

Might be good to add some stuff to jxl_from_tree to set custom enum spaces

Also for HDR jxl art and stuff :)

hm, just changing the whitepoint on a grayscale image doesn't seem to make it render any differently

looks like just a normal grayscale gradient to me

``` $ jxlinfo 0.jxl JPEG XL image, 1024x1024, (possibly) lossless, 10-bit Grayscale Color space: Grayscale, Custom, white_point(x=0.100000,y=0.700000), sRGB transfer function, rendering intent: Perceptual ```

<@604964375924834314> maybe I'm misunderstanding how WhitePoint is supposed to work or what it is supposed to mean. I never quite completely understood what WhitePoint is supposed to mean exactly...

AFAIU, in grayscale, it would only matter when using the “absolute colorimetric” rendering intent

perceptual and relative do chromatic adaptation from the source to the target colorspaces

but in a grayscale image, everything has the chromaticity of the white point, so…

I tried kAbsolute too, did the same thing

Maybe when I view the decoded png my viewer just didn't bother

that’s quite possible

iirc, Skia doesn’t do absolute, for example

Question is if I set it to kAbsolute with that absurdly purple whitepoint, what should it look like and how will it look in the various jxl viewers?

isn’t it green?

Or whatever it is, not white in any case

Yeah very green

In theory with kPerceptual you should be able to make sepia toned images and stuff like that?

I mean kAbsolute

yes, I think so

We should test if that works and if not, how to make it work, because that's a nifty trick

although, if you were to convert to a colorspace with a warmer white point, the image may actually look more blue

absolute colorimetric is considered kind of a niche use case

most useful when simulating printing or this sort of thing

(“soft proofing”)

So kPerceptual and kRelative map source wp to target wp, right?

But kAbsolute doesn't

What about kSaturation?

iirc, it also does

and to preserve saturation, it cares less about preserving hue

it’s also a bit niche; from what I understand, the use-case that is usually mentioned is computer graphics that must keep their “pop” at the expense of accuracy

https://www.cambridgeincolour.com/tutorials/color-space-conversion.htm is a nice article from what I recall

So if I understand correctly, kRelative and kAbsolute are the only ones that try to be 'correct', i.e. they clip out of gamut colors to the nearest in gamut color, and the difference is whether they adjust for WP too or not

kPerceptual just scales/stretches one gamut to another, so e.g. when going from wide gamut to small gamut it will desaturate

And kSaturation also stretches to fit in the target gamut, but prioritizes saturation preservation instead of hue preservation

your and my understandings coincide

Yay!

I like how you didn't say that it's correct xD

Who knows what is correct in all this mess where correctness is ultimately decided by what the popular tools happen to implement 😅

so if I make this jxl file: $ jxlinfo 0.jxl JPEG XL image, 1024x1024, (possibly) lossless, 10-bit Grayscale Color space: Grayscale, Custom, white_point(x=0.700000,y=0.100000), sRGB transfer function, rendering intent: Absolute

and djxl it to a png

then the png has some synthesized icc profile: icc©️ Copyright 2019 Google LLC, CC-BY-SA 3.0 Unported license(https://creativecommons.org/licenses/by-sa/3.0/legalcode) icc:description: Gra_0.7;0.1_Abs_SRG

but no viewer shows it in any different way than with a normal white point

if I cjxl the png to make an XYB VarDCT jxl file, it also looks like just regular grayscale

same if I make a color image with a funky whitepoint and kAbsolute rendering intent — the whitepoint seems to be just ignored

ah no, interesting

when I make an RGB image (that only contains R=G=B) with a funky white point and kAbsolute rendering intent, something is happening in some viewers

left is how photoshop shows the grayscale and rgb images, on the right is how Preview shows the rgb image

firefox and chrome just show the rgb image as grayscale though

no idea what photoshop is doing but it looks weird to me

`mpv` might display it properly once my patch to libavcodec gets merged

we'll have to see

currently I'm awaiting a different patch

can you upload this file? and is it XYB encoded?

or is it modular grayscale

it would be a good test sample anyway

I tried just modular grayscale and modular rgb

Looks like nothing I could find does white point adjustment for grayscale png images

And only Preview does white point adjustment for rgb png images (maybe some others too, didn't try a lot)

Photoshop does something but it looks buggy

My guess is that most things just ignore whitepoint and only convert the primaries, or perhaps they don't ignore it but just always map it to the target whitepoint (which is ok for all rendering intents that are in common use)

<@&807636211489177661> ban

looks like this person's from matrix bridge

Yeah, cannot ban without killing the matrix bridge

you can delete the message tho right?

idk who has authority to ban that user on the matrix side (if they haven't already been banned)

Yeah I can delete the message

ty

here's the grayscale image with funky whitepoint

and here's the rgb one (that is also grayscale in the sense that r=g=b) with the same funky whitepoint

you can't make this stuff up

this is how Finder and Preview show these two images (when converted to png)

Finder is interesting: at first it shows the image as if it doesn't have a funky whitepoint, but when I resize the Finder window that somehow triggers it to do something else

so this is the crash I'm getting on windows

```gdb (gdb) where #0 0x00007ff7c21a9d3c in operator() (__closure=0x9c2c7fc8e0, task=<optimized out>) at /opt/ct-ng/lib/gcc/x86_64-w64-mingw32/11.2.0/include/avxintrin.h:881 #1 0x00007ff7c23a7a58 in jpegxl::ThreadParallelRunner::RunRange (self=self@entry=0x29183ce9bc0, command=command@entry=2160, thread=thread@entry=3) at ../lib/threads/thread_parallel_runner_internal.cc:139 #2 0x00007ff7c23a7b3a in jpegxl::ThreadParallelRunner::ThreadFunc (self=0x29183ce9bc0, thread=3) at ../lib/threads/thread_parallel_runner_internal.cc:169 #3 0x00007ff7c304b151 in execute_native_thread_routine () #4 0x00007ff7c11b44d3 in pthread_create_wrapper (args=0x29183d1e1c0) at src/thread.c:1533 #5 0x00007ffa6a0cdfb4 in msvcrt!_beginthreadex () from /c/WINDOWS/System32/msvcrt.dll #6 0x00007ffa6a0ce08c in msvcrt!_endthreadex () from /c/WINDOWS/System32/msvcrt.dll #7 0x00007ffa6ad954e0 in KERNEL32!BaseThreadInitThunk () from /c/WINDOWS/System32/KERNEL32.DLL #8 0x00007ffa6bd8485b in ntdll!RtlUserThreadStart () from /c/WINDOWS/SYSTEM32/ntdll.dll #9 0x0000000000000000 in ?? () Backtrace stopped: previous frame inner to this frame (corrupt stack?) (gdb) frame 0 #0 0x00007ff7c21a9d3c in operator() (__closure=0x9c2c7fc8e0, task=<optimized out>) at /opt/ct-ng/lib/gcc/x86_64-w64-mingw32/11.2.0/include/avxintrin.h:881 881 in /opt/ct-ng/lib/gcc/x86_64-w64-mingw32/11.2.0/include/avxintrin.h (gdb) frame 1 #1 0x00007ff7c23a7a58 in jpegxl::ThreadParallelRunner::RunRange (self=self@entry=0x29183ce9bc0, command=command@entry=2160, thread=thread@entry=3) at ../lib/threads/thread_parallel_runner_internal.cc:139 139 self->data_func_(self->jpegxl_opaque_, task, thread); (gdb) frame 2 #2 0x00007ff7c23a7b3a in jpegxl::ThreadParallelRunner::ThreadFunc (self=0x29183ce9bc0, thread=3) at ../lib/threads/thread_parallel_runner_internal.cc:169 169 RunRange(self, command, thread); (gdb) frame 3 #3 0x00007ff7c304b151 in execute_native_thread_routine () (gdb) ```

what's weird is my cpu supports AVX so I don't know what the issue is

So one hypothesis is it's a compiler bug that somehow only gets triggered in windows builds

mingw-w64 btw

Another hypothesis is that we're using an avx2 instruction that we think does not require alignment, but it perhaps actually does require alignment, and for some reason memory is aligned when building/running in linux but not in windows

Another hypothesis is that it's a bug on our end that just happens to never trigger on any CI build but does trigger in that windows build

Another hypothesis is that there's something wrong with the build flags or something when building for windows

this one can easily be tested though, can it? by somehow requiring aligned memory

and seeing if that fixes the problem

I'm not sure how to force alligned allocs though, tbh

We have a macro for that, don't recall what it is but probably JXL_ALIGNED or HWY_ALIGNED or something

The image buffer allocs we do are already aligned

But things like local arrays or something may not be

this is a client using libjxl, if I can create a `malloc` that forces alignment, I can pass it to `libjxl_memory_manager` and then test it that way too, right?

Do you also get a crash at -e 5 but not at -e 5 --gaborish=0?

this is a libjxl client so I can't set gaborish to 0

not cjxl

There's a libjxl encode option for it too I think

Should be in any case

okay, so I checked, and apparently I set the libjxl memory manager to use `av_malloc` as its malloc function, which apparently aligns *all* blocks of memory suitable for vectorization, according to the documentation

The explicit allocs are not the issue I think, it's more things like the code saying `const float foo[32] = bla` and in one build this will put it somewhere aligned and in another it may be unaligned.

But doesn't hurt to check if changing the malloc to see if that helps

does this force proper stack alignment as well?

in addition to heap memory allocations?

Iirc we already do stuff to force alignment for image buffers where we have plenty of stuff that requires alignment. I think we just allocate more than needed and round the pointer up so it is aligned.

since the issue appears to be stack allocated memory

https://github.com/libjxl/libjxl/blob/main/lib/jxl/convolve.cc#L363 is that the spot that could be the issue, <@179701849576833024> ?

What would be the way to write that without using LoadDup but with LoadU?

It's not trivial

Easier to require alignment on the weights fields and struct/variable:)

Ah actually not

Ugh this is a bit of a pain

We *can* change the impl of loaddup though

I can't test but I think we need to urgently fix this, too many crashes reported in windows, I just saw Krita is also getting reports about it

I'll give it a look later

<@853026420792360980> can you see which of the 3 impl is selected here? https://github.com/google/highway/blob/master/hwy/ops/x86_256-inl.h#L2226

> Backtrace stopped: previous frame inner to this frame (corrupt stack?) I have to say that's not very reassuring

how would I do that?

mhhh

should I just `#error` all 3 with different error messages?

one easy way is to add a `#error "impl 1"`

yeeeep

I mean that's what I would do

What's the purpose/reason for having 3 implementations in the first place?

don't ask me 🙂

Is that clang or gcc? Usually gcc, right?

third one, the #else

at least *some* build Sami got had actually msvc apparently

or at least something looking very much like it

```c #if HWY_LOADDUP_ASM #error "impl 1" __m256 out; asm("vbroadcastf128 %1, %[reg]" : [ reg ] "=x"(out) : "m"(p[0])); return Vec256<float>{out}; #elif HWY_COMPILER_MSVC && !HWY_COMPILER_CLANG #error "impl 2" const __m128 v128 = LoadU(Full128<float>(), p).raw; return Vec256<float>{ _mm256_insertf128_ps(_mm256_castps128_ps256(v128), v128, 1)}; #else #error "impl 3" return Vec256<float>{_mm256_broadcast_ps(reinterpret_cast<const __m128*>(p))}; #endif ```

impl 3 was returned

ok, then can you do 2 things... (a) print the pointer 😄 (b) switch to impl 2

print *which* pointer

p

how?

`fprintf(stderr, "%p\n", p);` should do it

%p, I didn't know that was a thing

will do some spam possibly but only the last one before the crash matters

I always do %X or something

Maybe first worth checking if switching to impl 2 actually fixes it

How do we know LoadDup is even the culprit?

Is it because the remaining stuff in the enc gaborish convolution is unlikely to be broken because if it were, a lot more stuff would be broken too?

well, there were some stack traces failing on vbroadcastf128

which is pretty much what LoadDup *is*

Ah I see

Well, impl 1 and 3 are that

LoadDup is not in the hot loop, right?

Do we use LoadDup in any hot loop?

it loads some xyb constants and other similar things

If not, can we just not bother with speed of that initialization? There is no way it will make any difference

So if the crashing pointer happens to be unaligned, what we are dealing with then is an avx2 documentation bug, right? Since officially the instruction does not require alignment but in reality it seems like it does.

I think so yes

but it might be *very* unaligned, like not aligned to float width, which is documented as a crashing condition (and probably a compiler bug)

https://c.tenor.com/GFSbcD9PgycAAAAM/drumroll-drum.gif

how would I switch to impl 2 anyway? just remove the #else, replace the #elif foo with #else?

Yes, but first print the pointer pls

currently rebuilding

I'm curious what its last digits are...

I also had to #include cstdio but otherwise yes it worked

i.e. it compiled

Could of course be that adding the print messes things up and makes it work, lol

no, I mean it compiled

I haven't run it yet

wouldn't be the first time, and won't be the last

this is all cross-compiling inside a docker image so it's all very slow

Yeah and changing that file probably causes a near full recompile

and then FFmpeg has to recompile too

and then to actually test it I gotta reboot to my windows install

yuck

I can test it in wine but I'd prefer a native install for true diagnostic info

not the fastest build-test cycle ever lol

testing it in wine, it's spitting out *lots* of pointers

god this is slow

time to reboot and test natively

Can you do | tail or something?

``` 00007ff670510980 00007ff670510920 00007ff670510970 Thread 31 received signal SIGSEGV, Segmentation fault. [Switching to Thread 13140.0x3384] 0x00007ff66ebaa7dc in operator() (__closure=0xca469fcd90, task=<optimized out>) at /opt/ct-ng/lib/gcc/x86_64-w64-mingw32/11.2.0/include/avxintrin.h:881 881 /opt/ct-ng/lib/gcc/x86_64-w64-mingw32/11.2.0/include/avxintrin.h: No such file or directory. (gdb) where #0 0x00007ff66ebaa7dc in operator() (__closure=0xca469fcd90, task=<optimized out>) at /opt/ct-ng/lib/gcc/x86_64-w64-mingw32/11.2.0/include/avxintrin.h:881 #1 0x00007ff66edab8b8 in jpegxl::ThreadParallelRunner::RunRange (self=self@entry=0x2484eacc680, command=command@entry=2160, thread=thread@entry=1) at ../lib/threads/thread_parallel_runner_internal.cc:139 #2 0x00007ff66edab99a in jpegxl::ThreadParallelRunner::ThreadFunc (self=0x2484eacc680, thread=1) at ../lib/threads/thread_parallel_runner_internal.cc:169 #3 0x00007ff66fa53b51 in execute_native_thread_routine () #4 0x00007ff66dbb44d3 in pthread_create_wrapper (args=0x2484eaccd40) at src/thread.c:1533 #5 0x00007ffe7b23dfb4 in msvcrt!_beginthreadex () from /c/WINDOWS/System32/msvcrt.dll #6 0x00007ffe7b23e08c in msvcrt!_endthreadex () from /c/WINDOWS/System32/msvcrt.dll #7 0x00007ffe7ba954e0 in KERNEL32!BaseThreadInitThunk () from /c/WINDOWS/System32/KERNEL32.DLL #8 0x00007ffe7d06485b in ntdll!RtlUserThreadStart () from /c/WINDOWS/SYSTEM32/ntdll.dll #9 0x0000000000000000 in ?? () Backtrace stopped: previous frame inner to this frame (corrupt stack?) (gdb) ```

ran it inside gdb

lemme try with threads=1

So

70

It's not printing anything with threads=1

Means it is aligned to 16 bytes but not to 32 bytes, right?

well, threads=0

now it prints nothing

disabling threading makes it easier to hit

The previous pointers are aligned to 32 but that one only to 16

``` (gdb) where #0 0x00007ff66ebaa7dc in operator() (__closure=0x7b0f7fcda0, task=<optimized out>) at /opt/ct-ng/lib/gcc/x86_64-w64-mingw32/11.2.0/include/avxintrin.h:881 #1 0x00007ff66edab6de in jpegxl::ThreadParallelRunner::Runner (runner_opaque=0x203175792c0, jpegxl_opaque=0x7b0f7fcd90, init=<optimized out>, func=0x7ff66ebaa910 <jxl::ThreadPool::RunCallState<jxl::Status(long long unsigned int), jxl::N_AVX2::SRGBToXYB(const Image3F&, float const*, jxl::ThreadPool*, jxl::Image3F*)::<lambda(uint32_t, size_t)> >::CallDataFunc(void *, uint32_t, size_t)>, start_range=0, end_range=2160) at ../lib/threads/thread_parallel_runner_internal.cc:72 #2 0x00007ff66eba87f3 in jxl::ThreadPool::Run<jxl::Status(long long unsigned int), jxl::N_AVX2::SRGBToXYB(const Image3F&, float const*, jxl::ThreadPool*, jxl::Image3F*)::<lambda(uint32_t, size_t)> > (begin=0, init_func= @0x7ff66f7cfe30: {jxl::Status (unsigned long long)} 0x7ff66f7cfe30 <jxl::ThreadPool::NoInit(unsigned long long)>, caller=<synthetic pointer>, data_func=..., end=<optimized out>, this=0x203175bbd00) at ../lib/jxl/base/data_parallel.h:50 #3 jxl::RunOnPool<jxl::Status(long long unsigned int), jxl::N_AVX2::SRGBToXYB(const Image3F&, float const*, jxl::ThreadPool*, jxl::Image3F*)::<lambda(uint32_t, size_t)> > (begin=0, init_func=@0x7ff66f7cfe30: {jxl::Status (unsigned long long)} 0x7ff66f7cfe30 <jxl::ThreadPool::NoInit(unsigned long long)>, caller=<synthetic pointer>, data_func=..., end=<optimized out>, pool=0x203175bbd00) at ../lib/jxl/base/data_parallel.h:108 #4 jxl::N_AVX2::SRGBToXYB (srgb=..., premul_absorb=<optimized out>, premul_absorb@entry=0x7b0f7fce60, pool=pool@entry=0x203175bbd00, xyb=<optimized out>, xyb@entry=0x7b0f7fd700) at ../lib/jxl/enc_xyb.cc:195 ```

apparently that's indeed the case

there's also a way to get the local asm

one sec

if I turn off threading it prints nothing

ever

disassemble

ok that is *weird*

I mean that's a gdb command

actually wait

oh, the gdb command prints nothing

it's printing stuff fine if I disable threading

could be gdb + threads is broken

god, it's a heisenbug

well if it's really due to alignment then yep indeed

it appears it doesn't need 32-bit alignment though

``` 00007ff6705108f0 00007ff670510940 00007ff6705108e0 00007ff670510980 00007ff670510920 00007ff670510970 00007ff670510910 00007ff670510960 00007ff670510900 00007ff670510950```

this got printed

mhhhhh

Does it ever crash just after a 32 aligned pointer?

maybe we hit a stack size limit? that'd be rather odd though

Could it be that alignment is needed but it sometimes works when not aligned?

https://0x0.st/oaa9.log

does this mean anything to you?

... looks like it's not on failing on a loaddup

but on a Load()

well, a store in this case

So in the convolve loop?

no it's in SRGBToXYB now

what.

That is weird

still AVX2 though

are there any helpful options I can pass to highway to help debug this?

that looks *suspiciously* like argument passing under some calling convention

give me a sec

That is also used at e < 5

currently it's using e7

e3 produces an identical crash

In the same SRGBToXYZ function

which is not surprising, if that's where it is

I think this might be *another* problem

you mentioned that `HWY_ALIGN` can force alignment, but how I can enable that option? running libjxl's cmake with `-DHWY_ALIGN=ON` just yields a warning that it is unused

no that would be a C++ macro

can you print %rsp in gdb?

Yeah no that's something we would need to use in the code on whatever variable needs to be aligned

If this issue is not in gaborish it is something completely different

how?

print %rsp

I think

``` (gdb) print %rsp A syntax error in expression, near `%rsp'. ```

whatever

`info registers`

let's go as verbose as we can xD

``` (gdb) info registers rax 0x8a529fc710 594091689744 rbx 0x24e8b310d80 2536365952384 rcx 0x8a529fc750 594091689808 rdx 0x8a529fc730 594091689776 rsi 0x8a529fc730 594091689776 rdi 0x8a529fc750 594091689808 rbp 0x24e972efd00 0x24e972efd00 rsp 0x8a529fc650 0x8a529fc650 r8 0x0 0 r9 0x7ff66ebaa910 140696396409104 r10 0x8a529fc8b0 594091690160 r11 0x24e8d301c80 2536399445120 r12 0x24e952efc80 2536533589120 r13 0x0 0 r14 0x8a529fc8c0 594091690176 r15 0x24e8d301c80 2536399445120 rip 0x7ff66ebaa7dc 0x7ff66ebaa7dc <operator()(uint32_t, size_t) const+188> eflags 0x10246 [ PF ZF IF RF ] cs 0x33 51 ss 0x2b 43 ds 0x2b 43 es 0x2b 43 fs 0x53 83 gs 0x2b 43 (gdb) ```

well that's an interesting stack pointer

I had to run it again btw, so the virtual address space might be a bit different

should I repost the disassemble log?

nah

rsp is not aligned

that's not great

Can the stack pointer be messed up causing gdb to think it is in a different function?

looks 16-bit aligned, yea

okay, interesting. sometimes, it crashes immediately, and sometimes it starts spitting out the printf %ps

I'm wondering if it's random based on if the pointer happens to be aligned or not

almost certain

Then I go back to the assumption that unaligned LoadDup is just causing random trouble

I'm going to bet it depends on whether rsp is 16-bit aligned or 32-bit aligned

is there a way to *force* it to be 32-bit aligned, perhaps via compile flag?

trying to figure it out

I could try using `-mstack-align` or `-mdata-align` if that is necessary

I don't know if those work on x86 though

what compiler are you using for this build?

mingw-w64

which would be gcc

I guess it would be useful to try if switching to impl 2 makes it work, to verify that that is indeed the issue

no, this is unrelated

this is a compiler-generated register spill / parameter passing

one shouldn't do an unaligned store to 0xe0(%rsp) if rsp is 16-bit aligned 🙂