Skip to content

Conversation

wantehchang
Copy link
Contributor

No description provided.

@wantehchang
Copy link
Contributor Author

@farindk Dirk: Please review. Thanks!

@kmilos
Copy link
Contributor

kmilos commented Sep 5, 2025

@wantehchang I'm not sure about the newly shipped AOMConfig.cmake & friends...

E.g. there is set(AOM_LIBRARIES "aom;aom_static") to support legacy scripts, but why link to both targets simultaneously, seems inappropriate? Should only be "aom" if BUILD_SHARED_LIBS, "aom_static" if not? And I guess these should be "AOM::aom" and "AOM::aom_static"?

See e.g. https://github.com/libsdl-org/sdl2-compat/blob/main/SDL2Config.cmake.in

Also, it would seem the version is not bumped: set(PACKAGE_VERSION "3.12.0") in AOMConfigVersion.cmake

@wantehchang
Copy link
Contributor Author

@kmilos Miloš: Thanks for the report. Could you file a bug report in the libaom issue tracker? Thanks.

@jzern FYI.

@jzern
Copy link

jzern commented Sep 5, 2025

@kmilos Miloš: Thanks for the report. Could you file a bug report in the libaom issue tracker? Thanks.

@jzern FYI.

I sent a couple of fixes:

  1. https://aomedia-review.googlesource.com/c/aom/+/202781 (version)
  2. https://aomedia-review.googlesource.com/c/aom/+/202782 (AOM_STATIC_LIBRARIES)

@wantehchang
Copy link
Contributor Author

Miloš: Could you review the second patch (AOM_STATIC_LIBRARIES) by James? Thanks.

SoftCreatR pushed a commit to SoftCreatR/aom that referenced this pull request Sep 5, 2025
Rather than use `AOM_INSTALL_LIBS`, which may include both the static
and dynamic versions of the library (aom, aom_static) when
`BUILD_SHARED_LIBS=1`, check for the presence of `AOM::aom_static` and
set `AOM_STATIC_LIBRARIES` instead. When `BUILD_SHARED_LIBS=0`,
`AOM::aom` is a static library. `AOM_STATIC_LIBRARIES` is set to
`AOM::aom` in that case.

See:
strukturag/libheif#1592 (comment)

Bug: 441135035
Change-Id: I39ba77e6f075dfeb75c09ebaa0b20191a66a63cc
@kmilos
Copy link
Contributor

kmilos commented Sep 6, 2025

@wantehchang @jzern Thanks you both, looking much better indeed in 3.13.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants