fzwoch / obs-vaapi

OBS Studio VAAPI support via GStreamer
GNU General Public License v2.0
118 stars 3 forks source link

Issues trying to compile #2

Closed GloriousEggroll closed 2 years ago

GloriousEggroll commented 2 years ago

I'm trying to compile this as a standalone plugin, hitting some issues. My meson inside my rpm spec sheet is very simple:

%meson --buildtype=release \
  %{nil}
%{meson_build}

I'm using the same build requirement packages as the obs-gstreamer package (which still builds successfully), with only pciutils-devel added for the device name additions we discussed.

First I hit this issue:

../obs-vaapi.c:23:10: fatal error: obs-module.h: No such file or directory
   23 | #include <obs-module.h>

Fixed by changing to :

#include <obs/obs-module.h>

But now I'm hitting this issue and unable to resolve it:

../obs-vaapi.c: In function ‘create’:
../obs-vaapi.c:115:14: error: ‘VIDEO_FORMAT_I010’ undeclared (first use in this function); did you mean ‘VIDEO_FORMAT_I420’?
  115 |         case VIDEO_FORMAT_I010:
      |              ^~~~~~~~~~~~~~~~~
      |              VIDEO_FORMAT_I420
../obs-vaapi.c:115:14: note: each undeclared identifier is reported only once for each function it appears in
../obs-vaapi.c:117:56: error: ‘VIDEO_FORMAT_P010’ undeclared (first use in this function); did you mean ‘VIDEO_FORMAT_Y800’?
  117 |                                                        VIDEO_FORMAT_P010);
      |                                                        ^~~~~~~~~~~~~~~~~
      |                                                        VIDEO_FORMAT_Y800
../obs-vaapi.c:138:14: error: ‘VIDEO_CS_2100_PQ’ undeclared (first use in this function); did you mean ‘VIDEO_CS_709’?
  138 |         case VIDEO_CS_2100_PQ:
      |              ^~~~~~~~~~~~~~~~
      |              VIDEO_CS_709
../obs-vaapi.c:142:14: error: ‘VIDEO_CS_2100_HLG’ undeclared (first use in this function)
  142 |         case VIDEO_CS_2100_HLG:
      |              ^~~~~~~~~~~~~~~~~
