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.

Sign up for GitHub

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

Jump to bottom

[RFC] TexturePacker: add zstd compression support #23306

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

[RFC] TexturePacker: add zstd compression support #23306

wants to merge 13 commits into from

Conversation

lrusak
Copy link
Contributor

@lrusak lrusak commented May 20, 2023

This is something I've been playing with after doing the TexturePacker modernization.

This add zstd compression/decompression support.

I originally had this in two parts but the changes in the first part wouldn't really make sense without the second part. The first branch was here -> https://github.com/lrusak/xbmc/commits/TexturePacker-PR-4

Compression support to TexturePacker and decompression to Kodi via CTextureBundleXBT.

I've added a switch to TexturePacker (-compression-method) to allow selecting the compression method method. If we want to allow the use of zstd compression we may look at deprecating lzo (though it's harmless to keep both) other than dependencies.

This also bumps the XBTF_VERSION because of the new m_compressionMethod member that is added to the serialization in the xbt file.

I've also added the cmake option -DTEXTUREPACKER_COMPRESSION_METHOD=[zstd|lzo|none] that can be used to select the compression method used to create the Texture.xbt bundle at build time.

I plan to add some more metrics that measure the speeds in the future but this is what I have for now. This is all on my i7 so it would be interesting to see on a low power arm device

TexturePacker:

TexturePacker: processed 681 files in 466.403 ms using zstd

vs

TexturePacker: processed 681 files in 2343.757 ms using lzo

Decompressions time is similar (zstd | lzo):

colors/white50.png: 0.027 ms (bundled)			   |	colors/white50.png: 0.016 ms (bundled)
colors/white.png: 0.007 ms (bundled)			   |	colors/white.png: 0.006 ms (bundled)
colors/white70.png: 0.031 ms (bundled)			   |	colors/white70.png: 0.005 ms (bundled)
osd/progress/nub_bar.png: 0.007 ms (bundled)			osd/progress/nub_bar.png: 0.007 ms (bundled)
buttons/slider-back.png: 0.022 ms (bundled)		   |	buttons/slider-back.png: 0.016 ms (bundled)
buttons/slider-nib.png: 0.011 ms (bundled)		   |	buttons/slider-nib.png: 0.009 ms (bundled)
colors/red50.png: 0.007 ms (bundled)			   |	colors/red50.png: 0.006 ms (bundled)
progress/texturebg_border_white.png: 0.015 ms (bundled)	   |	progress/texturebg_border_white.png: 0.013 ms (bundled)
progress/texturebg_white.png: 0.008 ms (bundled)	   |	progress/texturebg_white.png: 0.009 ms (bundled)
pointer_arrow.png: 0.027 ms (bundled)			   |	pointer_arrow.png: 0.050 ms (bundled)
pointer_click.png: 0.020 ms (bundled)			   |	pointer_click.png: 0.034 ms (bundled)
buttons/dialogbutton-fo.png: 0.258 ms (bundled)		   |	buttons/dialogbutton-fo.png: 0.268 ms (bundled)
buttons/dialogbutton-nofo.png: 0.011 ms (bundled)	   |	buttons/dialogbutton-nofo.png: 0.014 ms (bundled)
buttons/roundbutton-fo.png: 0.034 ms (bundled)		   |	buttons/roundbutton-fo.png: 0.036 ms (bundled)
icons/power.png: 0.032 ms (bundled)			   |	icons/power.png: 0.023 ms (bundled)
buttons/radio-button-on.png: 0.030 ms (bundled)			buttons/radio-button-on.png: 0.030 ms (bundled)
buttons/radio-button-off.png: 0.026 ms (bundled)	   |	buttons/radio-button-off.png: 0.029 ms (bundled)
icons/settings.png: 0.023 ms (bundled)			   |	icons/settings.png: 0.021 ms (bundled)
icons/search.png: 0.022 ms (bundled)			   |	icons/search.png: 0.018 ms (bundled)
icons/now-playing/fullscreen.png: 0.018 ms (bundled)	   |	icons/now-playing/fullscreen.png: 0.016 ms (bundled)
frame/menu-nofo.png: 0.017 ms (bundled)			   |	frame/menu-nofo.png: 0.013 ms (bundled)
icons/menu.png: 0.009 ms (bundled)			   |	icons/menu.png: 0.008 ms (bundled)
icons/favourites.png: 0.020 ms (bundled)		   |	icons/favourites.png: 0.016 ms (bundled)
overlays/shadow.png: 0.028 ms (bundled)			   |	overlays/shadow.png: 0.023 ms (bundled)
lists/panel.png: 0.011 ms (bundled)			   |	lists/panel.png: 0.012 ms (bundled)
lists/focus.png: 0.048 ms (bundled)			   |	lists/focus.png: 0.103 ms (bundled)
colors/black.png: 0.008 ms (bundled)			   |	colors/black.png: 0.007 ms (bundled)
icons/sidemenu/movies.png: 0.022 ms (bundled)		   |	icons/sidemenu/movies.png: 0.027 ms (bundled)
icons/sidemenu/tv.png: 0.023 ms (bundled)		   |	icons/sidemenu/tv.png: 0.039 ms (bundled)
icons/sidemenu/music.png: 0.024 ms (bundled)		   |	icons/sidemenu/music.png: 0.038 ms (bundled)
icons/sidemenu/games.png: 0.053 ms (bundled)		   |	icons/sidemenu/games.png: 0.033 ms (bundled)
icons/sidemenu/addons.png: 0.028 ms (bundled)		   |	icons/sidemenu/addons.png: 0.044 ms (bundled)
icons/sidemenu/pictures.png: 0.054 ms (bundled)		   |	icons/sidemenu/pictures.png: 0.045 ms (bundled)
icons/sidemenu/videos.png: 0.018 ms (bundled)		   |	icons/sidemenu/videos.png: 0.027 ms (bundled)
icons/sidemenu/favourites.png: 0.025 ms (bundled)	   |	icons/sidemenu/favourites.png: 0.040 ms (bundled)
icons/sidemenu/weather.png: 0.025 ms (bundled)		   |	icons/sidemenu/weather.png: 0.030 ms (bundled)
frame/InfoBar.png: 0.021 ms (bundled)			   |	frame/InfoBar.png: 0.030 ms (bundled)

