Refactor library structure #550

legerch · 2025-12-29T10:43:47Z

Hi everyone,
I made some big changes to the library, not in the code itself but more on the structure and build process.
I propose it as a draft PR because I don't know if such a refactor is welcome or not :) But I think it could ease the build and integration process !

Why the refactor

The library can be "hard" to build, we have many depenpencies and a lot of CMake instructions are let to the app using the library, so if your not developping the library, it can be hard to follow.

Now users who want to use the library only have to:

Build the library by including it in their root CMakeLists file:

add_subdirectory(qtavplayer) # Or if library is put in a folder "dependencies": add_subdirectory(dependencies/qtavplayer)

Link the library to the application/library using it:

target_link_libraries(${PROJECT_NAME} PRIVATE qtavplayer)

What has been changed:

Separate public API from private sources. The library already used PIMPL idiom, so most of the work was already done, now:
- Public API files are in lib/include/qtavplayer. Users still use the api like this: include "qtavplayer/qavplayer.h
- Private headers and sources files are in lib/src
- Public classes are now properly exported via QTAVPLAYER_EXPORT macro
Rework CMake build instructions:
- Add library metadata informations (author, project page, version, etc...)
- Facilitate dependencies management according to each options and/or platforms :
  - Easier to know which package are needed
  - Responsibility of linking dependencies is no longer on application using the library
- Allow to choose platforms where to use HW acceleration via CMake option()
- Also provide options allowing to choose whether or not examples and tests must be build when building the library
Provide multiple library macros (like semver macros) inside qtavplayerglobal.h, which is automatically generated when CMake is run (template file is qtavplayerglobal.h.in)
README file now include a tips to know which development package are needed (especially useful under Linux OSes)
Example video links of "Big Buck Bunny" were dead (404 error), replace those with a valid link

What has been verified:

This refactor has been tested for:

Linux (Ubuntu 24.04):
- Software decoding
- HW va-drm acceleration
- HW va-x11 acceleration
- HW va-vdpau acceleration
Windows (Windows 10 and Windows 11)
- Software decoding
- HW d3d11 acceleration
MacOS (macOS Sequoia 15.4)
- Software decoding
- HW Videotoolbox acceleration

Under the Qt versions:

Qt 5.15.2
Qt 6.8.3
Qt 6.9.3

Why this PR is proposed as a draft

I don't have access to some platforms, so:
- Android has not been tested
- iOS has not been tested
Only tests tests/auto/integration/qavplayer has been run, tests tests/auto/integration/qavdemuxer cannot be built for now since it use private headers (I wanted to know first if this PR was welcome before allowing linking to private API)
File related to CI/CD pipeline has not been modified (so it is broken for now)

Have a nice day ! And thank you for the time invested to make this great library !

Separate public API from private sources based on the "Pitchfork convention". More details here : - https://github.com/vector-of-bool/pitchfork

Will be used to manage library version, exported symbols, etc...

This will allow to build tests and examples the same way that external application using "QtAvPlayer" library. Examples will no longer include parent directory

Previous link is dead (404 error), replace with valid one

valbok · 2025-12-29T10:56:55Z

examples/qml_video/main.cpp

 * Free Qt Media Player based on FFmpeg.                       *
 ***************************************************************/

-#include <QtAVPlayer/qavplayer.h>


fyi, QtAVPlayer was used because legacy from Qt modules, f.e. #include <QtMultimedia/QMediaPlayer>

QtAVPlayer - the name of the module.
QAVPlayer - the name of the player.

Also the code structure was based on Qt Modules too, f.e. https://github.com/qt/qtmultimedia/tree/dev/src/multimedia which contains public and private headers in the same dir together with cpp files.

Yes, Qt are using a lot of include with mixed upper/lower cases, I can revert that if you want. This is because I always personally struggle to include files because I can't remember where the uppercase letters are... (and hard to type).
When all use the same, you don't have to look in the source 😄

<QtAVPlayer/qavplayer.h> is suggested way to be used inside the Qt community, so would prefer to keep it,

the header file is always lowercase but with *.h, but during the installing Qt created a link

/usr/include/x86_64-linux-gnu/qt5/QtMultimedia$ cat QMediaPlayer #include "qmediaplayer.h"

So suggested way is #include <QtMultimedia/QMediaPlayer>

We could also try to have upper case but without .h

valbok · 2025-12-29T11:15:19Z

lib/src/qavaudiocodec_p.h

 // We mean it.
 //

 #include "qavframecodec_p.h"


Also interesting question about private headers: these days when it was decided to provide the private headers, it was planned to deploy the library separately and independently. After it was decided to bundly the library into the user's project directly, no big reason in private headers anymore.
Since the using of the library is always deployed together with the library.

valbok · 2025-12-29T11:16:52Z

