Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JPEG XL support with libjxl #99

Merged
merged 6 commits into from
Jan 7, 2023
Merged

JPEG XL support with libjxl #99

merged 6 commits into from
Jan 7, 2023

Conversation

qbnu
Copy link
Contributor

@qbnu qbnu commented Jan 1, 2023

Includes support for animation.
Closes #91

@sylikc sylikc added the enhancement New feature or request label Jan 3, 2023
#include "MaxImageDef.h"
#include <vector>

JxlDecoderPtr cached_jxl_decoder = NULL;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really clear how these variables are scoped. So this variable appears to be scoped globally more or less, and not tied to a class... but later in JxlReader::DeleteCache() it sets this to NULL?

Does that mean these should be static class members of JxlReader and not file scoped?

Am I reading this code right?

Copy link
Contributor Author

@qbnu qbnu Jan 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the caching I did was pretty messy. Would something like this in be better?
In JXLWrapper.h:

// ...
private:
	struct jxl_cache;
	static jxl_cache cache;
	static inline bool DecodeJpegXlOneShot(const uint8_t* jxl, size_t size, std::vector<uint8_t>* pixels, int& xsize,
		int& ysize, bool& have_animation, int& frame_count, int& frame_time, std::vector<uint8_t>* icc_profile, bool& outOfMemory);
};

In JXLWrapper.cpp

struct JxlReader::jxl_cache {
	JxlDecoderPtr decoder;
	JxlResizableParallelRunnerPtr runner;
	JxlBasicInfo info;
	uint8_t* data;
	size_t data_size;
	int prev_frame_timestamp;
	int width;
	int height;
};