File size is similar:

zstd:

-rw-r--r--. 1 lukas lukas 1012K May 19 19:59 addons/skin.estuary/media/Textures.xbt
-rw-r--r--. 1 lukas lukas 1035686 May 19 19:59 addons/skin.estuary/media/Textures.xbt

vs

lzo

-rw-r--r--. 1 lukas lukas 1.3M May 19 20:01 addons/skin.estuary/media/Textures.xbt
-rw-r--r--. 1 lukas lukas 1269532 May 19 20:01 addons/skin.estuary/media/Textures.xbt

This is just with the default zstd compression level (3).

What is the effect on users?

Faster Textures.xbt bundle decompression? Smaller Textures.xbt bundle size?

lrusak added 10 commits May 17, 2023 20:05
@lrusak
TexturePacker: convert m_flags to m_compressionMethod
224306d 
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
@lrusak
TexturePacker: add -compression-method switch
345f278 
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
@lrusak
TexturePacker: add packed size to console output
fce8f52 
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
@lrusak
CXBTFFrame: add XBTFCompressionMethod as a member
fbaf75e 
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
@lrusak
TexturePacker: add compression method to console output
d3429ea 
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
@lrusak
CTextureBundleXBT: use switch/case for compression method
b990ffe 
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
@lrusak
tools/depends: add zstd for native build
1878b12 
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
@lrusak
tools/depends: add zstd for target build
5aa5c0e 
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
@lrusak
TexturePacker: add zstd compression
afe7c8c 
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
@lrusak
[cmake] add option to set TexturePacker compression method
1eb7203 
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
@lrusak lrusak added this to the Omega 21.0 Alpha 2 milestone May 20, 2023
@lrusak
[TEMP] benchmarks
0f14b67 
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
@lrusak
fixup! tools/depends: add zstd for target build
b23f173
@lrusak
tools/depends: gnutls: disable zstd support
6ba3478 
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
@jenkins4kodi
Copy link
Contributor

