diff mbox series

[bug#46113] gnu: obs: Update obs to fb347c.

Message ID 87eei7wugf.fsf@trop.in
State Accepted
Headers show
Series [bug#46113] gnu: obs: Update obs to fb347c. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Andrew Tropin Jan. 26, 2021, 3:59 p.m. UTC
This version of obs adds support for OBS_PLUGINS*_PATH environment variables,
which are required to be able to load plugins, which are present in profile.

It will make it possible for following packages to work:
http://issues.guix.gnu.org/45961
http://issues.guix.gnu.org/45960

* gnu/packages/video.scm (obs): Update to fb347c.
---
 gnu/packages/video.scm | 116 ++++++++++++++++++++++-------------------
 1 file changed, 63 insertions(+), 53 deletions(-)

Comments

Andrew Tropin Jan. 26, 2021, 4:26 p.m. UTC | #1
It's a commit from master branch containing changes from PR:
https://github.com/obsproject/obs-studio/pull/4067

It's just few commits after 26.1.2 and it seems there is no any big
changes since that and should be safe to use.

I asked for 26.1.3 tag, but it seems that it won't happen in nearest
future: https://github.com/obsproject/obs-studio/issues/4100

As one of developers said to me, usually obs releases once in 3-4
months, that is why I decided to use a commit instead of tag.

The changes is needed to be able to specify plugin load path using
native-search-paths to be able to package obs plugins with guix.
Leo Famulari Jan. 26, 2021, 9:10 p.m. UTC | #2
On Tue, Jan 26, 2021 at 06:59:39PM +0300, Andrew Tropin wrote:
> 
> This version of obs adds support for OBS_PLUGINS*_PATH environment variables,
> which are required to be able to load plugins, which are present in profile.
> 
> It will make it possible for following packages to work:
> http://issues.guix.gnu.org/45961
> http://issues.guix.gnu.org/45960

Thanks!

Your patch doesn't apply to the current master branch. Can you rebase it
and send a revision?
Alexey Abramov Jan. 27, 2021, 7:27 a.m. UTC | #3
Hi Adrew,

I patched obs here [1]. I checked the upstream patch, and noticed that it ammends AddExtraModulePaths function. With Guix the list of plugin directories will contain *two* locations with the very same plugins, one with the obs store and guix-profile, hence obs will print 'Duplicate library?' warning messages.

I packaged the main location to avoid such a message.

Footnotes:
[1]  https://issues.guix.gnu.org/45707
Andrew Tropin Jan. 27, 2021, 9:36 a.m. UTC | #4
> I patched obs here [1]. I checked the upstream patch, and noticed that
> it ammends AddExtraModulePaths function. With Guix the list of plugin
> directories will contain *two* locations with the very same plugins,
> one with the obs store and guix-profile, hence obs will print
> 'Duplicate library?' warning messages.

It's very true, obs, will have same plugins in load paths twice, but
from what I found the problem will happen only during shutdown of obs,
when it will try to unload the same plugin twice, which doesn't affect
runtime anyhow. I had a small workaround for that (a separate
envirnoment variable, which toggles the loading of plugins from
INSTALL_PREFIX), but during review one of obs mantainers said that it's
an adhoc solution and better to solve this problem in general. The
comment from @kkartaltepe and related changes [fn:1].

The other solution was to check if OBS_PLUGINS_PATH is present and ommit
loading of builtin plugins in that case, but it will break in case
someone wants to specify just additional plugin load path (some non-guix
use case).

I decided to go without any fix to that problem as it is not directly
related to changeset I proposed and doesn't affect runtime, but it's
probably a good idea to report double free on shutdown issue to
upstream.

* Footnotes

[fn:1] https://github.com/obsproject/obs-studio/pull/4067
Andrew Tropin Feb. 3, 2021, 2:40 p.m. UTC | #5
> Should we wait for the next OBS release instead of packaging an
> arbitrary commit?

I'm not in a hurry, but as I explained earlier in this thread [fn:2] it seems
relatively safe for me to use this commit and getting back later to
26.1.3 or 26.2.0 or whatever next release will be.

> I am not insist on keeping [1], but I do think that is more cleaner
> solution.

In terms of implememntation I like that [fn:1] prevents double loading
of plugins, by excluding obs installation dir from "load-path". However
double loading of the same plugin doesn't seem to break anything. Also,
OBS_PLUGINS_DIRECTORY variable name maybe a little better than
OBS_PLUGINS_PATH as it contains only one path.

The problem is that now there are two almost identical mechanisms (one
in upstream and one via patch [fn:1]), which can bring some maintanance
problems in the future.

There are two good option in my opinion:
- contribute patch from [fn:1] to upstream (reverting OBS_PLUGINS_PATH)
- revert [fn:1] and use OBS_PLUGINS_PATH from upstream

If Alexey ready to contibute OBS_PLUGINS_DIRECTORY patch to obs
(reverting OBS_PLUGINS_PATH), I would be glad to support it. Otherwise,
I would prefer to revert [fn:1] and apply this one. To prevent
maintanance problems in the future.

* Footnotes

[fn:2] http://issues.guix.gnu.org/46113 

[fn:1] https://issues.guix.gnu.org/45707 

--
Best regards,
Andrew Tropin
diff mbox series

Patch

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 28cde06f04..0f9b405261 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -3076,62 +3076,72 @@  be used for realtime video capture via Linux-specific APIs.")
     (license (list license:lgpl2.1+ license:gpl2))))
 
 (define-public obs
-  (package
-    (name "obs")
-    (version "26.1.0")
-    (source (origin
-              (method git-fetch)
-              (uri (git-reference
-                    (url "https://github.com/obsproject/obs-studio")
-                    (commit version)))
-              (file-name (git-file-name name version))
-              (sha256
-               (base32
-                "0p8wdzm9imn3s17arr206sz92g4pkacfcpfbwvhvgkrrs4w000bx"))))
-    (build-system cmake-build-system)
-    (arguments
-     `(#:configure-flags
-       (list (string-append "-DOBS_VERSION_OVERRIDE=" ,version)
-             "-DENABLE_UNIT_TESTS=TRUE")
-       #:phases
-       (modify-phases %standard-phases
-         (add-after 'install 'wrap-executable
-           (lambda* (#:key outputs #:allow-other-keys)
-             (let ((out (assoc-ref outputs "out"))
-                   (plugin-path (getenv "QT_PLUGIN_PATH")))
-               (wrap-program (string-append out "/bin/obs")
-                 `("QT_PLUGIN_PATH" ":" prefix (,plugin-path))))
-             #t)))))
-    (native-inputs
-     `(("cmocka" ,cmocka)
-       ("pkg-config" ,pkg-config)))
-    (inputs
-     `(("alsa-lib" ,alsa-lib)
-       ("curl" ,curl)
-       ("eudev" ,eudev)
-       ("ffmpeg" ,ffmpeg)
-       ("fontconfig" ,fontconfig)
-       ("freetype" ,freetype)
-       ("jack" ,jack-1)
-       ("jansson" ,jansson)
-       ("libx264" ,libx264)
-       ("libxcomposite" ,libxcomposite)
-       ("mbedtls" ,mbedtls-apache)
-       ("mesa" ,mesa)
-       ("pulseaudio" ,pulseaudio)
-       ("qtbase" ,qtbase)
-       ("qtsvg" ,qtsvg)
-       ("qtx11extras" ,qtx11extras)
-       ("speexdsp" ,speexdsp)
-       ("v4l-utils" ,v4l-utils)
-       ("zlib" ,zlib)))
-    (synopsis "Live streaming software")
-    (description "Open Broadcaster Software provides a graphical interface for
+  (let ((commit "fb347c3c62ced2ea302769e449d300fd923c2d4b")
+        (revision "1"))
+    (package
+      (name "obs")
+      (version (git-version "26.1.2" revision commit))
+      (source (origin
+                (method git-fetch)
+                (uri (git-reference
+                      (url "https://github.com/obsproject/obs-studio")
+                      (commit commit)))
+                (file-name (git-file-name name version))
+                (sha256
+                 (base32
+                  "017llgj1hlfvk2622qa44d8iz6d0kahhckn421dypj09a4n6aajz"))))
+      (build-system cmake-build-system)
+      (arguments
+       `(#:configure-flags
+         (list (string-append "-DOBS_VERSION_OVERRIDE=" ,version)
+               "-DENABLE_UNIT_TESTS=TRUE")
+         #:phases
+         (modify-phases %standard-phases
+           (add-after 'install 'wrap-executable
+             (lambda* (#:key outputs #:allow-other-keys)
+               (let ((out (assoc-ref outputs "out"))
+                     (plugin-path (getenv "QT_PLUGIN_PATH")))
+                 (wrap-program (string-append out "/bin/obs")
+                   `("QT_PLUGIN_PATH" ":" prefix (,plugin-path))))
+               #t)))))
+      (native-inputs
+       `(("cmocka" ,cmocka)
+         ("pkg-config" ,pkg-config)))
+      (inputs
+       `(("alsa-lib" ,alsa-lib)
+         ("curl" ,curl)
+         ("eudev" ,eudev)
+         ("ffmpeg" ,ffmpeg)
+         ("fontconfig" ,fontconfig)
+         ("freetype" ,freetype)
+         ("jack" ,jack-1)
+         ("jansson" ,jansson)
+         ("libx264" ,libx264)
+         ("libxcomposite" ,libxcomposite)
+         ("mbedtls" ,mbedtls-apache)
+         ("mesa" ,mesa)
+         ("pulseaudio" ,pulseaudio)
+         ("qtbase" ,qtbase)
+         ("qtsvg" ,qtsvg)
+         ("qtx11extras" ,qtx11extras)
+         ("speexdsp" ,speexdsp)
+         ("v4l-utils" ,v4l-utils)
+         ("zlib" ,zlib)))
+      (native-search-paths
+       (list
+        (search-path-specification
+         (variable "OBS_PLUGINS_DATA_PATH")
+         (files '("share/obs/obs-plugins")))
+        (search-path-specification
+         (variable "OBS_PLUGINS_PATH")
+         (files '("lib/obs-plugins")))))
+      (synopsis "Live streaming software")
+      (description "Open Broadcaster Software provides a graphical interface for
 video recording and live streaming.  OBS supports capturing audio and video
 from many input sources such as webcams, X11 (for screencasting), PulseAudio,
 and JACK.")
-    (home-page "https://obsproject.com")
-    (license license:gpl2+)))
+      (home-page "https://obsproject.com")
+      (license license:gpl2+))))
 
 (define-public libvdpau
   (package