lib/include/qtavplayer/qtavplayerglobal.h.in

+ * #endif
+ * \endcode
+ */
+#define QTAVPLAYER_VERSION_ENCODE(major, minor, path)    ((major) * 10000 + (minor) * 100 + (patch))


The same question for versioning, and releasing of the library - since the library is always bundled, it is always described by the commit in master branch which must be always the latest and the stable version, unstable commits should not be merged into master.

So the commit already contains the version/date and describes the functionality/features.

Maybe the semver indeed is not adapted for this library but this kind of macro can help for example if the library has a "big change", the application using it can adapt the code to easily switch version/commit

Usually commit or tag is used to define the version of the external library inside the project. So usually the apps don't struggle on the versions, since it is git repo and need to know only the tag/commit/branch.

But you say the application code could use versions from the library? We hoped to avoid this and just always use the needed version without macro nightmare as we do inside the library today.

valbok · 2025-12-29T11:20:58Z

lib/include/qtavplayer/qtavplayerglobal.h.in

+ * Library management
+ *********************************/
+#if defined(QTAVPLAYER_LIBRARY_BUILD)
+#  define QTAVPLAYER_EXPORT Q_DECL_EXPORT


On the beginning it was a reason to have exports, since the library was deployed independently as shared object or dll, but when it was started to link into the user's app directly, afair these exports started to be useless.

Are you planning to deploy qtavplayer as library and link dynamically?

Can I ask the reason why the library has stopped the option to be a shared library and let the users bundle it into their app/library ?
To answer your question, yes I use it as a shared library because it's easier to maintain if multiple projects are using it (the main issue with current build process was the dependencies and the fact that generated object files where not with the rest of application build objects).

Was it due to technical issues the fact to stop building it as a shared library and let users bundle it ?

We had tons of issues related how to build the library and how to deploy and how to test on CI, so the easiest way we invented it was always statically build to the application and forget about external dll, exports, linking issues, how to deploy etc. Avoided many pain.

And did not see any issues with rebuilding the code for each app, f.e. how google test was used, when needed to rebuild it on each project.

It can be an issue if, for example, the library is used in a library AND an application, we have two "same" targets. We had in fact the same issue with gtest, two different sub-library including it as is and building it (was a nightmare !), this was solved by using vcpkg which allow to use gtest as a shared library 😅

(I do not try to convince you, if you prefer keeping the library as bundle lib, no issue. That why I have proposed the PR as a draft 😄)

if you have a need in shared library, maybe possible to provide a way how to do that, but to keep current approach to statically link to the app by default. Do you see any ways how to allow shared lib there?

Btw, qmake must be still supported. And current behavior should be kept by default.

But for cmake we could provide native BUILD_SHARED_LIBS and make some magic in cmake files - BUT the current CMakeLists.txt is added only for ref, since usually the applications rewrite it for their needs where they could build shared lib and link to the app.

Building the lib statically by default can be done but to allow an option for shared library, it is needed to separate public API headers from private headers/source code (otherwise, linking issues will arise for sure) and marking public classes as symbols to export, that what I have done in this PR (only includes and export where set in source files, no code was modified except the bad link)

I don't think it's possible to keep current structure and only manage it in CMake (or at least, I don't know how😅)

legerch and others added 14 commits December 17, 2025 14:59

Move files to separate public API and private sources

bf3e0f6

Separate public API from private sources based on the "Pitchfork convention". More details here : - https://github.com/vector-of-bool/pitchfork

Add library custom macros

9537287

Will be used to manage library version, exported symbols, etc...

Rewrite CMake file to add library metadatas infos and proper linking

4ae4b8f

Use public API includes

80557e5

Properly export public API classes

6498e1f

Move library source files into dedicated folder

72743a5

This will allow to build tests and examples the same way that external application using "QtAvPlayer" library. Examples will no longer include parent directory

Add example build support

e2b2cb0

Update "bunny" video to a valid link

f5c6c05

Previous link is dead (404 error), replace with valid one

Add build support of QAVPlayer tests

dc4bdfc

Fix HW acceleration for Linux VA-DRM implementation

a377c0b

Fix HW acceleration for Linux VA-X11 implementation

d99142c

Fix HW acceleration for Linux VDPAU implementation

d760546

Fix linking issues

44c2207

Update README with new build instructions

e528d6b

valbok reviewed Dec 29, 2025

View reviewed changes

Refactor library structure #550

Are you sure you want to change the base?

Refactor library structure #550

Uh oh!

Conversation

legerch commented Dec 29, 2025

Why the refactor

What has been changed:

What has been verified:

Why this PR is proposed as a draft

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

valbok Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

legerch Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

valbok Dec 29, 2025 •

edited

Loading

legerch Dec 29, 2025 •

edited

Loading