AOMediaCodec / libavif

libavif - Library for encoding and decoding .avif files
Other
1.53k stars 195 forks source link

libavif.a should not be a combined archive library #2157

Closed wantehchang closed 2 months ago

wantehchang commented 4 months ago

This issue is blocking the libavif 1.1.0 release.

In https://github.com/AOMediaCodec/libavif/commit/12e066686892df1c8201cfb0d8d6c68ad248c872 the static library libavif.a became a combined archive library that contains not only libavif but also the libraries it depends on.

This changed the meaning of libavif.a. We should change libavif.a back to the standard meaning (containing only libavif itself) that most people expect. If we need to provide a combined archive library, it should have a different name, such as libavif_combined.a.

@fdintino Frankie: I seem to remember you agreed to make this change. Would you still be able to make this change?

fdintino commented 4 months ago

I believe it only combines the archives for dependencies that are built locally. A system static library is in principle portable, whereas ext/SVT-AV1/Bin/Release/libSvtAv1Enc.a is not. If this is not the desired behavior, I don't think it would be terribly difficult to put it behind some sort of FULL_STATIC_BUILD cmake option. Should I open a pull request to that effect?

wantehchang commented 4 months ago

Frankie: Thanks for the quick reply. I think it is best if libavif.a always has the standard meaning that people expect, containing only libavif itself. We can add a cmake option to produce an additional static library, say libavif_combined.a, that also includes the archives for dependencies.

I have a question about your comment. You said "A system static library is in principle portable". By "portable", did you mean position-independent code?

fdintino commented 4 months ago

I mean that you could distribute libavif.a and reasonably expect other users to be able to link against it. In an rpm/deb/apk for instance, where ABI compatibility can be enforced. (edit: pkg-config could also serve this purpose, if we had dependencies in the .pc file)

fdintino commented 4 months ago

If someone wanted to distribute a library for node or python that had libavif bindings, it would be convenient to be able to take advantage of the local dependency builds, but if those dependencies weren't combined in the archive then when you went to link libavif.a you'd also need to track down all of the static files for those dependencies, wherever they might have ended up as interim build products.

fdintino commented 4 months ago

What if it only libraries that were compiled in the build directory (in other words, not in the ext directory) are merged into libavif.a? That would ensure that any projects that depend on the past behavior and locations of the static libraries in the ext folder would continue to work. Whereas the directory structure for intermediate externalproject and fetchcontent artifacts inside the build directory is meant to be opaque. It's not reasonable to expect a user to hunt the static lib files down so that they can link against libavif.a.

wantehchang commented 2 months ago

@fdintino Frankie: I am sorry I forgot to reply. I recently found that when the BUILD_SHARED_LIBS cmake option is on, the libavif build system does not install any static library. This differs from the libaom build system (which I also work on); it installs not only the shared library but also the static library in that case.

I believe libavif system packages are built with the BUILD_SHARED_LIBS cmake option enabled. So I am now less worried whether libavif.a is libavif only or a merged archive library.

In https://github.com/AOMediaCodec/libavif/issues/2157#issuecomment-2103159531, you wrote:

I believe it only combines the archives for dependencies that are built locally.

I think this behavior is very reasonable. I verified it indeed works this way with simple experiments (one with a system-installed libaom.a, the other with a locally-built libaom.a).