../obs-vaapi.c: In function ‘encode’:
../obs-vaapi.c:335:41: error: ‘VIDEO_FORMAT_I010’ undeclared (first use in this function); did you mean ‘VIDEO_FORMAT_I420’?
  335 |         if (video_info.output_format == VIDEO_FORMAT_I010 ||
      |                                         ^~~~~~~~~~~~~~~~~
      |                                         VIDEO_FORMAT_I420
../obs-vaapi.c:336:41: error: ‘VIDEO_FORMAT_P010’ undeclared (first use in this function); did you mean ‘VIDEO_FORMAT_Y800’?
  336 |             video_info.output_format == VIDEO_FORMAT_P010) {
      |                                         ^~~~~~~~~~~~~~~~~
      |                                         VIDEO_FORMAT_Y800
ninja: build stopped: subcommand failed.

Additional note: (this may need it's own Issue open, depends what you prefer)

It may be beneficial here to convert from meson to cmake so that the plugin can be built as part of OBS. I have several other plugins that I'm able to build as part of OBS which use cmake simply by doing this in my obs build:

# Prepare plugins/obs-vkcapture
git clone --recurse-submodules https://github.com/nowrep/obs-vkcapture plugins/obs-vkcapture
cd plugins/obs-vkcapture
sed -i 's/install_obs_plugin_with_data/setup_plugin_target/g' CMakeLists.txt
cd ../../

# Prepare plugins/streamfx
git clone --recurse-submodules https://github.com/Xaymar/obs-StreamFX plugins/streamfx
cd plugins/streamfx
git checkout 1d1e646a2319d45ff84e31b1ab0da4a5395d845b
cd ../../

# Prepare plugins/obs-source-record
git clone --recurse-submodules https://github.com/exeldro/obs-source-record plugins/obs-source-record
cd plugins/obs-source-record
sed -i 's/install_obs_plugin_with_data/setup_plugin_target/g' CMakeLists.txt
cd ../../

Then adding them to OBS's existing plugin list:

diff --git a/plugins/CMakeLists.txt b/plugins/CMakeLists.txt
index d20bce142..febeff6b7 100644
--- a/plugins/CMakeLists.txt
+++ b/plugins/CMakeLists.txt
@@ -60,6 +60,9 @@ elseif(OS_LINUX)
   add_subdirectory(vlc-video)
   add_subdirectory(sndio)
   add_subdirectory(obs-vst)
+  add_subdirectory(streamfx)
+  add_subdirectory(obs-vkcapture)
+  add_subdirectory(obs-source-record)

   check_obs_browser()
 elseif(OS_FREEBSD)
eli-schwartz commented 2 years ago

I have several other plugins that I'm able to build as part of OBS which use cmake simply by doing this in my obs build:

Seems a bit odd to make source code modifications to OBS just to make it build a plugin that is presumably supposed to compile just fine on its own. :eyes:

This makes it hard/impossible to compile separate projects separately, upgrade one without the other, correctly mark the rpm name/version/upstream url for the plugin...

GloriousEggroll commented 2 years ago

I have several other plugins that I'm able to build as part of OBS which use cmake simply by doing this in my obs build:

Seems a bit odd to make source code modifications to OBS just to make it build a plugin that is presumably supposed to compile just fine on its own. eyes

This makes it hard/impossible to compile separate projects separately, upgrade one without the other, correctly mark the rpm name/version/upstream url for the plugin...

Seems you may be a bit confused by my post.

I would like to compile the plugin independently, which is great. That is the starting point.

I would -also- like to be able to compile it as part of OBS as a path-moving-foward notion in the event that it should supercede the current FFMPEG VAAPI plugin already in OBS.

In my OBS build I am not modifying any OBS code other than adding plugins to the plugins list. My git clones are literally just dropping each plugin's code into obs's plugin directory. This is actually common practice for things such as the obs browser and other plugins that are pending being merged into obs. If you read the StreamFX documentation it also provides options for compiling StreamFX standalone -or- as part of OBS. Nothing new here.

fzwoch commented 2 years ago

I tested with OBS 28.0 RC1 at the moment.

I know that have been some issues with the pkg-config file in 27.x releases, so perhaps that is the reason obs-module.h is not found.

The defines not being found is a similar issue. It checkes for the new 10 bit formats in OBS 28.0 as it should be possible to record in 10 bit too.

Guess I will have to look atsomethings to make it compile with older OBS headers too,

GloriousEggroll commented 2 years ago

I tested with OBS 28.0 RC1 at the moment.

I know that have been some issues with the pkg-config file in 27.x releases, so perhaps that is the reason obs-module.h is not found.

The defines not being found is a similar issue. It checkes for the new 10 bit formats in OBS 28.0 as it should be possible to record in 10 bit too.

Guess I will have to look atsomethings to make it compile with older OBS headers too,

My last OBS build was from a few days ago (f5be6f5fdd2fe34b18518ecd38030f0768845688) -- which is the tagged rc1 build so it should be there afaik :/. As I noted the obs include was found/resolved once I added obs/ infront of it (thats how its done in obs-gstreamer), but i dont know why the second error is occurring

fzwoch commented 2 years ago

How did you install your OBS? It is important that it installs the header files / pkg-config file in the regular locations. Perhaps instead meson is finding something from another install/package?

GloriousEggroll commented 2 years ago

How did you install your OBS? It is important that it installs the header files / pkg-config file in the regular locations. Perhaps instead meson is finding something from another install/package?

It's a cloned copy of rpmfusion's obs build spec sheet. nothing abnormal there. obs-studio-devel package provides the headers in /usr/include/obs/

eli-schwartz commented 2 years ago

I know that have been some issues with the pkg-config file in 27.x releases, so perhaps that is the reason obs-module.h is not found.

Since obsproject/obs-studio@1fd7770548f287ad47260360e00d11e16fb40b1f the libobs.pc dependency sets /usr/include/obs as the include directory. Previous versions of libobs.pc were apparently broken, as they set /usr/include and installed headers to obs/.h but since at least 2017 the documentation states to include as `<.h>` without obs/

This is an obs-studio bug that apparently... no one ever noticed? Ouch.

fzwoch commented 2 years ago

For me video_io.h:74 has these:

    /* planar 4:2:0 format, 10 bpp */
    VIDEO_FORMAT_I010, /* three-plane */
    VIDEO_FORMAT_P010, /* two-plane, luma and packed chroma */

Can you check if you have that too?

fzwoch commented 2 years ago

I know that have been some issues with the pkg-config file in 27.x releases, so perhaps that is the reason obs-module.h is not found.

Since obsproject/obs-studio@1fd7770 the libobs.pc dependency sets /usr/include/obs as the include directory. Previous versions of libobs.pc were apparently broken, as they set /usr/include and installed headers to obs/.h but since at least 2017 the documentation states to include as `<.h>` without obs/

This is an obs-studio bug that apparently... no one ever noticed? Ouch.

I think I know where that comes from. In OBS source tree's obs.h is found at libobs/obs.h so obs/obs.h is not helpful in in-tree builds. So regular builds and in-tree builds are kind of incompatible in that regard.

eli-schwartz commented 2 years ago

In my OBS build I am not modifying any OBS code other than adding plugins to the plugins list. My git clones are literally just dropping each plugin's code into obs's plugin directory. This is actually common practice for things such as the obs browser and other plugins that are pending being merged into obs.

It's uncommon practice outside of OBS, and I'd argue it's bad practice as a general software development thing... it's a violation of scope and not at all required for merging if it hasn't been merged, and not the most effective thing to optimize for.

Also, it's obviously a separate issue.

Seems you may be a bit confused by my post.

No, I think I understood quite well. :p

eli-schwartz commented 2 years ago

I think I know where that comes from. In OBS source tree's obs.h is found at libobs/obs.h so obs/obs.h is not helpful in in-tree builds. So regular builds and in-tree builds are kind of incompatible in that regard.

Hmm, but this really does not seem to be true as the documentation actually does say to use the same style either way.

I really just think this is broken. The best option, probably, is to do a configure check to see whether cc.has_header('obs.h', dependencies: libobs_dep) works, and if so, set a define for an if/else that selects which header style to use.

GloriousEggroll commented 2 years ago

In my OBS build I am not modifying any OBS code other than adding plugins to the plugins list. My git clones are literally just dropping each plugin's code into obs's plugin directory. This is actually common practice for things such as the obs browser and other plugins that are pending being merged into obs.

It's uncommon practice outside of OBS, and I'd argue it's bad practice as a general software development thing... it's a violation of scope and not at all required for merging if it hasn't been merged, and not the most effective thing to optimize for.

Also, it's obviously a separate issue.

Seems you may be a bit confused by my post.

No, I think I understood quite well. :p

The goal behind compiling it as part of OBS is per the original discussion here:

https://ideas.obsproject.com/posts/1868/replace-linux-ffmpeg-vaapi-plugin-with-new-gstreamer-vaapi

I very much understand the reasoning behind keeping a "standalone" plugin as standalone --IF it is to remain standalone. As I've stated already, the request to allow it to build as part of OBS is aligned with the discussion noted -- eventually merging this plugin into OBS as a replacement, not keeping it standalone.

fzwoch commented 2 years ago

I fully understand the desire for this kind of build.

Once the plugin turns out usable I may consider looking at that. TL;DR is that I hate writing CMake to the spine and writing meson for me is like almost fun in contrast. But I also haven't taken a look at OBS's CMake rewrite. It certainly is possible it probably should get its own issue in the tracker..