I've made some formatting changes to meet the current code style. The diffs are available in the following links:

For more information please see our current code style guidelines.

garbear
garbear approved these changes May 27, 2023
View reviewed changes
Copy link
Member

@garbear garbear left a comment

Choose a reason for hiding this comment

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

I took a brief glance at the code but unfortunately can't go through it. This is a really cool feature. Do you think zstd could be used in RetroPlayer some day?

@jenkins4kodi jenkins4kodi added the Rebase needed PR that does not apply/merge cleanly to current base branch label Jun 9, 2023
@jenkins4kodi
Copy link
Contributor

@lrusak this needs a rebase

@thezoggy
Copy link

From another project which used lzo/lz4 I mentioned to them about the guy that created lz4, created zstd. Just its a bit more modern and offers more flexibility in compression levels.

They added support and I was able to do some testing, my take away is that zstd takes advantage of modern hardware better and even at its default compression levels was offering better compression (~20%) for the same or quickier performance.

@garbear
Copy link
Member

garbear commented Aug 8, 2023 •
edited

From another project which used lzo/lz4 I mentioned to them about the guy that created lz4, created zstd. Just its a bit more modern and offers more flexibility in compression levels.

They added support and I was able to do some testing, my take away is that zstd takes advantage of modern hardware better and even at its default compression levels was offering better compression (~20%) for the same or quickier performance.

Hell yeah! I integrated lz4 three years ago, and now I have zstd available as well. I want to try both at compressing XOR'd savestate memory and see how each algo does.

@lrusak I'm getting compiler errors on Android arm/arm64/x86:

  clang: error: the clang compiler does not support '-march=all'

https://jenkins.kodi.tv/job/android-arm64-docker/2991/console

@garbear
Copy link
Member

garbear commented Aug 10, 2023 •
edited

Getting a libcurl linking error, linking libkodi (I know this PR is TexturePacker only but it seems to affect libkodi in my personal test builds):

22:31:48 Undefined symbols for architecture x86_64:
22:31:48   "_ZSTD_createDStream", referenced from:
22:31:48       _zstd_init_writer in libcurl.a(libcurl_la-content_encoding.o)
22:31:49   "_ZSTD_decompressStream", referenced from:
22:31:49       _zstd_unencode_write in libcurl.a(libcurl_la-content_encoding.o)
22:31:49   "_ZSTD_freeDStream", referenced from:
22:31:49       _zstd_close_writer in libcurl.a(libcurl_la-content_encoding.o)
22:31:49   "_ZSTD_isError", referenced from:
22:31:49       _zstd_unencode_write in libcurl.a(libcurl_la-content_encoding.o)
22:31:49 ld: symbol(s) not found for architecture x86_64
22:31:49 clang: error: linker command failed with exit code 1 (use -v to see invocation)
22:31:49 make[2]: *** [Kodi.app/Contents/MacOS/Kodi] Error 1
22:31:49 make[1]: *** [CMakeFiles/kodi.dir/all] Error 2
22:31:49 make: *** [all] Error 2
22:31:49 Build step 'Execute shell' marked build as failure

LMK if you rebase and want further testing.

@fuzzard fuzzard added v22 "P" and removed v21 Omega labels Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garbear garbear garbear approved these changes

Assignees
No one assigned
Labels
Component: Build Component: Depends Component: GUI rendering Rebase needed PR that does not apply/merge cleanly to current base branch Type: Feature non-breaking change which adds functionality v22 "P"
Projects
None yet
Milestone
"P" 22.0 Alpha 1
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@lrusak @jenkins4kodi @thezoggy @garbear @fuzzard