Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Upgrade Basis plugins to 1.15 #112
Upgrade Basis plugins to 1.15 #112
Changes from all commits
3bc79d8
d83eb5b
09f13dd
75e47e7
7b9d26a
aec8e3d
564f380
256efbc
c603aa5
e7c5843
b2c3c84
f315885
38abfea
7cc5707
f1c5d05
015f24e
3c4852d
426da44
2bb860f
1bd045b
a4348cc
6f479a7
1b78acf
85ffabb
85e46e7
d29a013
d85bfb1
2d3a38e
42ea61f
57865fe
e65f438
a88304b
9d7a1e1
fc64838
cc933cf
d6c9dc7
2db78c8
b63e2e5
2ce8853
0542a35
cbb05c7
25bdbc3
5cc3bbf
e8b48ce
3cd27aa
7935c91
3626ca2
36af4d8
ed50c26
3ec1fc6
3d9914b
d6eb04b
6f41d58
68bcaa9
0f295d5
a2e9d49
6594d79
b06f6e9
3e03f27
fef35be
d191233
ebfb048
e97ed9c
08f9abd
be76be9
d7c50ca
319a050
4ff0737
a5aca85
4597fb1
1ac0592
4f791ed
439a883
94b1798
d1ef9ab
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sigh... let's hope the unused functions get indeed thrown away by the linker. If the Emscripten binary size inflates a lot, we can reconsider.
One option would be to have the Encoder and Transcoder independent, but that only moves the problem one level up, to any binary that links to both of them (i.e., the
BasisImageConverterTest
). So that won't really help.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we turned BasisUniversal::{Encoder, Decoder} into a static library, would there be a way to make this work? Or is that bad style for a find module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK throwing away duplicates from static libraries generally works only at the object file level -- i.e., if there would be
zstdenclib.c
andzstddeclib.c
and thenBasisUniversalTranscoder.a
would containzstddeclib.o
whileBasisUniversalEncoder.a
having bothzstdenclib.o
andzstddeclib.o
, then linking both to a final executable would make the linker pick only one of the two*.o
copies. (And I think that could maybe work even without going through static libraries, if the*.o
files get directly put into the final executable the linker could prune duplicates as well.)But if
zstddeclib.o
contains symbols that are also inzstd.o
, I don't think linkers can do much... except if those would be marked as weak, which is not a thing on Windows...Another idea I had was to build & bundle our own
zstdenclib.c
using the same scriptzstd.c
was made with, except for leaving out the stuff that's already inzstddeclib.c
. But at that point I feel it's a much better option to just depend on the real zstd library instead, like i said below. That way integrating upstream bugfixes and improvements is also much easier, compared to relying on a bundled file that might just get upgraded never again.But ... leave that for later. Unless @Squareys sees it's really making wasm binaries twice as huge, I don't think it's a priority :)
This file was deleted.