|
_wb_
|
2021-11-20 04:32:52
|
That's when you know the output buffer contains the decoded image
|
|
2021-11-20 04:33:48
|
(or a frame of an animation)
|
|
|
|
Vincent Torri
|
2021-11-20 04:34:51
|
and what should I do in that event ?
|
|
|
|
_wb_
|
2021-11-20 04:39:32
|
Paint, I guess? Or copy the out buffer to some frame storage? I dunno how your toolkit works...
|
|
|
|
Vincent Torri
|
2021-11-20 04:41:39
|
i have a BGRA buffer, i just need to fill it with the decoded image
|
|
|
|
_wb_
|
2021-11-20 04:46:04
|
You have it when that event occurs
|
|
|
|
Vincent Torri
|
2021-11-20 04:50:13
|
hmm, also, i need to think how i can get the frame #n
|
|
2021-11-20 04:51:23
|
i have to count the number of times the event JXL_DEC_FULL_IMAGE is called, i guess
|
|
2021-11-20 04:58:58
|
need to do more tests
|
|
|
|
Hello71
|
2021-11-20 06:20:19
|
arch packagers aren't that dumb. java is makedepend, but optdepend at runtime. if you want to build (not install) libjxl on arch, then you will need java, unless you edit the pkgbuild
|
|
|
|
Vincent Torri
|
2021-11-20 06:32:07
|
<@!765599758559215616> i'll tell the user, thanks
|
|
2021-11-20 06:32:16
|
users*
|
|
2021-11-20 09:02:48
|
<@!794205442175402004> i have decoded a jxlimga, but the colors are not correct. I have not dealt with JXL_DEC_COLOR_ENCODING event
|
|
2021-11-20 09:03:06
|
<@!794205442175402004> would it be the reason for this problem ?
|
|
|
|
_wb_
|
2021-11-20 10:56:44
|
Yes, you have to get the icc profile and take it into account (probably your toolkit already has something for that, it's the same with png or jpeg)
|
|
|
|
Vincent Torri
|
2021-11-21 12:33:59
|
<@!794205442175402004> libjxl output imge only in rgba ? (and not in bgra)
|
|
|
|
_wb_
|
2021-11-21 12:36:46
|
At the moment, yes. We might add bgra at some point. In any case, I think with most color management systems you can have e.g. input in rgba floats and output in bgra uint8
|
|
|
|
Vincent Torri
|
2021-11-21 02:56:25
|
<@!794205442175402004> for libjxl, RGBA means R highest byte and A lowest byte, right?
|
|
|
|
_wb_
|
2021-11-21 02:59:57
|
Big endian R is highest byte, yes
|
|
2021-11-21 03:00:14
|
Little endian R would be lowest byte
|
|
2021-11-21 03:00:48
|
Uint8 RGBA is just R, G, B, A bytes in that order
|
|
2021-11-21 03:02:09
|
So if you interpret a pixel as uint32, on nearly any system things are little-endian, so it would be A | (B << 8) | (G << 16) | (R << 24)
|
|
|
|
Vincent Torri
|
2021-11-21 03:03:22
|
thanks
|
|
2021-11-21 03:07:41
|
ok, it works
|
|
2021-11-21 03:47:50
|
doessomeone see a reason why JxlDecoderProcessInput() would segfault ?
|
|
|
|
_wb_
|
2021-11-21 03:49:21
|
It's a bug if that happens, maybe use a debug build of the library to see if it's hitting some assert
|
|
|
|
Vincent Torri
|
2021-11-21 03:52:43
|
i'll compile libjxl with debug symbols for a bactrace
|
|
2021-11-21 03:55:51
|
i also have warnings when I compile libjxl
|
|
2021-11-21 03:56:19
|
" may be used uninitialized" warnings
|
|
|
|
_wb_
|
2021-11-21 03:57:47
|
That is strange, we should compile with warnings turned into errors so there shouldn't be any warnings
|
|
|
|
Vincent Torri
|
2021-11-21 04:00:28
|
do I open an issue ?
|
|
2021-11-21 04:02:52
|
for the warnings
|
|
|
|
_wb_
|
|
|
Vincent Torri
|
2021-11-21 06:05:22
|
done
|
|
2021-11-22 01:29:54
|
when I see some explicit call of delete in libjxl, i'm wondering why libjxl is not written in C
|
|
2021-11-22 01:30:17
|
and in C it would be around 4% faster
|
|
|
|
_wb_
|
2021-11-22 02:27:47
|
4% faster why?
|
|
|
|
Vincent Torri
|
2021-11-22 03:21:49
|
because of all the C++ machinery (exception, etc...
|
|
|
|
_wb_
|
2021-11-22 03:41:18
|
yes but we don't use things that come at a perf price, I think
|
|
|
|
Vincent Torri
|
2021-11-22 07:07:16
|
and you use more memory than necessary
|
|
2021-11-22 07:07:22
|
like using sdt::vector
|
|
|
|
_wb_
|
2021-11-22 07:14:25
|
Well yes, but the difference will be quite minimal. It's rare that things have a fixed size and can be done without the overhead of having an explicit size.
|
|
2021-11-22 07:16:11
|
What's the overhead of std::vector compared to a manually allocated array with implicit size? 8 bytes?
|
|
2021-11-22 07:19:43
|
I guess it's more if you don't reserve the right capacity and rely on the default capacity growth, which could be a bit wasteful
|
|
2021-11-22 07:20:20
|
There might be some spots where we could trim some memory overhead
|
|
2021-11-22 07:21:06
|
But imo that's not a C vs C++ thing, that's just generic data structures vs doing something more specific
|
|
|
|
Vincent Torri
|
2021-11-22 07:21:30
|
my opinionis that such a library should be as fast as possible and consume the less memory possible
|
|
2021-11-22 07:21:45
|
that means, imho, C code + hand-written asm
|
|
2021-11-22 07:21:50
|
it's possible
|
|
2021-11-22 07:22:11
|
like libjpeg-turbo or dav1d
|
|
2021-11-22 07:22:23
|
they are quite efficient
|
|
|
|
_wb_
|
2021-11-22 07:23:57
|
Well I think it should also be as portable as possible and security/robustness is also a thing
|
|
2021-11-22 07:24:18
|
Hand-written asm is not the greatest thing to do a whole codec in
|
|
2021-11-22 07:24:36
|
It's ok for very hot loops on popular architectures
|
|
|
|
Vincent Torri
|
2021-11-22 07:25:24
|
C is portable, and it's common known that stl could bring incompatibilities
|
|
|
|
_wb_
|
2021-11-22 07:26:07
|
But we also want to do something reasonable on less popular architectures; I think highway (a simd abstraction layer) is a better approach than fully hand-written asm
|
|
2021-11-22 07:26:27
|
Yes C would be fine, just a bit trickier to avoid memleaks etc
|
|
|
|
Vincent Torri
|
2021-11-22 07:26:40
|
valgrind helps you for that
|
|
2021-11-22 07:27:20
|
asan too
|
|
|
|
_wb_
|
2021-11-22 07:27:24
|
Anyway it might at some point happen that someone makes a C, or Rust, or whatever reimplementation
|
|
2021-11-22 07:28:04
|
For now, C++ is a good trade-off between speed and dev comfort, imo
|
|
|
|
Vincent Torri
|
2021-11-22 07:28:28
|
i agree that c++ can help writing for faster
|
|
2021-11-22 07:30:31
|
-for
|
|
|
|
_wb_
|
2021-11-22 07:30:46
|
Having some slightly suboptimal implementation in totally non-hot code is ok, imo. But I agree, we always have to try to make things as fast and memory-optimized as possible
|
|
|
|
Vincent Torri
|
2021-11-22 07:32:43
|
icame for a project where we care a lot about optimisations and small memory footprint
|
|
2021-11-22 07:33:28
|
and we might use libjxl in replacement of libjpeg if it is better
|
|
|
|
Deleted User
|
2021-11-22 09:44:26
|
Another thing is that libjxl is a reference implementation, so it's supposed to be quite easy to understand and maintain.
|
|
|
|
Vincent Torri
|
2021-11-23 08:39:35
|
you can have C code fallback + asm code too. C fallback is easy to understand, and asm is for more speed
|
|
|
w
|
2021-11-23 01:36:39
|
what's the plan for a shared library jxl_enc?
|
|
|
|
_wb_
|
2021-11-23 04:03:24
|
encoder only?
|
|
2021-11-23 04:03:53
|
is there a use case for that? would barely save any binary size compared to a full libjxl (dec+enc)
|
|
|
|
Vincent Torri
|
2021-11-23 07:55:37
|
you would have less problems with meson instead of cmake
|
|
2021-11-23 07:55:45
|
less build problems
|
|
|
|
_wb_
|
2021-11-23 08:07:31
|
Too many build systems nowadays
|
|
2021-11-23 08:08:01
|
It used to be that everything was autoconf/automake
|
|
|
|
Vincent Torri
|
2021-11-23 08:16:08
|
and before it was imake
|
|
2021-11-23 08:16:25
|
autoconf/automake/libtool is too slow
|
|
2021-11-23 08:16:52
|
too many languages (bash, m4, autotools)
|
|
2021-11-23 08:17:03
|
meson is fast and simple
|
|
2021-11-23 08:17:15
|
ho btw,new bug filed
|
|
|
|
_wb_
|
2021-11-23 08:52:08
|
Oh, a bug in premultiplied alpha decoding?
|
|
|
|
Vincent Torri
|
2021-11-23 09:00:41
|
it seems so
|
|
2021-11-23 09:00:51
|
honestly, i have no idea what to do
|
|
2021-11-23 09:01:02
|
maybe my code is wrong somewhere
|
|
2021-11-23 09:01:15
|
i can disable multi threading
|
|
2021-11-23 09:01:27
|
shoot ideas in the issue and i'll try
|
|
|
|
_wb_
|
2021-11-23 09:05:37
|
It's probably our bug
|
|
2021-11-23 09:06:13
|
Premultiplied alpha is not something the encoder does by default, and it's probably not well tested
|
|
2021-11-23 09:07:04
|
Maybe try if it still happens if you ask for output in 16-bit or anything other than uint8
|
|
2021-11-23 09:07:33
|
Could be that the bug is related to the special code path for uint8 output
|
|
|
|
Vincent Torri
|
2021-11-23 09:12:04
|
ok
|
|
2021-11-23 09:13:35
|
but gimp seems to open it correctly
|
|
|
|
w
|
2021-11-23 11:13:45
|
for the shared libraries
> # Shared library include path contains only the "include/" paths.
how do i include the sources under libjxl/lib?
jxl_enc-obj is fine but jxl_dec-obj gives me
> /usr/bin/ld: libjxl/lib/CMakeFiles/jxl_dec-obj.dir/jxl/quant_weights.cc.o: relocation R_X86_64_PC32 against symbol '_ZN3jxl15DequantMatrices14required_size_E' can not be used when making a shared object; recompile with -fPIC
|
|
2021-11-23 11:15:19
|
cmake hurts my head
|
|
|
|
_wb_
|
2021-11-24 06:24:11
|
I don't get what you're trying to do. Have a libjxl_enc.so and libjxl_dec.so?
|
|
|
|
w
|
2021-11-24 06:26:38
|
im trying to create a shared library using includes used in jxl_from_tree
|
|
|
|
_wb_
|
2021-11-24 06:29:26
|
What is wrong with the normal libjxl.so?
|
|
|
|
w
|
2021-11-24 06:30:26
|
im a noob at cmake, i assume that is just the target named 'jxl' right?
|
|
|
|
_wb_
|
2021-11-24 06:35:25
|
Uh I guess so. It includes the whole api, encoder and decoder.
|
|
2021-11-24 06:38:53
|
Libjxl_dec is not meant to be distributed as a shared library, it's intended for statically linked apps that only need the decoder, like browsers or maybe games etc.
|
|
|
|
w
|
2021-11-24 06:41:26
|
oh that makes sense
|
|
2021-11-24 06:46:35
|
the problem i have now is that when linking to jxl, it doesnt contain sources so i get an error such as `undefined symbol: _ZTIN3jxl24DefaultEncoderHeuristicsE`
|
|
|
|
_wb_
|
2021-11-24 06:53:01
|
Oh, you mean jxl_from_tree using only symbols from libjxl?
|
|
2021-11-24 06:53:47
|
That seems tricky, it's a hacky encoder that uses way deeper trickery than what we intend to expose in the encode api
|
|
2021-11-24 06:55:16
|
We're still working on migrating cjxl/djxl to the api
|
|
2021-11-24 06:55:47
|
jxl_from_tree is probably not going to happen, it doesn't really make sense
|
|
|
|
w
|
2021-11-24 06:55:56
|
<:sadge:860304344771461130>
|
|
|
|
_wb_
|
2021-11-24 06:56:56
|
It just needs to know too much about internals and it doesn't seem worth the effort to expose all that in an encode api
|
|
2021-11-24 06:57:55
|
(in terms of packaging, it would also be not the kind of tool you would include in the default install, it is pretty niche)
|
|
|
|
Vincent Torri
|
2021-11-25 10:18:21
|
<@!794205442175402004> so my segfault is because of highway
|
|
2021-11-25 03:57:13
|
<@!794205442175402004> for animated jxl files, if i want the image #n, i have to launch the infinite loop, and wait for JXL_DEC_FRAME, select frame #n, and wait JXL_DEC_FULL_IMAGE, right ?
|
|
2021-11-25 03:57:20
|
or is there another solution ?
|
|
2021-11-25 03:58:26
|
more precisely, in a timer callback, i have to display the next frame
|
|
|
|
_wb_
|
2021-11-25 04:06:53
|
when displaying a looped animation, you probably don't want to re-decode each frame every time, but instead cache decoded frames until memory becomes a problem
|
|
|
|
Vincent Torri
|
2021-11-25 06:47:38
|
<@!794205442175402004> ok, thanks
|
|
2021-11-27 12:42:52
|
<@!794205442175402004> i cache the the decoded frame in the JXL_DEC_FULL_IMAGE event, right ?
|
|
|
|
_wb_
|
2021-11-27 12:51:25
|
Yes, that is when you have the pixels in the buffer you gave it
|
|
|
|
Vincent Torri
|
2021-11-27 12:54:39
|
is that what the gdk plugin does ?
|
|
|
|
_wb_
|
2021-11-27 01:53:07
|
I suppose it does, or probably it passes the decoded pixels and timing info to something in gdk that stores the frames and does the playback thing
|
|
|
|
Vincent Torri
|
2021-11-27 05:53:39
|
there is something that i do not understand in the build
|
|
2021-11-27 05:53:53
|
you are checking for pthread, but why ?
|
|
2021-11-27 05:54:05
|
why not using c++11 std::thread ?
|
|
2021-11-27 05:56:15
|
if you are writing a code in c++, use c++ features
|
|
2021-11-29 08:17:43
|
not a lot of work in libjxl these days
|
|
|
|
_wb_
|
2021-11-29 08:50:01
|
that's called "the weekend" 🙂
|
|
|
|
Vincent Torri
|
2021-11-29 08:54:11
|
haha
|
|
2021-11-29 08:54:39
|
just a question : would the devs agree with another directory structure, like :
|
|
2021-11-29 08:54:42
|
src/
|
|
2021-11-29 08:54:46
|
bin/
|
|
2021-11-29 08:54:56
|
cjxl/
|
|
2021-11-29 08:55:17
|
arg, space not taken into account
|
|
2021-11-29 08:55:27
|
src/bin/cjxl
|
|
2021-11-29 08:55:39
|
src/bin.djxl
|
|
2021-11-29 08:55:48
|
src/lib
|
|
2021-11-29 08:55:52
|
src/tests
|
|
2021-11-29 08:55:55
|
etc...
|
|
2021-11-29 08:56:16
|
data/ for data files
|
|
2021-11-29 08:57:08
|
src/include if you want header files appart from source files
|
|
2021-11-29 10:11:01
|
about animated files :
|
|
2021-11-29 10:11:35
|
do you consider I and P frames, or is just a concatenation of images ?
|
|
2021-11-29 10:12:01
|
(like mjpeg in the cocatenation case)
|
|
|
|
w
|
2021-11-29 10:12:31
|
i think it works like mjpeg right now
|
|
2021-11-29 10:17:45
|
actually, in jxl, can't frames be delta encoded if the original images dont have alpha?
|
|
|
|
_wb_
|
2021-11-29 10:40:55
|
they can be delta encoded in any case
|
|
2021-11-29 10:41:26
|
there's a blendmode for the color image, and each extra channel (including alpha) has its own blendmode
|
|
2021-11-29 10:41:59
|
so you can do kAdd for the color image (so subtract the previous frame) and do e.g. kReplace for the alpha channel
|
|
|
|
w
|
2021-11-29 10:44:58
|
that can't take away color right?
|
|
2021-11-29 10:45:07
|
i assume it can only work well with kblend
|
|
|
|
_wb_
|
2021-11-29 10:45:57
|
how do you mean take away color?
|
|
|
|
w
|
2021-11-29 10:46:05
|
like make it darker
|
|
|
|
_wb_
|
2021-11-29 10:46:16
|
sure it can
|
|
2021-11-29 10:46:28
|
the 'image' you're encoding just has negative values too
|
|
|
|
w
|
2021-11-29 10:46:38
|
oh, didnt know they can be negative
|
|
|
|
_wb_
|
2021-11-29 10:47:23
|
they already are for most channels anyway, luma is positive but X and B go negative too
|
|
|
|
Vincent Torri
|
2021-11-29 11:10:23
|
ok, so it's annoying
|
|
2021-11-29 11:10:44
|
being compressed like that is nice but it annoys me 🙂
|
|
|
|
w
|
2021-11-29 11:15:00
|
well if you want a video format you would use a video format
|
|
|
|
Vincent Torri
|
2021-11-29 11:24:36
|
<@!794205442175402004> so even if i parse the jxl file and go to frame #n, i could need frames #m with m < n, right ?
|
|
|
|
_wb_
|
2021-11-29 11:25:23
|
yes, this is why we have JxlDecoderRewind and JxlDecoderSkipFrame, because it's not so trivial
|
|
2021-11-29 11:26:35
|
if you use those functions to go to frame #n, it will decode only the frames that are needed to reconstruct frame n and following frames
|
|
|
|
Vincent Torri
|
2021-11-29 11:48:36
|
ok
|
|
2021-11-29 11:48:46
|
it's some kind of optimisation
|
|
2021-11-29 11:50:14
|
for frame #n , i should use JxlDecoderSkipFrames(dec, n-1) ?
|
|
2021-11-29 11:50:23
|
or JxlDecoderSkipFrames(dec, n) ?
|
|
|
|
_wb_
|
2021-11-29 12:42:48
|
Uh, if you count frames from 0, then skipframes(n) means the next frame that will be returned is frame n
|
|
|
thedonthe
|
2021-11-29 03:26:14
|
got an approval on https://github.com/libjxl/libjxl/pull/898 but one of the checks is failing. is it possible to rerun them or is the tree red?
|
|
|
|
_wb_
|
|
got an approval on https://github.com/libjxl/libjxl/pull/898 but one of the checks is failing. is it possible to rerun them or is the tree red?
|
|
2021-11-29 06:08:12
|
Rerunning will not work, it will fail again. You need to rebase to current main and force-push to get the CI to succeed
|
|
|
|
thedonthe
|
2021-11-29 06:10:14
|
k sounds good will do
|
|
|
|
Vincent Torri
|
2021-11-30 11:04:42
|
is it easy to add a BGRA (B lowest byte) output in libjxl ?
|
|
2021-12-01 08:17:30
|
i would like to contribute such code if i can
|
|
|
|
_wb_
|
2021-12-01 09:44:07
|
I remember we were discussing at some point how to add that in a reasonably elegant / bw compatible way
|
|
2021-12-01 09:45:13
|
it's of course easy enough to return pixels in any interleaving order, we have to convert from our internal planar representation anyway so in this case you'd just have to swap the row pointers for B and R and you're done
|
|
2021-12-01 09:45:56
|
I think we first need to know what other stuff people want, so we can make sure the api is generic enough
|
|
2021-12-01 09:46:45
|
RGBX was another one I think someone was requesting (so no alpha but still use 4 bytes per pixel, with don't care values for alpha)
|
|
|
|
Vincent Torri
|
2021-12-01 10:55:38
|
imlib2 and the EFL need BGRA (B lowest)
|
|
2021-12-01 10:57:13
|
if you support RGBA, RGBX and BGRA, you will cover, I think, most usages
|
|
2021-12-01 11:07:18
|
gdk seems to be RGBA or RGB (24 bits), according to the plugin
|
|
2021-12-01 12:04:27
|
<@!794205442175402004> st = JxlDecoderSubscribeEvents(loader->decoder, JXL_DEC_FULL_IMAGE);
|
|
2021-12-01 12:04:33
|
is it valid ?
|
|
2021-12-01 12:04:48
|
or must I at least provide another flag ?
|
|
|
|
_wb_
|
2021-12-01 01:25:51
|
I think that's fine, you should be able to subscribe to just that if you only want to get the final pixels
|
|
|
|
Vincent Torri
|
2021-12-01 04:02:52
|
<@!794205442175402004> can you tell me why an infinite loop is needed ?
|
|
2021-12-01 04:03:08
|
<@!794205442175402004> why couldn't i call the functions one after the others ?
|
|
|
|
_wb_
|
2021-12-01 04:16:26
|
if you pass it the whole file at once, and you don't care about animation, then you indeed don't need a loop, you can just call the functions
|
|
|
|
Vincent Torri
|
2021-12-01 04:25:30
|
ok
|
|
2021-12-01 04:25:47
|
i indeed pass the whole file at once
|
|
2021-12-01 04:25:57
|
about animations, i'll see that later
|
|
2021-12-01 06:48:37
|
<@!794205442175402004> in that case, should I call JxlDecoderProcessInput ?
|
|
2021-12-01 06:48:48
|
i would say no
|
|
|
|
_wb_
|
2021-12-01 06:49:20
|
if you don't process the input, nothing happens, I think?
|
|
|
|
Vincent Torri
|
2021-12-02 10:51:22
|
<@!794205442175402004> is libjxl thread safe ? (note that i don't use a runner)
|
|
|
|
_wb_
|
2021-12-02 10:52:17
|
it should be, yes
|
|
|
|
Vincent Torri
|
2021-12-02 11:08:40
|
and if i don't use a runner, are threads still used in libjxl ?
|
|
|
|
_wb_
|
2021-12-02 11:11:40
|
I don't think so - it will just do everything in the main thread then
|
|
|
Dirk
|
2021-12-02 09:52:14
|
Are there plans to improve error reporting for the libjxl c-api ? The `JXL_API_ERROR` approach does not really work well because it is printing to `stderr`.
|
|
|
|
_wb_
|
2021-12-02 10:46:07
|
I proposed something for the encode side: https://github.com/libjxl/libjxl/pull/823
|
|
2021-12-02 10:46:36
|
For the decode side I guess we could have something similar
|
|
|
|
Vincent Torri
|
2021-12-03 12:56:13
|
is the parallel runner also useful with JXL_DEC_BASIC_INFO and JXL_DEC_FRAME
|
|
2021-12-03 12:56:15
|
?
|
|
|
|
_wb_
|
2021-12-03 04:07:00
|
Nah
|
|
2021-12-03 04:07:27
|
Only if you're doing actual decoding
|
|
|
|
Vincent Torri
|
2021-12-03 05:40:20
|
<@!794205442175402004> with JXL_DEC_FULL_IMAGE ?
|
|
|
|
_wb_
|
|
|
Vincent Torri
|
2021-12-03 05:42:14
|
also, is the saver working correctly ?
|
|
2021-12-06 07:48:10
|
<@!794205442175402004> about conformance/testcases/animation_spline/input.jxl, i have the following values for animation :
|
|
2021-12-06 07:48:38
|
duration : 1347691232
|
|
2021-12-06 07:48:43
|
tps num : 100
|
|
2021-12-06 07:48:51
|
tps den : 1
|
|
2021-12-06 07:49:21
|
how with this can i get the time spent between 2 frames (in s.) ??
|
|
2021-12-06 07:49:36
|
the duration value seems random
|
|
|
|
veluca
|
2021-12-06 08:49:53
|
that sounds like it might be a bug in the API
|
|
|
|
Vincent Torri
|
2021-12-06 10:36:04
|
actually, no bug in the jxl code
|
|
2021-12-06 10:36:14
|
there was an unset var in my side
|
|
2021-12-06 10:36:23
|
now i get 0.2s
|
|
2021-12-06 10:36:35
|
as duration
|
|
2021-12-06 10:36:46
|
it seems correct
|
|
|
|
Dirk
|
|
_wb_
I proposed something for the encode side: https://github.com/libjxl/libjxl/pull/823
|
|
2021-12-06 10:50:32
|
Looks nice, we probably want something similar in the decoder.
|
|
|
|
Vincent Torri
|
2021-12-07 10:21:46
|
i now have my jxl animated file in the EFL toolkit 🙂
|
|
2021-12-07 10:23:26
|
not perfect though
|
|
2021-12-07 10:24:03
|
<@!794205442175402004> where can I show my code for a review ?
|
|
|
|
_wb_
|
2021-12-07 10:51:07
|
I dunno, here?
|
|
|
|
Deleted User
|
2021-12-08 01:09:21
|
*Please* remember to write
\`\`\`c
as a top row above your code and
\`\`\`
below it in order to enable C syntax highlighting (since EFL uses C) to make reading your code easier for everyone 🙂
|
|
|
|
vtorri
|
2021-12-08 08:18:55
|
if I call several times skipframes(d,1), i iterate over the frames one after the other,right ?
|
|
|
|
_wb_
|
2021-12-08 10:54:00
|
iterating over the frames happens automatically, after a JXL_DEC_FULL_IMAGE event you'll just get another JXL_DEC_FULL_IMAGE event for the next frame if you keep processing the input
|
|
2021-12-08 10:54:33
|
if you're going to call skipframes(d,1) between frames, then you'll only see the even-numbered frames because it will skip all the odd-numbered ones
|
|
|
|
Vincent Torri
|
2021-12-08 12:25:02
|
<@!794205442175402004> and once the last frame is reached, should I call rewind to get back to the beginning ?
|
|
|
|
_wb_
|
2021-12-08 02:34:10
|
not if you just want to decode all frames
|
|
2021-12-08 02:36:09
|
rewind is useful if you're skipping frames during playback (because decode is too slow to do realtime playback), and then during the second time the looping animation is played, you want to fill in the gaps
|
|
2021-12-08 02:37:14
|
or if you need to free the memory of the decoded frames, so you need to decode them again
|
|
|
|
Vincent Torri
|
2021-12-08 03:06:55
|
<@!794205442175402004> so suppose that i want to play several times the same animation, how do I start again at the beginning ?
|
|
2021-12-08 07:11:21
|
no idea ?
|
|
|
|
_wb_
|
2021-12-08 07:47:02
|
Normally you would keep the frames in memory so you can loop forever without spending much cpu on decode
|
|
2021-12-08 07:48:07
|
But if you don't have enough memory for that (if it's a high res animation with lots of frames), then you call rewind and just start decoding again
|
|
|
|
vtorri
|
2021-12-08 07:54:51
|
so rewind + FULL event
|
|
2021-12-08 07:55:24
|
i have written the loader and the animation is smooth
|
|
2021-12-08 07:55:33
|
dell xps 13 4K
|
|
2021-12-08 07:55:50
|
spline jxl in conformance test
|
|
2021-12-08 08:03:40
|
<@!794205442175402004> after a rewind, should i do something again ? like settings the pixels, or .. ?
|
|
|
|
_wb_
|
2021-12-08 08:08:02
|
After rewind you basically have to do everything again, exactly like the first time
|
|
|
|
vtorri
|
2021-12-08 08:36:28
|
ok, thank you
|
|
|
|
Vincent Torri
|
2021-12-09 08:14:55
|
<@!794205442175402004> is there a document with the full spec of jpeg xl ?
|
|
|
|
_wb_
|
|
|
Vincent Torri
|
2021-12-09 08:16:36
|
is it freely available ?
|
|
|
|
_wb_
|
2021-12-09 08:16:44
|
Alas no
|
|
|
|
Vincent Torri
|
2021-12-09 08:16:47
|
ok
|
|
2021-12-09 08:16:56
|
that's why i didn't find it
|
|
|
|
_wb_
|
2021-12-09 08:19:21
|
It's very frustrating, I am one of the authors of that spec, but I cannot freely share it because ISO has the copyright
|
|
2021-12-09 08:20:15
|
(I 'voluntarily' gave ISO the copyright because that's a condition if you want to be an editor of an ISO standard)
|
|
|
|
Vincent Torri
|
2021-12-09 08:21:23
|
sure, i understand, i know all those stuff
|
|
2021-12-09 08:21:44
|
there's no problem
|
|
|
|
_wb_
|
2021-12-09 08:22:14
|
See also: https://docs.google.com/document/d/12Gmy2s4Nmkw6VDv2B6b5K1DLYhPrTUqSntrlmYzJpNw/edit?usp=drivesdk
|
|
|
|
Vincent Torri
|
2021-12-09 08:26:24
|
thanks for the link
|
|
|
improver
|
2021-12-09 09:22:34
|
there was fdis on z-library tho iirc
|
|
|
|
Vincent Torri
|
2021-12-09 09:27:23
|
conformance/testcases/animation_spline_5/input.jxl :
|
|
2021-12-09 09:27:32
|
./lib/jxl/decode.cc:2086: invalid signature
|
|
2021-12-09 09:32:26
|
does someone have also that message with that jxl file ?
|
|
2021-12-11 06:42:45
|
<@!794205442175402004> do i open a request for a bgra output ?
|
|
|
|
_wb_
|
2021-12-11 07:05:22
|
Sure, either an issue or a PR is fine :)
|
|
|
|
Vincent Torri
|
2021-12-11 07:28:33
|
<@!794205442175402004> PR would mean i know where to start 🙂
|
|
2021-12-11 07:28:53
|
+ a correct API to select color spaces
|
|
|
|
_wb_
|
2021-12-11 07:44:28
|
The imagemagick way is actually not that crazy, you pass a string like "RGBA" or "BGRA" and you get pixels interleaved in that way
|
|
2021-12-11 07:46:53
|
We could have that, and then if you want to get planar data, you just request "R", "G" and "B" separately.
|
|
2021-12-11 07:48:05
|
We can have K for kBlack, D for kDepth, T for kThermal, etc
|
|
2021-12-11 07:48:27
|
X for don't care/ignore
|
|
|
|
Vincent Torri
|
2021-12-11 07:50:28
|
and where is the RGBA data built in the code ?
|
|
|
|
_wb_
|
2021-12-11 07:55:11
|
https://github.com/libjxl/libjxl/blob/main/lib/jxl/decode.cc#L1034
|
|
|
|
Vincent Torri
|
2021-12-11 08:00:37
|
<@!794205442175402004> i think it's more there :
|
|
2021-12-11 08:00:38
|
https://libjxl.readthedocs.io/en/latest/api_common.html#_CPPv414JxlPixelFormat
|
|
2021-12-11 08:00:50
|
btw, it's even mentioned in the doc
|
|
2021-12-11 08:01:07
|
"TODO(lode): support different channel orders if needed (RGB, BGR, …) "
|
|
2021-12-11 08:02:34
|
I have currently written C code to convert RGBA to BGRA, and borrow a SSE3 code to do it faster
|
|
2021-12-11 08:03:06
|
but with a data in a BGRA colorspace would be faster
|
|
2021-12-11 08:04:51
|
and you should drop highway
|
|
2021-12-11 08:05:25
|
have a proper C fallback code + ASM (SSE3/4 or AVX(2))
|
|
|
|
_wb_
|
2021-12-11 08:16:37
|
<@179701849576833024> I suppose your recent SIMD'd interleaving code could help to do the 2- and 4-channel cases, right?
|
|
2021-12-11 08:18:03
|
I think we could special-case the case where you want to get 4 channels, and do the rest with generic slow code.
|
|
2021-12-11 08:18:25
|
It doesn't matter much which 4 channels and in what order, since we do everything planar internally anyway
|
|
|
|
veluca
|
2021-12-11 08:19:06
|
ehhh that code only works with floats though
|
|
2021-12-11 08:19:17
|
or 32-bit things
|
|
|
|
_wb_
|
2021-12-11 08:20:40
|
Yes if you want uint16 or uint8 then I guess it makes more sense to do the narrowing conversion before the interleave, or does that make no difference?
|
|
|
|
veluca
|
2021-12-11 08:20:53
|
it definitely makes a difference
|
|
2021-12-11 08:21:11
|
I'm not 100% sure the code wouldn't be trivial to adapt to other width
|
|
2021-12-11 08:21:29
|
assuming InterleaveLower/Upper does the correct thing, it should work
|
|
|
|
_wb_
|
2021-12-11 08:23:43
|
What about interleaving 3 or 5?
|
|
2021-12-11 08:23:56
|
Like RGB or CMYKA
|
|
2021-12-11 08:24:05
|
Or RGBDT
|
|
|
|
veluca
|
2021-12-11 08:24:08
|
eh
|
|
2021-12-11 08:24:12
|
that's harder
|
|
|
|
_wb_
|
2021-12-11 08:25:21
|
CMYKA is the most channels I've seen actually used in the wild (for non-niche use case like satelite imagery)
|
|
|
|
veluca
|
2021-12-11 08:26:05
|
on arm, up to 4 is trivial (`vst{2,3,4}_s{8,16,32}` and friends exist and do the right thing)
|
|
|
|
_wb_
|
2021-12-11 08:26:25
|
With depth sensors and maybe thermal/infrared sensors becoming more mainstream, RGBADT could become useful
|
|
|
|
veluca
|
2021-12-11 08:26:48
|
on x86, glhf figuring out how to non-pow2 shifts -- I think hwy implements 3 for all archs we care about, but 5... yeah...
|
|
|
|
_wb_
|
2021-12-11 08:27:07
|
Anyway probably up to 4 is fine
|
|
2021-12-11 08:27:27
|
If it's more, can get them separately
|
|
2021-12-11 08:29:39
|
RGBA, RGBX, RGBD, RGBK (CMYK), and maybe RGBT (and permutations of those, like ARGB and BGRA) are the ones I assume to be useful atm
|
|
2021-12-11 08:34:08
|
I think a string approach to define the order is not so crazy. Have a char[4] in PixelFormat and define letters to mean stuff.
|
|
|
|
veluca
|
2021-12-11 08:35:25
|
I'd say that anything but RGBA, CMYK and possibly BGRA are not necessary
|
|
|
|
_wb_
|
2021-12-11 08:36:22
|
R,G,B : main color channels
A: first alpha channel
K: kBlack
T: kThermal
X: ignore/skip
1: set to maxval
0: set to 0
|
|
2021-12-11 08:38:30
|
It's just a matter of setting the pointers to planes correctly, right?
|
|
|
|
Vincent Torri
|
2021-12-11 08:59:35
|
<@!794205442175402004> you can throw your ideas there if you want : https://github.com/libjxl/libjxl/issues/993
|
|
|
|
veluca
|
|
_wb_
It's just a matter of setting the pointers to planes correctly, right?
|
|
2021-12-11 09:07:59
|
yes, in theory ... in practice, this may cause maintenance annoyances over time that I cannot see now
|
|
|
|
_wb_
|
2021-12-11 09:14:38
|
Yes, I guess we have to weigh pros and cons.
|
|
|
|
Vincent Torri
|
2021-12-11 09:27:01
|
note that on MSDN (Direct2D) :
|
|
2021-12-11 09:27:05
|
"We recommend that you use DXGI_FORMAT_B8G8R8A8_UNORM as the pixel format for better performance. This is particularly helpful for software render targets. BGRA format targets perform better than RGBA formats."
|
|
2021-12-11 09:27:26
|
so they consider that BGRA is the way to go on Windows
|
|
2021-12-11 09:27:38
|
reference : https://docs.microsoft.com/en-us/windows/win32/direct2d/supported-pixel-formats-and-alpha-modes
|
|
2021-12-13 09:54:18
|
i guess that nobody is trying to build a Visual Studio solution and build it, right ?
|
|
2021-12-13 09:56:31
|
there are a HUGE number of warnings
|
|
2021-12-13 09:56:38
|
there is*
|
|
|
|
_wb_
|
2021-12-13 10:35:54
|
https://github.com/libjxl/libjxl/issues/933
|
|
2021-12-13 10:36:03
|
https://tenor.com/view/vegeta-dragon-ball-z-unlimited-power-over9000-power-level-gif-12316102
|
|
|
|
Vincent Torri
|
2021-12-13 10:42:18
|
<@!794205442175402004> there are a lot of them being numeric values stored in a float
|
|
2021-12-13 10:42:26
|
like float f = 1.2;
|
|
2021-12-13 10:42:41
|
easy to fix
|
|
2021-12-13 10:43:10
|
i have not 4000, but ~5800
|
|
|
diskorduser
|
2021-12-13 10:43:52
|
9000+ when
|
|
|
|
_wb_
|
2021-12-13 11:28:59
|
I don't have MSVC, if you can send me a file with all the warnings and can start fixing them, or if someone feels like making a PR for it, even better
|
|
|
|
Vincent Torri
|
2021-12-13 12:07:09
|
looking at the log, errors are reported twice or even more...
|
|
2021-12-13 12:07:18
|
hmm
|
|
2021-12-13 12:07:26
|
maybe because of static and shared lib
|
|
|
novomesk
|
|
_wb_
I don't have MSVC, if you can send me a file with all the warnings and can start fixing them, or if someone feels like making a PR for it, even better
|
|
2021-12-13 12:32:23
|
I have a build log here: https://ci.appveyor.com/project/novomesk/qt-jpegxl-image-plugin
|
|
|
|
veluca
|
|
novomesk
I have a build log here: https://ci.appveyor.com/project/novomesk/qt-jpegxl-image-plugin
|
|
2021-12-13 12:51:42
|
ohhh some of these look like *fun*
|
|
2021-12-13 12:51:47
|
```
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\vector(1245): warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc
```
|
|
|
|
_wb_
|
2021-12-13 03:16:54
|
https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4530?view=msvc-170
|
|
2021-12-13 03:17:07
|
> If no exceptions can possibly be thrown in your executable, you may safely ignore this warning.
|
|
2021-12-13 03:17:34
|
well that's helpful
|
|
|
spider-mario
|
2021-12-13 03:34:46
|
wouldn’t that technically require that we only use “new (std::nothrow)” and so on everywhere though?
|
|
|
|
_wb_
|
2021-12-13 05:25:35
|
yes
|
|
2021-12-13 05:25:59
|
I think we may actually want to catch OOM
|
|
|
|
improver
|
2021-12-13 09:18:20
|
just check for null pointers
|
|
2021-12-13 09:19:08
|
at least i think that's how nothrow new works
|
|
|
|
_wb_
|
2021-12-13 09:19:21
|
actually we don't use `new` very often, and MSVC seems to complain mostly about std::vector
|
|
2021-12-13 09:19:35
|
yes, nothrow new just gives you a nullptr
|
|
|
|
improver
|
2021-12-13 09:19:49
|
hrm. could maybe tell it to use custom allocator
|
|
2021-12-13 09:21:56
|
also shouldn't library generally have a way of specifying allocator used for allocating internal stuff anyway
|
|
|
|
_wb_
|
2021-12-13 09:23:16
|
Yes, I think we have that, but I guess we only use it for big allocs? No idea actually
|
|
2021-12-13 09:24:14
|
<@179701849576833024> or <@768090355546587137> or <@604964375924834314> probably know
|
|
|
|
improver
|
2021-12-13 09:24:16
|
guess it's kinda hard to guard all places especially like std::vector usage
|
|
|
|
_wb_
|
2021-12-13 09:25:08
|
Still, is nice if we can return an OOM error instead of just crashing
|
|
|
|
improver
|
2021-12-13 09:25:25
|
i don't remember C++ that well to know how or if nothrow new[] allocator even interacts with std::vector
|
|
2021-12-13 09:25:49
|
been mostly coding golang for years at this point..
|
|
2021-12-13 09:26:54
|
i really like zig's way of explicitly specifying allocators but this is going too offtopic tbh
|
|
2021-12-13 09:28:55
|
<https://stackoverflow.com/questions/4826838/do-standard-library-stl-containers-support-a-form-of-nothrow-allocation> ugh this doesn't look doable
|
|
2021-12-13 09:29:18
|
that is, usage of nothrow allocator with std::vector.. i guess another way would be catching exceptions at api border
|
|
2021-12-13 09:31:27
|
also it would probably make sense to do std::vector based type which forces initialization with allocator of current context and enforce its use instead of just trying to be sure all std::vector invocations do that right
|
|
|
|
veluca
|
2021-12-13 09:47:34
|
we don't use period IIRC
|
|
2021-12-13 09:47:47
|
probably something that should eventually be fixed
|
|
|
|
thedonthe
|
2021-12-13 10:14:13
|
just curious but is there any sort of release schedule hanging out anywhere for libjxl?
|
|
|
|
_wb_
|
2021-12-14 10:03:52
|
there isn't, but maybe we should have that
|
|
2021-12-14 10:04:15
|
the thing is we don't really have a real schedule 🙂
|
|
2021-12-14 10:05:01
|
atm our goal is to reach 1.0 "somewhere in February" but I wouldn't exactly call it a schedule — it will be ready when it is ready
|
|
|
Jim
|
2021-12-14 11:35:11
|
#goals
|
|
|
lithium
|
2021-12-14 12:16:34
|
libjxl 1.0 also include separate improvement?
|
|
|
|
_wb_
|
2021-12-14 12:58:20
|
1.0 is about api finalization mostly. Encoder improvements will likely be for afterwards.
|
|
|
|
lithium
|
2021-12-14 12:59:47
|
I understand, thank you 🙂
|
|
|
|
Vincent Torri
|
2021-12-15 10:59:30
|
<@!794205442175402004> hello
|
|
2021-12-15 11:00:01
|
<@!794205442175402004> i have written a jxl saver (using the encoder API) and I have a seg fault
|
|
2021-12-15 11:00:37
|
i think that i have misused the API, but on the other hand i don't think that the lib should seg fault
|
|
|
|
_wb_
|
2021-12-15 11:01:17
|
it shouldn't, no - it should return error when misusing the API
|
|
|
|
Vincent Torri
|
2021-12-15 11:02:07
|
relevant code :
|
|
2021-12-15 11:02:09
|
https://pastebin.com/qJ2V0DPC
|
|
2021-12-15 11:02:35
|
line of the segfault (in paste) : 96
|
|
2021-12-15 11:03:31
|
and backtrace : https://pastebin.com/xHi2kmiS
|
|
2021-12-15 11:04:42
|
i'm not sure that the last argument of JxlEncoderAddImageFrame() is correct
|
|
|
|
w
|
2021-12-15 11:04:52
|
just a quick glance but maybe `im->cache_entry.w * im->cache_entry.h * 4` is wider than specified `pixel_format.num_channels = 3`?
|
|
|
|
Vincent Torri
|
2021-12-15 11:05:26
|
i've thought about that
|
|
2021-12-15 11:07:41
|
but i pass my own data to encode
|
|
2021-12-15 11:08:00
|
so this API should know thesize of my buffer, no ?
|
|
|
|
w
|
2021-12-15 11:08:22
|
isnt the data just a pointer
|
|
|
|
_wb_
|
2021-12-15 11:53:18
|
Is im->data actually pointing to memory that can be accessed and that has that size?
|
|
2021-12-15 11:53:44
|
It's not using 8-bit buffers or something?
|
|
|
|
Vincent Torri
|
2021-12-15 11:54:29
|
im->image.data is a BGRA buffer
|
|
|
|
_wb_
|
2021-12-15 11:54:53
|
Yes, but is it using 4 bytes per pixel or 16?
|
|
|
|
Vincent Torri
|
2021-12-15 11:54:54
|
8bits per chan
|
|
|
|
_wb_
|
2021-12-15 11:55:04
|
Then your pixelformat is all wrong
|
|
|
|
Vincent Torri
|
2021-12-15 11:55:09
|
😄
|
|
2021-12-15 11:55:53
|
i've looked at the encode example
|
|
|
|
_wb_
|
2021-12-15 11:55:53
|
You are telling libjxl that you have RGB data in float, so 12 bytes per pixel
|
|
2021-12-15 11:56:20
|
And then you give it a buffer of size 4 bytes per pixel
|
|
|
|
Vincent Torri
|
|
|
_wb_
|
2021-12-15 11:56:40
|
And you tell libjxl that it can access a buffer of size 16 bytes per pixel
|
|
|
|
Vincent Torri
|
2021-12-15 11:56:40
|
num_channels : 4
|
|
2021-12-15 11:56:45
|
type INT
|
|
2021-12-15 11:56:54
|
?
|
|
|
|
_wb_
|
2021-12-15 11:56:59
|
JXL_UINT8, iirc
|
|
2021-12-15 11:57:28
|
Or JXL_TYPE_UINT8, probably
|
|
|
|
Vincent Torri
|
2021-12-15 11:57:51
|
JXL_TYPE_UINT8
|
|
2021-12-15 11:57:54
|
indeed
|
|
|
|
_wb_
|
|
_wb_
it shouldn't, no - it should return error when misusing the API
|
|
2021-12-15 11:58:57
|
Actually here it can only segfault, we cannot check if you passed a pointer+size that is actually valid
|
|
|
|
Vincent Torri
|
2021-12-15 12:02:17
|
<@!794205442175402004> is basic_info correct too ?
|
|
|
|
_wb_
|
2021-12-15 12:04:04
|
You can put whatever bitdepth you want there, but if the original is 8-bit then you probably don't want to claim that the data has 32-bit float precision
|
|
2021-12-15 12:04:32
|
So I would keep bitdepth/expbits at their default, which is 8-bit
|
|
|
|
Vincent Torri
|
2021-12-15 12:05:17
|
let's try
|
|
2021-12-15 12:05:58
|
i'm doing comparison with libjpeg and avif
|
|
2021-12-15 12:06:08
|
avif seems quite slow
|
|
2021-12-15 12:06:19
|
hmm, still seg fault
|
|
2021-12-15 12:11:21
|
updated code :
|
|
2021-12-15 12:13:08
|
https://pastebin.com/XB27wenJ
|
|
2021-12-15 12:13:12
|
brb
|
|
2021-12-15 12:43:08
|
someone should write an example where you encode (A)RGB data...
|
|
|
|
_wb_
|
2021-12-15 12:43:28
|
not sure why SetBasicInfo is not complaining about it, but it's invalid to have bitdepth-expbits < 3
|
|
|
|
Vincent Torri
|
2021-12-15 12:44:51
|
where ?
|
|
|
|
_wb_
|
2021-12-15 12:47:08
|
anyway, `sizeof(int)` is not the same thing as `sizeof(uint8_t)`
|
|
2021-12-15 12:47:27
|
likely `int` is either 4 or 8 bytes
|
|
2021-12-15 12:47:53
|
so you're still claiming the buffer is larger than it actually is
|
|
2021-12-15 12:49:13
|
(not sure why it would read past its actual end though)
|
|
|
|
Vincent Torri
|
2021-12-15 12:50:56
|
sizeof(int) != 4, where ? 99.99% of OS have sizeof(int) == 4
|
|
2021-12-15 12:51:15
|
for long, I agree, but int...
|
|
2021-12-15 12:51:59
|
the EFL run on mac, unix (linux, *BSD and solaris) and windows where int is of size 4 bytes, so good for me
|
|
|
|
_wb_
|
2021-12-15 01:33:21
|
well in any case your samples are not using 32-bit ints but they are using 8-bit ints
|
|
|
|
Vincent Torri
|
2021-12-15 01:35:01
|
yes
|
|
2021-12-15 01:35:31
|
documentation of JxlEncoderAddImageFrame :
|
|
2021-12-15 01:35:50
|
"size – size of buffer in bytes. "
|
|
2021-12-15 01:36:05
|
my buffer has w*h pixels
|
|
2021-12-15 01:36:29
|
one pixel is coded with 4 bytes (unsigned int)
|
|
2021-12-15 01:37:07
|
so i pass sizeof(int) (which is == to sizeof(unsigned int)) mulitplied with w*h
|
|
2021-12-15 01:37:29
|
wait
|
|
2021-12-15 01:37:35
|
8-bit ints ?
|
|
2021-12-15 01:37:58
|
what do you mean by that ?
|
|
|
|
w
|
2021-12-15 01:38:17
|
4 bytes per pixel
|
|
2021-12-15 01:38:21
|
8 bits x 4 bytes = 32 bits
|
|
2021-12-15 01:38:52
|
each component is 1 byte
|
|
|
|
Vincent Torri
|
2021-12-15 01:38:55
|
ok, enough discussion
|
|
2021-12-15 01:39:09
|
if you see an error and know the solution, please tell me
|
|
|
|
_wb_
|
2021-12-15 02:26:06
|
the error is that you do 4 * w * h * 4
|
|
2021-12-15 02:26:14
|
while it should be only one 4
|
|
2021-12-15 02:27:55
|
one pixel is coded with 4 uint8_t (unsigned char), not 4 ints
|
|
|
|
Vincent Torri
|
2021-12-15 02:38:16
|
indeed
|
|
2021-12-15 02:38:21
|
still seg fault
|
|
2021-12-15 02:42:37
|
oups
|
|
2021-12-15 02:42:37
|
```c
basic_info.bits_per_sample = 8;
basic_info.exponent_bits_per_sample = 8;```
|
|
2021-12-15 02:42:37
|
is it good ?
|
|
|
|
_wb_
|
2021-12-15 03:02:58
|
No
|
|
2021-12-15 03:03:39
|
You have 0 exp bits
|
|
2021-12-15 03:03:52
|
Since you have integers, not floats
|
|
2021-12-15 03:05:04
|
Exp bits is for when you want to do floats (we have explicit nb of exp bits so we can do both binary16 half-floats and bfloat16)
|
|
2021-12-15 03:06:47
|
I just updated the encoder to refuse to accept that basic info, since it is not valid - you need at least 2 mantissa bits and a sign bit, so if you have 8 bits_per_sample at most 5 of them can be exponent bits
|
|
|
|
Vincent Torri
|
2021-12-15 03:19:45
|
i'll pull it
|
|
2021-12-15 03:20:03
|
and alphabits and nul_color_channels ?
|
|
2021-12-15 03:20:08
|
num*
|
|
|
|
_wb_
|
2021-12-15 03:27:17
|
alphabits you'll want to set to 8 if you want to save the alpha
|
|
2021-12-15 03:27:32
|
num_color_channels = 3 and num_extra_channels = 1
|
|
2021-12-15 03:27:45
|
for RGB and A respectively
|
|
|
|
Hello71
|
|
_wb_
Actually here it can only segfault, we cannot check if you passed a pointer+size that is actually valid
|
|
2021-12-15 03:28:56
|
*technically*... https://man7.org/linux/man-pages/man3/malloc_usable_size.3.html
|
|
2021-12-15 03:29:27
|
although you should of course just use asan or valgrind
|
|
2021-12-15 03:30:55
|
actually if you want to be really dumb you could just install a SIGSEGV handler
|
|
|
|
_wb_
|
2021-12-15 04:01:27
|
I would expect malloc_usable_size to only work if the pointer points to something allocated by malloc
|
|
|
|
Vincent Torri
|
2021-12-15 04:06:33
|
ha, num_extra_channels was missing
|
|
2021-12-15 04:06:34
|
trying
|
|
2021-12-15 04:06:59
|
<@!794205442175402004> btw, what about adding an API to fill basic info for some scenarii ?
|
|
|
|
_wb_
|
2021-12-15 04:09:56
|
like a function that returns a common basic info, something like `JxlMakeBasicInfo(JXL_BOOL have_alpha, uint32_t bitdepth)`?
|
|
2021-12-15 04:11:25
|
could be convenient, please open an issue for it if you want us to add that - it's of course just syntactic sugar but it might indeed be nice
|
|
|
|
Vincent Torri
|
2021-12-15 04:11:28
|
or even JxlMakeBasicInfoRGB(), JxlMakeBasicInfoRGBA
|
|
2021-12-15 04:11:35
|
etc...
|
|
2021-12-15 04:11:47
|
ok for the issue
|
|
|
|
_wb_
|
2021-12-15 04:12:08
|
in the tests we have a function that makes a basicinfo from a pixelformat, because typically you want those to match
|
|
|
|
Vincent Torri
|
2021-12-15 04:12:21
|
right
|
|
|
|
_wb_
|
2021-12-15 04:12:59
|
i mean you can pass pixels in 16-bit and encode them as 8-bit, that would e.g. be what a default ImageMagick will do
|
|
|
|
Vincent Torri
|
2021-12-15 04:15:29
|
hmm
|
|
2021-12-15 04:16:11
|
about the buffer passed to JxlEncoderAddImageFrame
|
|
2021-12-15 04:16:24
|
is it delete by a destructor ?
|
|
2021-12-15 04:27:40
|
almost good : seg fault in JxlEncoderDestroy(encoder);
|
|
2021-12-15 04:37:51
|
if i remove this line, it seems ok
|
|
2021-12-15 04:38:07
|
but why does it seg fault ??
|
|
|
|
_wb_
|
2021-12-15 04:48:57
|
Good question
|
|
2021-12-15 04:49:12
|
Can you share the code from addimageframe to end?
|
|
|
|
Vincent Torri
|
2021-12-15 04:51:02
|
heer it is : https://pastebin.com/cnfjGrtA
|
|
2021-12-15 04:53:08
|
ok, around same psnr, jxl file is around half the size f a jpeg file
|
|
2021-12-15 04:53:14
|
quality == 90%
|
|
2021-12-15 04:53:46
|
not bad
|
|
2021-12-15 04:54:01
|
encoding is quite faster than with libavif
|
|
|
|
_wb_
|
2021-12-15 04:55:07
|
I think you shouldn't free the frame_settings before destroying
|
|
2021-12-15 04:56:42
|
Destroying the encoder will also destroy its settings
|
|
2021-12-15 04:57:04
|
Iirc
|
|
|
|
Vincent Torri
|
2021-12-15 04:57:21
|
ha
|
|
2021-12-15 04:57:37
|
trying
|
|
2021-12-15 04:59:38
|
"The returned pointer is an opaque struct tied to the encoder and it will be deallocated by the encoder when JxlEncoderDestroy() is called."
|
|
2021-12-15 04:59:58
|
in that case the returned pointer should be marked as const, no ?
|
|
2021-12-15 05:25:00
|
saver done
|
|
2021-12-15 05:25:14
|
i'll clean the code and add it to the efl git
|
|
2021-12-15 05:25:37
|
next efl release will have to loader/saver
|
|
2021-12-15 05:29:06
|
i hope that the will be a bgra output, that will make the loader/saver faster
|
|
|
|
_wb_
|
2021-12-15 05:34:31
|
it'll likely happen, eventually 🙂
|
|
|
Vincent Torri
in that case the returned pointer should be marked as const, no ?
|
|
2021-12-15 05:43:50
|
Good question, <@179701849576833024> WDYT?
|
|
|
|
veluca
|
2021-12-15 05:46:18
|
it would be weird to return a `const JxlFrameSettings*` and then have people use functions that modify it...
|
|
2021-12-15 05:46:37
|
(while technically allowed by the C(++) standard)
|
|
2021-12-15 05:47:01
|
(`const` in C(++) is more of a guideline than an actual rule...)
|
|
2021-12-15 05:47:32
|
also it wouldn't stop you from freeing it, nor it would give you indication that it shouldn't be freed I believe
|
|
|
|
_wb_
|
2021-12-15 05:50:10
|
Can we return a const pointer to a non-const object?
|
|
2021-12-15 05:50:33
|
Or am I talking nonsense
|
|
|
|
vtorri
|
2021-12-15 06:09:52
|
const is indeed a guideline, but it would prevent people to do what I did : freeing the pointer while i shouldn't
|
|
2021-12-15 06:10:35
|
if a function returns a const pointer, there it will be unlikely that the user will free it
|
|
2021-12-15 06:12:43
|
so yes, only guidelne
|
|
2021-12-15 06:13:48
|
well, at least, imho
|
|
|
|
improver
|
2021-12-15 06:26:50
|
function can't return const pointer, only pointer to const. if you have pointer you can free it regardless of const-ness. ownership and const-ness are separate concepts in C/C++
|
|
|
|
vtorri
|
2021-12-15 06:28:01
|
that's why it's guideline
|
|
|
|
improver
|
2021-12-15 06:33:06
|
if user can somehow modify its insides, even including use of library functions, it should be not pointing to const. this also includes destroy function
|
|
|
|
_wb_
|
2021-12-15 06:33:49
|
We could maybe change the example code to make it a const pointer in the example
|
|
|
|
vtorri
|
2021-12-15 06:34:07
|
or at least add a comment
|
|
|
|
_wb_
|
2021-12-15 06:34:19
|
Const pointer to non-const
|
|