// based on https://github.com/libjxl/libjxl/blob/main/examples/decode_oneshot.cc
inline bool JxlReader::DecodeJpegXlOneShot(const uint8_t* jxl, size_t size, std::vector<uint8_t>* pixels, int& xsize,
// ...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's about what I was thinking, encapsulating that data into the JxlReader class seems to be a better approach. Thanks!

@sylikc
Copy link
Owner

sylikc commented Jan 5, 2023

I reviewed the rest of the code and it looks good. I'll merge this after that cache fix

@qbnu thanks for all your hard work. you make this look easy! It also nudges me to do more of the code I've been slow to get to (fixing the INI and keymap defaults, graphics and installer updates, etc)

Any other formats you'd like to help add?

  • HEIC/HEIF built-in without using WIC would be nice.
  • I recall seeing an issue opened for AVIF, but that's such a new codec I don't even have an image downloaded with that yet...
  • SVG might be a challenge to add, I recall SVGs are just an XML file that a viewer has to calculate to render
  • JPEG 2000 seems to be a dying breed, I read that adoption rates for it were low.

😊😁😎

@qbnu
Copy link
Contributor Author

qbnu commented Jan 6, 2023

Yeah, I was thinking about adding HEIF/AVIF support with libheif (using libde265 and dav1d), and adding others with sail. I'll submit another PR to clean up the caching for other formats.

@sylikc
Copy link
Owner

sylikc commented Jan 6, 2023

yeah I had just looked into libheif recently, thinking how I might be able to use your recent implementations as an example to attempt... but i'm just not as knowledgeable with image data 🤷‍♂️

@sylikc
Copy link
Owner

sylikc commented Jan 6, 2023

ok, I must be doing something wrong... I'm trying to follow https://github.com/libjxl/libjxl/blob/main/doc/developing_in_windows_vcpkg.md to build libjxl and I can't get it working. Lots of things to do and it's non-trivial. Would you happen to remember what steps you took to get libjxl libs built?

[156/495] cmd.exe /C "cd . && "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -E vs_link_exe --intdir=third_party\highway\CMakeFiles\aligned_allocator_test.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100190~1.0\x86\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100190~1.0\x86\mt.exe --manifests  -- C:\PROGRA~2\MICROS~2\2019\COMMUN~1\VC\Tools\Llvm\bin\lld-link.exe /nologo third_party\highway\CMakeFiles\aligned_allocator_test.dir\hwy\aligned_allocator_test.cc.obj  /out:third_party\highway\tests\aligned_allocator_test.exe /implib:third_party\highway\aligned_allocator_test.lib /pdb:third_party\highway\tests\aligned_allocator_test.pdb /version:0.0 /machine:x64 /INCREMENTAL:NO /subsystem:console  third_party\highway\hwy.lib  third_party\highway\hwy_test.lib  lib\gtest_main.lib  third_party\highway\hwy.lib  lib\gtest.lib  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cmd.exe /C "cd /D H:\GitHub\jpegview\extras\libjxl\out\build\x64-Clang-Release\third_party\highway && C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -noprofile -executionpolicy Bypass -file H:/GitHub/vcpkg/scripts/buildsystems/msbuild/applocal.ps1 -targetBinary H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway/tests/aligned_allocator_test.exe -installedDir H:/GitHub/vcpkg/installed/x64-windows/bin -OutVariable out && cd /D H:\GitHub\jpegview\extras\libjxl\out\build\x64-Clang-Release\third_party\highway && "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -D TEST_TARGET=aligned_allocator_test -D TEST_EXECUTABLE=H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway/tests/aligned_allocator_test.exe -D TEST_EXECUTOR= -D TEST_WORKING_DIR=H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway -D TEST_EXTRA_ARGS= -D TEST_PROPERTIES= -D TEST_PREFIX= -D TEST_SUFFIX= -D NO_PRETTY_TYPES=FALSE -D NO_PRETTY_VALUES=FALSE -D TEST_LIST=aligned_allocator_test_TESTS -D CTEST_FILE=H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway/aligned_allocator_test[1]_tests.cmake -D TEST_DISCOVERY_TIMEOUT=60 -D TEST_XML_OUTPUT_DIR= -P "C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/share/cmake-3.20/Modules/GoogleTestAddTests.cmake"""
  FAILED: third_party/highway/tests/aligned_allocator_test.exe third_party/highway/aligned_allocator_test[1]_tests.cmake 
  cmd.exe /C "cd . && "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -E vs_link_exe --intdir=third_party\highway\CMakeFiles\aligned_allocator_test.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100190~1.0\x86\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100190~1.0\x86\mt.exe --manifests  -- C:\PROGRA~2\MICROS~2\2019\COMMUN~1\VC\Tools\Llvm\bin\lld-link.exe /nologo third_party\highway\CMakeFiles\aligned_allocator_test.dir\hwy\aligned_allocator_test.cc.obj  /out:third_party\highway\tests\aligned_allocator_test.exe /implib:third_party\highway\aligned_allocator_test.lib /pdb:third_party\highway\tests\aligned_allocator_test.pdb /version:0.0 /machine:x64 /INCREMENTAL:NO /subsystem:console  third_party\highway\hwy.lib  third_party\highway\hwy_test.lib  lib\gtest_main.lib  third_party\highway\hwy.lib  lib\gtest.lib  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cmd.exe /C "cd /D H:\GitHub\jpegview\extras\libjxl\out\build\x64-Clang-Release\third_party\highway && C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -noprofile -executionpolicy Bypass -file H:/GitHub/vcpkg/scripts/buildsystems/msbuild/applocal.ps1 -targetBinary H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway/tests/aligned_allocator_test.exe -installedDir H:/GitHub/vcpkg/installed/x64-windows/bin -OutVariable out && cd /D H:\GitHub\jpegview\extras\libjxl\out\build\x64-Clang-Release\third_party\highway && "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -D TEST_TARGET=aligned_allocator_test -D TEST_EXECUTABLE=H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway/tests/aligned_allocator_test.exe -D TEST_EXECUTOR= -D TEST_WORKING_DIR=H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway -D TEST_EXTRA_ARGS= -D TEST_PROPERTIES= -D TEST_PREFIX= -D TEST_SUFFIX= -D NO_PRETTY_TYPES=FALSE -D NO_PRETTY_VALUES=FALSE -D TEST_LIST=aligned_allocator_test_TESTS -D CTEST_FILE=H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway/aligned_allocator_test[1]_tests.cmake -D TEST_DISCOVERY_TIMEOUT=60 -D TEST_XML_OUTPUT_DIR= -P "C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/share/cmake-3.20/Modules/GoogleTestAddTests.cmake"""
  CMake Error at C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/share/cmake-3.20/Modules/GoogleTestAddTests.cmake:77 (message):
    Error running test executable.
  
      Path: 'H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway/tests/aligned_allocator_test.exe'
      Result: Exit code 0xc0000139
  
    
  
      Output:
        
  
  Call Stack (most recent call first):
    C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/share/cmake-3.20/Modules/GoogleTestAddTests.cmake:173 (gtest_discover_tests_impl)
  
  
  [157/495] cmd.exe /C "cd . && "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -E vs_link_exe --intdir=third_party\highway\CMakeFiles\highway_test.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100190~1.0\x86\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100190~1.0\x86\mt.exe --manifests  -- C:\PROGRA~2\MICROS~2\2019\COMMUN~1\VC\Tools\Llvm\bin\lld-link.exe /nologo third_party\highway\CMakeFiles\highway_test.dir\hwy\highway_test.cc.obj  /out:third_party\highway\tests\highway_test.exe /implib:third_party\highway\highway_test.lib /pdb:third_party\highway\tests\highway_test.pdb /version:0.0 /machine:x64 /INCREMENTAL:NO /subsystem:console  third_party\highway\hwy.lib  third_party\highway\hwy_test.lib  lib\gtest_main.lib  third_party\highway\hwy.lib  lib\gtest.lib  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cmd.exe /C "cd /D H:\GitHub\jpegview\extras\libjxl\out\build\x64-Clang-Release\third_party\highway && C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -noprofile -executionpolicy Bypass -file H:/GitHub/vcpkg/scripts/buildsystems/msbuild/applocal.ps1 -targetBinary H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway/tests/highway_test.exe -installedDir H:/GitHub/vcpkg/installed/x64-windows/bin -OutVariable out && cd /D H:\GitHub\jpegview\extras\libjxl\out\build\x64-Clang-Release\third_party\highway && "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -D TEST_TARGET=highway_test -D TEST_EXECUTABLE=H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway/tests/highway_test.exe -D TEST_EXECUTOR= -D TEST_WORKING_DIR=H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway -D TEST_EXTRA_ARGS= -D TEST_PROPERTIES= -D TEST_PREFIX= -D TEST_SUFFIX= -D NO_PRETTY_TYPES=FALSE -D NO_PRETTY_VALUES=FALSE -D TEST_LIST=highway_test_TESTS -D CTEST_FILE=H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway/highway_test[1]_tests.cmake -D TEST_DISCOVERY_TIMEOUT=60 -D TEST_XML_OUTPUT_DIR= -P "C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/share/cmake-3.20/Modules/GoogleTestAddTests.cmake"""
  [158/495] cmd.exe /C "cd . && "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -E vs_link_exe --intdir=third_party\highway\CMakeFiles\mul_test.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100190~1.0\x86\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100190~1.0\x86\mt.exe --manifests  -- C:\PROGRA~2\MICROS~2\2019\COMMUN~1\VC\Tools\Llvm\bin\lld-link.exe /nologo third_party\highway\CMakeFiles\mul_test.dir\hwy\tests\mul_test.cc.obj  /out:third_party\highway\tests\mul_test.exe /implib:third_party\highway\mul_test.lib /pdb:third_party\highway\tests\mul_test.pdb /version:0.0 /machine:x64 /INCREMENTAL:NO /subsystem:console  third_party\highway\hwy.lib  third_party\highway\hwy_test.lib  lib\gtest_main.lib  third_party\highway\hwy.lib  lib\gtest.lib  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cmd.exe /C "cd /D H:\GitHub\jpegview\extras\libjxl\out\build\x64-Clang-Release\third_party\highway && C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -noprofile -executionpolicy Bypass -file H:/GitHub/vcpkg/scripts/buildsystems/msbuild/applocal.ps1 -targetBinary H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway/tests/mul_test.exe -installedDir H:/GitHub/vcpkg/installed/x64-windows/bin -OutVariable out && cd /D H:\GitHub\jpegview\extras\libjxl\out\build\x64-Clang-Release\third_party\highway && "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -D TEST_TARGET=mul_test -D TEST_EXECUTABLE=H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway/tests/mul_test.exe -D TEST_EXECUTOR= -D TEST_WORKING_DIR=H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway -D TEST_EXTRA_ARGS= -D TEST_PROPERTIES= -D TEST_PREFIX= -D TEST_SUFFIX= -D NO_PRETTY_TYPES=FALSE -D NO_PRETTY_VALUES=FALSE -D TEST_LIST=mul_test_TESTS -D CTEST_FILE=H:/GitHub/jpegview/extras/libjxl/out/build/x64-Clang-Release/third_party/highway/mul_test[1]_tests.cmake -D TEST_DISCOVERY_TIMEOUT=60 -D TEST_XML_OUTPUT_DIR= -P "C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/share/cmake-3.20/Modules/GoogleTestAddTests.cmake"""
  ninja: build stopped: subcommand failed.

Build All failed.

Also, they only have x64 build instructions... I'm guessing with x86, I'd use vcpkg to install the x86 stuff on use the x86-Clang-Release configuration?

@qbnu
Copy link
Contributor Author

qbnu commented Jan 6, 2023

I built it with GH Actions using cmake and MSVC, despite what it says in their instructions.
From the libjxl folder:

git submodule update --init --recursive --depth 1 --recommend-shallow
mkdir build
cd build
cmake -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=OFF .. -A Win32
msbuild.exe /p:Platform=Win32 /p:configuration="Release" LIBJXL.sln /t:jxl_dec /t:jxl_threads

Switch Win32 to x64 for 64-bit version. It takes a really long time to compile.

Honestly the hardest part about adding new image formats is trying to get libraries to build and link properly.

@sylikc
Copy link
Owner

sylikc commented Jan 7, 2023

Thanks for those instructions. I knew it had to be user error 🤦‍♂️

I've documented them and will merge them with the PR

sEnding += 1;
if (_tcsicmp(sEnding, _T("JXL")) == 0) {
_tcscat_s(buff, BUF_LEN, _T("\n"));
_tcscat_s(buff, BUF_LEN, CNLS::GetString(_T("JXL decoding requires the Microsoft Visual C++ Redistributable")));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the elegant solution of displaying the error message directly on JPEGView window. Very cool way of implementing it.

Took me awhile to break a Windows system enough to not have the vcruntime140.dll, so I could test this use case

sylikc added a commit that referenced this pull request Jan 7, 2023
- includes support for animation
- Microsoft Visual C++ Runtime vcruntime140.dll required when opening JPEG XL files

Merge PR #99 by https://github.com/qbnu

See PR #92 for other comments, especially related to the MSVC++ Redistributable
@sylikc sylikc merged commit 786f5eb into sylikc:master Jan 7, 2023
@Pentaphon
Copy link

Any other formats you'd like to help add?

  • HEIC/HEIF built-in without using WIC would be nice.
  • I recall seeing an issue opened for AVIF, but that's such a new codec I don't even have an image downloaded with that yet...
  • SVG might be a challenge to add, I recall SVGs are just an XML file that a viewer has to calculate to render
  • JPEG 2000 seems to be a dying breed, I read that adoption rates for it were low.

Out of all those, the only one that seems remotely useful is SVG. The rest I have never needed to open with an image viewer. Ever.

Otherwise I think JPEG-XL is the biggest thing that has ever happened to this fork and I think that was the only format I ever felt this project needed due to being a JPEG viewer and JPEG-XL being so promising.

@sylikc sylikc added the format support Related to add/remove/change of a specific format support. label Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request format support Related to add/remove/change of a specific format support.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please add JPEG XL support
3 participants