Git Product home page Git Product logo

Comments (15)

rillian avatar rillian commented on August 26, 2024 1

Everyone happy with the code now?

from vorbis.

eliasdaler avatar eliasdaler commented on August 26, 2024

Actually, after looking into it more, it seems like the issue is that vorbis expects to find already built libogg and doesn't allow you to do something like this:

add_subdirectory(ogg) # xiph/ogg
add_subdirectory(vorbis) # this repo

Is it possible to somehow make find_package(Ogg REQUIRED) in main CMakeLists.txt optional?

from vorbis.

evpobr avatar evpobr commented on August 26, 2024

If FindOgg.cmake is not found, CMake with switch to package config mode. Latest Ogg provides config file, it is generated in build directory (usually build). You can use <Package-Name>_Dir to set config directory, in your case it is Ogg_Dir.

Try:

cmake .. -DOgg_Dir=<path-to-ogg-directory>/<build-directory>

from vorbis.

DatCaptainHorse avatar DatCaptainHorse commented on August 26, 2024

I've been trying to get this messy OGG-Vorbis CMake conga working for multiple hours now, it's 3am and I will voice my opinions in semi-contained anger.

[FindOgg.cmake - line:85] Do NOT create a OGG library in Vorbis! WHY. It's up to OGG's CMakeLists to make sure Ogg::ogg is available and usable. Do you go to a SHIPYARD to build a CAR?

[FindOgg.cmake - line:56] Looking for a system library, okay. But if you are trying to build using add_subdirectory(), enjoy your torment! WHY do you need OGG immediately? You COULD ATLEAST add an if/else statement in the CMakeLists checking if we are building as subdirectory if for some reason having OGG there right now is so important.

Easy fix? Delete FindOgg.cmake and set Ogg_DIR pointing to where OGG is being built to, but hey that opens another can of worms in OGG's side :-)

I could also list issues from OGG's CMakeLists side but atleast I am able differentiate it from Vorbis and not put in in the issue here. Spoiler: Exports are unusable

If I feel particularly codey after a good nights sleep, I may try to fix the CMake conga or in worse case rewrite it. I'm tired of having to do set(OGG_HACK_DIR) things to make this work.

from vorbis.

evpobr avatar evpobr commented on August 26, 2024

Find modules are correct (more or less), @CaptainHorse.

The truth is that Ogg and Vorbis ะกMake projects are not designed to be used as subprojects. Sorry for this.

But the good news is that the fix is trivial. I guess we need to use ALIAS libraries:

Can you test https://gitlab.xiph.org/evpobr/ogg/-/commit/163c5d6aab4c6a25df110e0f80f6e7dc6a0360be.

This will expose Ogg::ogg target if you add Ogg CMake project with add_subdirectory.

If it works i will create pull requests for Ogg and Vorbis.

from vorbis.

evpobr avatar evpobr commented on August 26, 2024

And Vorbis: https://gitlab.xiph.org/evpobr/vorbis/-/commit/e1b2fabbe3624c0d0dd3226f2af48f5ef582744b

from vorbis.

evpobr avatar evpobr commented on August 26, 2024

So i've tested it.

Directory layout:

cmake_test/
cmake_test/ogg
cmake_test/vorbis

cmake_minimum_required(VERSION 3.13..3.18)

project(cmake_test VERSION 0.1.0)

add_subdirectory(ogg)
add_subdirectory(vorbis)

add_executable(cmake_test cmake_test.c)
target_link_libraries(cmake_test PRIVATE Vorbis::vorbisenc)

cmake_test.c:

#include <vorbis/vorbisenc.h>

#include <stdlib.h>
#include <stdio.h>

int main(void)
{
    vorbis_info vi = {0};
    vorbis_encode_setup_init(&vi);

    return 0;
}

Output:

[build] [2/2 100% :: 0.874] cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E vs_link_exe --intdir=CMakeFiles\cmake_test.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100183~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100183~1.0\x64\mt.exe --manifests  -- C:\PROGRA~2\MICROS~1\2019\COMMUN~1\VC\Tools\MSVC\1427~1.291\bin\Hostx64\x64\link.exe  CMakeFiles\cmake_test.dir\cmake_test.c.obj  /out:cmake_test.exe /implib:cmake_test.lib /pdb:cmake_test.pdb /version:0.0 /machine:x64 /debug /INCREMENTAL /subsystem:console  vorbis\lib\vorbisenc.lib  vorbis\lib\vorbis.lib  ogg\ogg.lib  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd ."

But Ogg dependency is still problematic. The only solution i've found is to set Ogg_DIR to Ogg's build directory:

cmake .. -DOgg_DIR="<cmake-test-directory>/ogg/build"

Now everything works for me as expected.

from vorbis.

DatCaptainHorse avatar DatCaptainHorse commented on August 26, 2024

Hail Omnissiah, that almost works

But Ogg dependency is still problematic.

After deleting line 65 find_package(Ogg REQUIRED) in the main CMakeLists things work. Removing REQUIRED and using some checks whether Ogg was found can b used if the line is super necessary.

Thank you for tackling this issue, I very much hope this gets to the main branch after some fixes to the Ogg dependency.

from vorbis.

evpobr avatar evpobr commented on August 26, 2024

I'm pretty sure you are using add_subdirectory incorrectly. This function is designed to organize your project into subprojects. In this case, everything looks logical and it becomes clear why find_package does not work.

If you try a normal package manager like Vcpkg, you will see that it downloads and build dependencies, installs them in a specific location, and then configures search paths so that find_package call in your project finds them.

But OK, fix for you is simple. Remove REQUIRED is not good, because Ogg dependency is, hmm, required. I guess the correct way is to wrap find_package(Ogg) and find_dependency(Ogg) calls in:

if(TARGET Ogg::ogg)
    ...
endif()

Check it please if you have time.

from vorbis.

evpobr avatar evpobr commented on August 26, 2024

https://gitlab.xiph.org/xiph/ogg/-/merge_requests/6

https://gitlab.xiph.org/xiph/vorbis/-/merge_requests/14

from vorbis.

rillian avatar rillian commented on August 26, 2024

I merged @evpobr's changes, if that makes it easier to test.

from vorbis.

DatCaptainHorse avatar DatCaptainHorse commented on August 26, 2024

These changes are working ๐Ÿ‘

Yeah as @evpobr thought I may not be using add_subdirectory in the most correct way, but I've found it to be the most simple and minimal setup for anyone cloning and setting up my engine project.

Currently working on application using the engine and it's add_subdirectories basically going like this:

  • Application

    • Any application specific Git submodules (Dear-ImGui, nlohmann_json..)

    • Engine

      • Git submodules (Ogg, Vorbis, Opus, Opusfile..)

        • Any submodule dependencies (googletest, zlib..)
      • Engine modules (Audio, Graphics, Compute, Maths, Networking..)

Setting up clean project is as easy as cloning it and doing git submodule update --init --remote --recursive (unless you want to build Python scripting module, CPython does not make anything easy, no CMake support, source and config files spread everywhere.. It's the only dependency I wish to keep pre-built blobs of due to it being a pain to compile with correct settings..)

from vorbis.

eliasdaler avatar eliasdaler commented on August 26, 2024

add_subdirectory is not only used for adding your projects into CMake build. See FetchContent` documentation - add_subdirectory is done to download and configure all dependencies at configuration time without any pre-build step.
Also thanks for the changes - it looks like this will solve the issue, I'll test it soon.

from vorbis.

DatCaptainHorse avatar DatCaptainHorse commented on August 26, 2024

Yes

from vorbis.

rillian avatar rillian commented on August 26, 2024

Ok, great. Treating this as resolved.

from vorbis.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.