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

Update/gstreamer suite rtsp #11

Open
wants to merge 6 commits into
base: update/gstreamer-suite
Choose a base branch
from

Conversation

fnadeau
Copy link

@fnadeau fnadeau commented Jan 10, 2025

Summary

Changes to recipe: gstreamer suit

Motivation

Add gst-rtsp-server subproject. This would close the subject on gstreamer-full for me since gst-rtsp-server was the main reason I had, rest was just extra.

Details

  • Bump version to latest
  • Bump glib requirement required by gst-rtsp-server
  • glib wouln't build on my machine due to sysprof, prevent auto detection
  • Initial support for gst-rtsp-server

This cause unresolve symbols since libsysprof is never linked against.
g_once_init_enter_pointer/g_once_init_leave_pointer are required and only present in glib starting with 2.80.
From release notes, only bug fixes.
@@ -228,7 +228,7 @@ def layout(self):
def requirements(self):
self.requires(f"gstreamer/{self.version}", transitive_headers=True, transitive_libs=True)
self.requires(f"gst-plugins-base/{self.version}", transitive_headers=True, transitive_libs=True)
self.requires("glib/2.78.3", transitive_headers=True, transitive_libs=True)
self.requires("glib/2.81.0", transitive_headers=True, transitive_libs=True)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep it at 2.78.3 for now. It's the version CCI recipes have currently converged on. I would like to bump it as well, but avoiding conflicts is more important.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's the case, gst-rtsp-server will have to wait. It needs glib >= 2.80 due to these two function
https://github.com/GNOME/glib/blob/main/glib/gthread.h#L246

In rtsp-server, used in define
https://github.com/GStreamer/gst-rtsp-server/blob/0b037e35e7ed3259ca05be748c382bc40e2cdd91/gst/rtsp-server/rtsp-session.h#L30
and used multiple places, example
https://github.com/GStreamer/gst-rtsp-server/blob/0b037e35e7ed3259ca05be748c382bc40e2cdd91/gst/rtsp-server/rtsp-session-pool.c#L205

If I revert commit 1229261, rebuild gstreamer and gst-plugins-base and gst-rstp-server I get this

gst-rtsp-server/1.24.11 (test package): Running CMake.build()
gst-rtsp-server/1.24.11 (test package): RUN: cmake --build "/home/redacted/src/conan-center-index/recipes/gst-rtsp-server/all/test_package/build/gcc-14-x86_64-gnu17-release" -- -j8
[ 50%] Building CXX object CMakeFiles/test_package.dir/test_package.cpp.o
[100%] Linking CXX executable test_package
/usr/bin/ld : /home/redacted/.conan2/p/b/gst-r24b7613cb3dab/p/lib/libgstrtspserver-1.0.a(rtsp-server.c.o) : dans la fonction « gst_rtsp_server_get_type » :
rtsp-server.c:(.text+0xa94) : référence indéfinie vers « g_once_init_enter_pointer »
/usr/bin/ld : rtsp-server.c:(.text+0xab4) : référence indéfinie vers « g_once_init_leave_pointer »
/usr/bin/ld : /home/redacted/.conan2/p/b/gst-r24b7613cb3dab/p/lib/libgstrtspserver-1.0.a(rtsp-session-pool.c.o) : dans la fonction « gst_rtsp_session_pool_get_type » :

-- 50 lines of output omitted --

/usr/bin/ld : rtsp-address-pool.c:(.text+0xb34) : référence indéfinie vers « g_once_init_leave_pointer »
/usr/bin/ld : /home/redacted/.conan2/p/b/gst-r24b7613cb3dab/p/lib/libgstrtspserver-1.0.a(rtsp-address-pool.c.o) : dans la fonction « gst_rtsp_address_pool_get_type » :
rtsp-address-pool.c:(.text+0xb74) : référence indéfinie vers « g_once_init_enter_pointer »
/usr/bin/ld : rtsp-address-pool.c:(.text+0xb94) : référence indéfinie vers « g_once_init_leave_pointer »
collect2: erreur: ld a retourné le statut de sortie 1
make[2]: *** [CMakeFiles/test_package.dir/build.make:130: test_package] Error 1
make[1]: *** [CMakeFiles/Makefile2:87: CMakeFiles/test_package.dir/all] Error 2
make: *** [Makefile:91: all] Error 2

Let me know, if 2.78.3 is keept, I'll revert 1229261 and dfa80b7.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's disappointing... There are several GNOME-related PRs open currently, though. Maybe it's possible to move to a newer glib version in them.

Realistically though, I'm not even sure whether any of them will get review/merged this year at current rate, so I would not hold my breath.

@@ -113,6 +113,7 @@ def package_info(self):
self.cpp_info.set_property("cmake_file_name", "ZXing")
self.cpp_info.set_property("cmake_target_name", "ZXing::ZXing")
self.cpp_info.set_property("pkg_config_name", "zxing")
self.cpp_info.includedirs = [os.path.join("include", "ZXing")]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also noticed that the zxing-cpp recipe is incorrect in its exported include dirs compared to the official exports, but I really don't like the library's choice of leaving out the prefix from its include paths. It adds some pretty bad header names like Error.h, Result.h, etc to the list of visible includes.
For that reason, I would prefer to use self.cpp_info.includedirs.append(os.path.join("include", "ZXing")) here and leave it out of this PR entirely.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll revert this and open an PR only for ZXing at conan-centre. Append makes more sense indeed.

@valgur
Copy link
Owner

valgur commented Jan 11, 2025

Thanks a lot, @fnadeau!! Looks good. I'll gladly merge this, if you can revert the glib versions and zxing-cpp tweaks.

The zxing-cpp recipe needs fixing, but I would prefer to leave it to a separate PR.

I had not noticed that a new GStreamer version is out either. That's nice. I'll have to check whether there are any changes to the plugins as well, probably.

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.

2 participants