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.

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

Build error when used as library in another project #17

Closed
panosdim opened this issue Jan 29, 2021 · 14 comments
Closed

Build error when used as library in another project #17

panosdim opened this issue Jan 29, 2021 · 14 comments

Comments

@panosdim
Copy link

Hi, I am trying to include tagparser library as a dependency in my project using the following in my CMakelist.txt file.

set(CMAKE_INSTALL_RPATH_USE_LINK_PATH true)
add_subdirectory(libs/cpp-utilities-5.10.0 c++utilities)
list(APPEND CMAKE_MODULE_PATH ${CPP_UTILITIES_SOURCE_DIR}/cmake/modules)
link_directories(${CPP_UTILITIES_BINARY_DIR})

add_subdirectory(libs/tagparser-9.4.0 tagparser)
link_directories(${TAG_PARSER_BINARY_DIR})

However, when I try to build it I get the following error:

Scanning dependencies of target tagparser
[ 47%] Building CXX object tagparser/CMakeFiles/tagparser.dir/aac/aaccodebook.cpp.o
In file included from /mnt/c/Users/padi/CLionProjects/autotag-cpp/libs/tagparser-9.4.0/aac/./aaccodebook.h:6,
                 from /mnt/c/Users/padi/CLionProjects/autotag-cpp/libs/tagparser-9.4.0/aac/aaccodebook.cpp:1:
/mnt/c/Users/padi/CLionProjects/autotag-cpp/libs/tagparser-9.4.0/aac/./../global.h:7:10: fatal error: c++utilities/application/global.h: No such file or directory
    7 | #include <c++utilities/application/global.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [tagparser/CMakeFiles/tagparser.dir/build.make:82: tagparser/CMakeFiles/tagparser.dir/aac/aaccodebook.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:1608: tagparser/CMakeFiles/tagparser.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
@Martchus
Copy link
Owner

Building these libraries as part of a bigger project is actually supported. In fact I'm doing that all the time. However, there is one caveat: The names of the Git checkout directories need to match the name of the header directory which would be installed into the include directory. In case of the cpp-utilities repository that directory is c++utilities. Sorry for being inconsistent here. I have just not put much thought into this when setting up the repository.

Long story short: The name of the Git directory must be c++utilities like these commands suggest: https://github.com/Martchus/tageditor#building-this-straight

@Martchus
Copy link
Owner

By the way, there's also the TARGET_INCLUDE_DIRECTORY_BUILD_INTERFACE variable you could set before including the subdirectory. Then it'll add this directory as include directory at build time. This directory must then contain a directory (or symlink to a directory) called c++utilities with all the headers inside. If not set, this variable defaults to "${CMAKE_CURRENT_SOURCE_DIR}/.." causing the caveat I've described in the previous comment.

@panosdim
Copy link
Author

When I renamed the directories to c++utilities and tagparser and now I get another error:

add_subdirectory(libs/c++utilities c++utilities)
list(APPEND CMAKE_MODULE_PATH ${CPP_UTILITIES_SOURCE_DIR}/cmake/modules)
link_directories(${CPP_UTILITIES_BINARY_DIR})

add_subdirectory(libs/tagparser tagparser)
link_directories(${TAG_PARSER_BINARY_DIR})
/mnt/c/Users/padi/CLionProjects/autotag-cpp/src/movie.cpp:11:10: fatal error: tagparser/mediafileinfo.h: No such file or directory
   11 | #include <tagparser/mediafileinfo.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [CMakeFiles/autotag.dir/build.make:108: CMakeFiles/autotag.dir/src/movie.cpp.o] Error 1

@Martchus
Copy link
Owner

Are you sure that the target movie.cpp belongs to links against the targparser target? Note that you really have to link to the tagparser target and not just against the library. Otherwise CMake won't pull in additional flags e.g. for the include directories. I guess that you'll still have to use find_package(tagparser SOME_VERSION REQUIRED) in the other sub project where you use the library to make the target available. Otherwise it might think that tagparser is just literally the path to the library on disk. (And yes, using find_package on a sub project which is part of the same build is possible. Just make sure you add the sub directories for c++utilities/tagparser before the sub project of your sorce code using the libraries.)

@panosdim
Copy link
Author

panosdim commented Feb 1, 2021

You are right I forgot to put it in the target_link_libraries. With the following, it is building correctly:

add_subdirectory(libs/c++utilities c++utilities)
list(APPEND CMAKE_MODULE_PATH ${CPP_UTILITIES_SOURCE_DIR}/cmake/modules)
link_directories(${CPP_UTILITIES_BINARY_DIR})

add_subdirectory(libs/tagparser tagparser)
link_directories(${TAG_PARSER_BINARY_DIR})

# find tagparser
find_package(tagparser${CONFIGURATION_PACKAGE_SUFFIX} 9.4.0 REQUIRED)
use_tag_parser()

target_link_libraries(autotag PRIVATE glog::glog restclient-cpp nlohmann_json::nlohmann_json ${TAG_PARSER_LIB})

@panosdim panosdim closed this as completed Feb 1, 2021
@Martchus
Copy link
Owner

Martchus commented Feb 1, 2021

Note that you don't have to use use_tag_parser(). I'm using this function as it helps to reduce boilerplate code within my projects but it requires the projects to follow a certain structure. I suppose in your case this function has no effect at all. Of course my way of structuring projects is not incurred to any other users of the library so doing the target_link_libraries call is just fine. By the way, it is also possible to add the subdirectories conditionally so the user can decide whether to use the bundled version of the library or the version found on the system (e.g. something like this https://github.com/Martchus/reflective-rapidjson/blob/master/CMakeLists.txt#L22).

@panosdim
Copy link
Author

panosdim commented Feb 1, 2021

Again you are right. Even after removing use_tag_parser() everything is compiling ok.
However, I have another problem when I try to retrieve the attachments mime type I get an error of Member access into incomplete type 'TagParser::AbstractAttachment'. Do you have any idea why I am getting this error?

fileInfo.setPath(movieInfo.path);
        fileInfo.open();
        fileInfo.parseContainerFormat(diag);
        auto container = fileInfo.container();
        fileInfo.parseAttachments(diag);

        if (fileInfo.attachmentsParsingStatus() == TagParser::ParsingStatus::Ok && container) {
            for (size_t i = 0, count = container->attachmentCount(); i < count; ++i) {
                auto attachment = container->attachment(i);
                LOG(ERROR) << "Attachments count " << count;
                LOG(ERROR) << "Mime type " << attachment->mimeType();
            }
        }

Nevermind, I found the problem was that I was missing one header file #include <tagparser/abstractattachment.h>

@Martchus
Copy link
Owner

Martchus commented Feb 1, 2021

Likely you've just forgotten #include <tagparser/abstractattachment.h>. This one isn't pulled in by mediafileinfo.h because it is not required to use MediaFileInfo itself.

@Martchus
Copy link
Owner

Martchus commented Feb 1, 2021

Ah, you've just updated your comment. By the way, I'm already at version 10.0.0 on master which this time unfortunately also contains some source-breaking changes because I want to move the library (slightly over time) to use newer C++ features. You might want to use the development branch directly.

By the way, regarding accessing the cover within Matroska files there's still an open issue: #13
(This just means you're using the right approach by accessing the attachments directly but I might provide a further API to streamline the access with other formats where the cover is part of the tag.)

@panosdim
Copy link
Author

panosdim commented Feb 1, 2021

Is there any guide on how to add, remove attachments from a Matroska file?

@Martchus
Copy link
Owner

Martchus commented Feb 1, 2021

For removing, there's AbstractAttachment::setIgnored() and for adding AbstractContainer::createAttachment(). To actually apply it, call MediaFileInfo::applyChanges(). And no, there's no guide. I usually just browse the headers to remind myself how things are done. I could extend the README at some point.

@panosdim
Copy link
Author

panosdim commented Feb 1, 2021

I am trying to add an attachment to an mkv with the following code

try {
        fileInfo.setPath(movieInfo.path);
        fileInfo.open();
        fileInfo.parseContainerFormat(diag);
        auto container = fileInfo.container();
        fileInfo.parseAttachments(diag);

        if (fileInfo.attachmentsParsingStatus() == TagParser::ParsingStatus::Ok && container) {
//            for (size_t i = 0, count = container->attachmentCount(); i < count; ++i) {
//                auto attachment = container->attachment(i);
//                if (attachment->mimeType() == "image/jpeg") {
//                    attachment->setIgnored(true);
//                }
//            }
            auto attachment = container->createAttachment();
            attachment->setName("cover.jpg");
            attachment->setFile(cover, diag);

            // create progress
            TagParser::AbortableProgressFeedback progress(logNextStep, logStepPercentage);

            // apply changes to the file on disk
            // (might throw exception derived from TagParser::Failure for fatal processing error or ios_base::failure for IO errors)
            fileInfo.applyChanges(diag, progress);
        }
    } catch (const TagParser::Failure &error) {
        LOG(ERROR) << "A parsing failure occurred when reading the file " << movieInfo.path;
        LOG(ERROR) << error.what();
    } catch (const std::ios_base::failure &) {
        LOG(ERROR) << "An IO failure occurred when reading the file " << movieInfo.path;
    }

But when I try to applyChanges i get the following error:
E0201 15:50:13.711947 18898 movie.cpp:126] A parsing failure occurred when reading the file /tmp/Test.mkv
E0201 15:50:13.711992 18898 movie.cpp:127] data to be parsed or to be made seems to be invalid

Is there any other way to find out why the function is failing?

@Martchus
Copy link
Owner

Martchus commented Feb 1, 2021

The diag object is supposed to contain something useful.

@panosdim
Copy link
Author

panosdim commented Feb 1, 2021

I found the problem. I was calling only parseContainerFormat and parseAttachments. When I used instead of the parseEverything function everything works ok.

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

No branches or pull requests

2 participants